Fix permission violation check for mmap and mprotect
This commit is contained in:
		
							parent
							
								
									a99c63e21d
								
							
						
					
					
						commit
						d416c8b1d1
					
				| @ -170,6 +170,11 @@ impl ShmManager { | ||||
|         if !vma.is_shared() { | ||||
|             return_errno!(EINVAL, "not a shared chunk"); | ||||
|         } | ||||
|         if let Some((file_ref, _)) = vma.writeback_file() { | ||||
|             if !file_ref.access_mode().unwrap().writable() && new_perms.can_write() { | ||||
|                 return_errno!(EACCES, "file is not writable"); | ||||
|             } | ||||
|         } | ||||
|         Self::apply_new_perms_if_higher(&mut vma, new_perms); | ||||
|         Ok(()) | ||||
|     } | ||||
|  | ||||
| @ -284,6 +284,12 @@ impl ChunkManager { | ||||
|                 Some(intersection_vma) => intersection_vma, | ||||
|             }; | ||||
| 
 | ||||
|             if let Some((file_ref, _)) = intersection_vma.writeback_file() { | ||||
|                 if !file_ref.access_mode().unwrap().writable() && new_perms.can_write() { | ||||
|                     return_errno!(EACCES, "file is not writable"); | ||||
|                 } | ||||
|             } | ||||
| 
 | ||||
|             if intersection_vma.range() == containing_vma.range() { | ||||
|                 // The whole containing_vma is mprotected
 | ||||
|                 containing_vma.set_perms(new_perms); | ||||
|  | ||||
| @ -89,6 +89,8 @@ impl VMManager { | ||||
|     } | ||||
| 
 | ||||
|     pub fn mmap(&self, options: &VMMapOptions) -> Result<usize> { | ||||
|         mmap_file_check_permissions(options)?; | ||||
| 
 | ||||
|         if LIBOS_CONFIG.feature.enable_posix_shm && options.is_shared() { | ||||
|             let res = self.internal().mmap_shared_chunk(options); | ||||
|             match res { | ||||
| @ -911,6 +913,12 @@ impl InternalVMManager { | ||||
|                 return Ok(()); | ||||
|             } | ||||
| 
 | ||||
|             if let Some((file_ref, _)) = containing_vma.writeback_file() { | ||||
|                 if !file_ref.access_mode().unwrap().writable() && new_perms.can_write() { | ||||
|                     return_errno!(EACCES, "file is not writable"); | ||||
|                 } | ||||
|             } | ||||
| 
 | ||||
|             let current_pid = current!().process().pid(); | ||||
|             let same_start = protect_range.start() == containing_vma.start(); | ||||
|             let same_end = protect_range.end() == containing_vma.end(); | ||||
| @ -1219,3 +1227,30 @@ impl VMRemapParser for InternalVMManager { | ||||
|         self.free_manager.is_free_range(request_range) | ||||
|     } | ||||
| } | ||||
| 
 | ||||
| // Based on the mmap man page:
 | ||||
| // A file descriptor refers to a non-regular file.  Or a file
 | ||||
| // mapping was requested, but fd is not open for reading.  Or
 | ||||
| // MAP_SHARED was requested and PROT_WRITE is set, but fd is
 | ||||
| // not open in read/write (O_RDWR) mode.  Or PROT_WRITE is
 | ||||
| // set, but the file is append-only.
 | ||||
| fn mmap_file_check_permissions(mmap_options: &VMMapOptions) -> Result<()> { | ||||
|     match mmap_options.initializer() { | ||||
|         VMInitializer::FileBacked { file } => { | ||||
|             let (file_ref, _) = file.backed_file(); | ||||
|             if !file_ref.access_mode().unwrap().readable() { | ||||
|                 return_errno!(EACCES, "mmap file is not readable"); | ||||
|             } | ||||
| 
 | ||||
|             let perms = mmap_options.perms(); | ||||
|             if let Some((file_ref, _)) = file.writeback_file() { | ||||
|                 if !file_ref.access_mode().unwrap().writable() && perms.can_write() { | ||||
|                     return_errno!(EACCES, "mmap file is not writable"); | ||||
|                 } | ||||
|             } | ||||
| 
 | ||||
|             return Ok(()); | ||||
|         } | ||||
|         _ => return Ok(()), | ||||
|     } | ||||
| } | ||||
|  | ||||
| @ -1608,6 +1608,38 @@ int test_shared_file_mmap_small_file() { | ||||
|     return 0; | ||||
| } | ||||
| 
 | ||||
| int test_shared_file_mmap_permissions() { | ||||
|     const char *file_path = "/root/mmap_file.data"; | ||||
|     int fd = open(file_path, O_CREAT | O_TRUNC | O_RDONLY, 0644); | ||||
|     if (fd < 0) { | ||||
|         THROW_ERROR("file creation failed"); | ||||
|     } | ||||
| 
 | ||||
|     char *buf = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); | ||||
|     if (buf != MAP_FAILED || errno != EACCES) { | ||||
|         THROW_ERROR("permission violation not detected"); | ||||
|     } | ||||
| 
 | ||||
|     close(fd); | ||||
|     fd = open(file_path, O_CREAT | O_TRUNC | O_RDONLY, 0644); | ||||
|     if (fd < 0) { | ||||
|         THROW_ERROR("file creation failed"); | ||||
|     } | ||||
| 
 | ||||
|     buf = mmap(NULL, PAGE_SIZE, PROT_READ, MAP_SHARED, fd, 0); | ||||
|     if (buf == MAP_FAILED) { | ||||
|         THROW_ERROR("mmap failed"); | ||||
|     } | ||||
| 
 | ||||
|     int ret = mprotect(buf, PAGE_SIZE, PROT_READ | PROT_WRITE); | ||||
|     if (ret != -1 || errno != EACCES) { | ||||
|         printf("ret = %d, errno = %d\n", ret, errno); | ||||
|         THROW_ERROR("permission violation not detected"); | ||||
|     } | ||||
| 
 | ||||
|     return 0; | ||||
| } | ||||
| 
 | ||||
| // ============================================================================
 | ||||
| // Test suite main
 | ||||
| // ============================================================================
 | ||||
| @ -1628,6 +1660,7 @@ static test_case_t test_cases[] = { | ||||
|     TEST_CASE(test_shared_file_mmap_flushing_with_fdatasync), | ||||
|     TEST_CASE(test_shared_file_mmap_flushing_with_fsync), | ||||
|     TEST_CASE(test_shared_file_mmap_small_file), | ||||
|     TEST_CASE(test_shared_file_mmap_permissions), | ||||
|     TEST_CASE(test_fixed_mmap_that_does_not_override_any_mmaping), | ||||
|     TEST_CASE(test_fixed_mmap_that_overrides_existing_mmaping), | ||||
|     TEST_CASE(test_fixed_mmap_with_non_page_aligned_addr), | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user