Fix permission violation check for mmap and mprotect

This commit is contained in:
Hui, Chunyang 2024-01-15 12:12:20 +00:00 committed by volcano
parent e637ddbdfe
commit ee77ee618b
4 changed files with 79 additions and 0 deletions

@ -170,6 +170,11 @@ impl ShmManager {
if !vma.is_shared() { if !vma.is_shared() {
return_errno!(EINVAL, "not a shared chunk"); 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); Self::apply_new_perms_if_higher(&mut vma, new_perms);
Ok(()) Ok(())
} }

@ -284,6 +284,12 @@ impl ChunkManager {
Some(intersection_vma) => intersection_vma, 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() { if intersection_vma.range() == containing_vma.range() {
// The whole containing_vma is mprotected // The whole containing_vma is mprotected
containing_vma.set_perms(new_perms); containing_vma.set_perms(new_perms);

@ -89,6 +89,8 @@ impl VMManager {
} }
pub fn mmap(&self, options: &VMMapOptions) -> Result<usize> { pub fn mmap(&self, options: &VMMapOptions) -> Result<usize> {
mmap_file_check_permissions(options)?;
if LIBOS_CONFIG.feature.enable_posix_shm && options.is_shared() { if LIBOS_CONFIG.feature.enable_posix_shm && options.is_shared() {
let res = self.internal().mmap_shared_chunk(options); let res = self.internal().mmap_shared_chunk(options);
match res { match res {
@ -911,6 +913,12 @@ impl InternalVMManager {
return Ok(()); 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 current_pid = current!().process().pid();
let same_start = protect_range.start() == containing_vma.start(); let same_start = protect_range.start() == containing_vma.start();
let same_end = protect_range.end() == containing_vma.end(); 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) 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; 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 // 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_fdatasync),
TEST_CASE(test_shared_file_mmap_flushing_with_fsync), TEST_CASE(test_shared_file_mmap_flushing_with_fsync),
TEST_CASE(test_shared_file_mmap_small_file), 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_does_not_override_any_mmaping),
TEST_CASE(test_fixed_mmap_that_overrides_existing_mmaping), TEST_CASE(test_fixed_mmap_that_overrides_existing_mmaping),
TEST_CASE(test_fixed_mmap_with_non_page_aligned_addr), TEST_CASE(test_fixed_mmap_with_non_page_aligned_addr),