Fix some `mprotect` issues

This commit is contained in:
Wang Siyuan 2025-09-21 12:19:03 +00:00 committed by Junyang Zhang
parent c007ac90e9
commit 57d3d9ded1
6 changed files with 137 additions and 10 deletions

View File

@ -84,6 +84,8 @@ fn do_sys_mmap(
vm_perms
};
let mut vm_may_perms = VmPerms::ALL_MAY_PERMS;
let user_space = ctx.user_space();
let root_vmar = user_space.root_vmar();
let vm_map_options = {
@ -124,14 +126,15 @@ fn do_sys_mmap(
if vm_perms.contains(VmPerms::READ) && !access_mode.is_readable() {
return_errno!(Errno::EACCES);
}
if option.typ() == MMapType::Shared
&& vm_perms.contains(VmPerms::WRITE)
&& !access_mode.is_writable()
{
return_errno!(Errno::EACCES);
if option.typ() == MMapType::Shared && !access_mode.is_writable() {
if vm_perms.contains(VmPerms::WRITE) {
return_errno!(Errno::EACCES);
}
vm_may_perms.remove(VmPerms::MAY_WRITE);
}
options = options
.may_perms(vm_may_perms)
.mappable(file.mappable()?)
.vmo_offset(offset)
.handle_page_faults_around();

View File

@ -4,8 +4,11 @@ use aster_rights::Rights;
use bitflags::bitflags;
use ostd::mm::PageFlags;
use crate::prelude::*;
bitflags! {
/// The memory access permissions of memory mappings.
// NOTE: `check` hardcodes `MAY_READ >> 3 == READ`, and so for r/w/x bits.
pub struct VmPerms: u32 {
/// Readable.
const READ = 1 << 0;
@ -13,6 +16,31 @@ bitflags! {
const WRITE = 1 << 1;
/// Executable.
const EXEC = 1 << 2;
/// May be protected to readable.
const MAY_READ = 1 << 3;
/// May be protected to writable.
const MAY_WRITE = 1 << 4;
/// May be protected to executable.
const MAY_EXEC = 1 << 5;
/// All permissions (READ | WRITE | EXEC).
const ALL_PERMS = Self::READ.bits | Self::WRITE.bits | Self::EXEC.bits;
/// All `MAY_*` permissions (MAY_READ | MAY_WRITE | MAY_EXEC).
const ALL_MAY_PERMS = Self::MAY_READ.bits | Self::MAY_WRITE.bits | Self::MAY_EXEC.bits;
}
}
impl VmPerms {
/// Checks whether all requested permissions (`READ`, `WRITE`, `EXEC`) are
/// allowed by their corresponding `MAY_*` capabilities.
pub fn check(&self) -> Result<()> {
let requested = *self & Self::ALL_PERMS;
// NOTE: `MAY_READ >> 3 == READ`, and so for r/w/x bits.
let allowed = VmPerms::from_bits_truncate((*self & Self::ALL_MAY_PERMS).bits >> 3);
if !allowed.contains(requested) {
return_errno_with_message!(Errno::EACCES, "permission denied");
}
Ok(())
}
}

View File

@ -465,9 +465,12 @@ impl Vmar_ {
}
for (vm_mapping_addr, vm_mapping_perms) in protect_mappings {
if perms == vm_mapping_perms {
if perms == vm_mapping_perms & VmPerms::ALL_PERMS {
continue;
}
let new_perms = perms | (vm_mapping_perms & VmPerms::ALL_MAY_PERMS);
new_perms.check()?;
let vm_mapping = inner.remove(&vm_mapping_addr).unwrap();
let vm_mapping_range = vm_mapping.range();
let intersected_range = get_intersected_range(&range, &vm_mapping_range);
@ -484,7 +487,7 @@ impl Vmar_ {
}
// Protects part of the `VmMapping`.
let taken = taken.protect(vm_space.as_ref(), perms);
let taken = taken.protect(vm_space.as_ref(), new_perms);
inner.insert_try_merge(taken);
}
@ -815,6 +818,7 @@ pub struct VmarMapOptions<'a, R1, R2> {
vmo: Option<Vmo<R2>>,
mappable: Option<Mappable>,
perms: VmPerms,
may_perms: VmPerms,
vmo_offset: usize,
size: usize,
offset: Option<usize>,
@ -839,6 +843,7 @@ impl<'a, R1, R2> VmarMapOptions<'a, R1, R2> {
vmo: None,
mappable: None,
perms,
may_perms: VmPerms::ALL_MAY_PERMS,
vmo_offset: 0,
size,
offset: None,
@ -849,6 +854,18 @@ impl<'a, R1, R2> VmarMapOptions<'a, R1, R2> {
}
}
/// Sets the `VmPerms::MAY*` memory access permissions of the mapping.
///
/// The default value is `MAY_READ | MAY_WRITE | MAY_EXEC`.
///
/// The provided `may_perms` must be a subset of all the may-permissions,
/// and must include the may-permissions corresponding to already requested
/// normal permissions (`READ | WRITE | EXEC`).
pub fn may_perms(mut self, may_perms: VmPerms) -> Self {
self.may_perms = may_perms;
self
}
/// Binds a [`Vmo`] to the mapping.
///
/// If the mapping is a private mapping, its size may not be equal to that
@ -992,6 +1009,7 @@ where
vmo,
mappable,
perms,
may_perms,
vmo_offset,
size: map_size,
offset,
@ -1072,7 +1090,7 @@ where
inode,
is_shared,
handle_page_faults_around,
perms,
perms | may_perms,
);
// Populate device memory if needed before adding to VMAR.
@ -1119,11 +1137,20 @@ where
/// Checks whether the permissions of the mapping is subset of vmo rights.
fn check_perms(&self) -> Result<()> {
if !VmPerms::ALL_MAY_PERMS.contains(self.may_perms)
|| !VmPerms::ALL_PERMS.contains(self.perms)
{
return_errno_with_message!(Errno::EACCES, "invalid perms");
}
let vm_perms = self.perms | self.may_perms;
vm_perms.check()?;
let Some(vmo) = &self.vmo else {
return Ok(());
};
let perm_rights = Rights::from(self.perms);
let perm_rights = Rights::from(vm_perms);
vmo.check_rights(perm_rights)
}
}

View File

@ -200,6 +200,13 @@ impl VmMapping {
Ok(())
}
/// Returns whether this mapping is a COW mapping.
///
/// Reference: <https://elixir.bootlin.com/linux/v6.16.5/source/include/linux/mm.h#L1470-L1473>
fn is_cow(&self) -> bool {
!self.is_shared && self.perms.contains(VmPerms::MAY_WRITE)
}
}
/****************************** Page faults **********************************/
@ -629,11 +636,16 @@ impl VmMapping {
/// Change the perms of the mapping.
pub(super) fn protect(self, vm_space: &VmSpace, perms: VmPerms) -> Self {
let mut new_flags = PageFlags::from(perms);
if self.is_cow() && !self.perms.contains(VmPerms::WRITE) {
new_flags.remove(PageFlags::W);
}
let preempt_guard = disable_preempt();
let range = self.range();
let mut cursor = vm_space.cursor_mut(&preempt_guard, &range).unwrap();
let op = |flags: &mut PageFlags, _cache: &mut CachePolicy| *flags = perms.into();
let op = |flags: &mut PageFlags, _cache: &mut CachePolicy| *flags = new_flags;
while cursor.virt_addr() < range.end {
if let Some(va) = cursor.protect_next(range.end - cursor.virt_addr(), op) {
cursor.flusher().issue_tlb_flush(TlbFlushOp::for_range(va));

View File

@ -0,0 +1,56 @@
// SPDX-License-Identifier: MPL-2.0
#define _GNU_SOURCE
#include <sys/fcntl.h>
#include <sys/mman.h>
#include <unistd.h>
#include <string.h>
#include "../test.h"
#define PAGE_SIZE 4096
const char *filename = "testfile";
FN_TEST(mprotect_shared_writable_mapping_on_read_only_file)
{
int fd = TEST_SUCC(open(filename, O_RDWR | O_CREAT | O_TRUNC, 0600));
TEST_SUCC(ftruncate(fd, PAGE_SIZE));
TEST_SUCC(write(fd, "AAAA", 5));
TEST_SUCC(close(fd));
fd = TEST_SUCC(open(filename, O_RDONLY));
char *addr =
CHECK_WITH(mmap(NULL, PAGE_SIZE, PROT_READ, MAP_SHARED, fd, 0),
_ret != MAP_FAILED);
TEST_ERRNO(mprotect(addr, PAGE_SIZE, PROT_READ | PROT_WRITE), EACCES);
TEST_SUCC(close(fd));
TEST_SUCC(unlink(filename));
}
END_TEST()
FN_TEST(mprotect_private_writable_mapping_copy_on_write)
{
int fd = TEST_SUCC(open(filename, O_RDWR | O_CREAT | O_TRUNC, 0600));
TEST_SUCC(ftruncate(fd, PAGE_SIZE));
TEST_SUCC(write(fd, "AAAA", 5));
char *addr1 =
CHECK_WITH(mmap(NULL, PAGE_SIZE, PROT_READ, MAP_PRIVATE, fd, 0),
_ret != MAP_FAILED);
char *addr2 =
CHECK_WITH(mmap(NULL, PAGE_SIZE, PROT_READ, MAP_PRIVATE, fd, 0),
_ret != MAP_FAILED);
TEST_RES(strcmp(addr1, "AAAA"), _ret == 0);
TEST_RES(strcmp(addr2, "AAAA"), _ret == 0);
TEST_SUCC(mprotect(addr1, PAGE_SIZE, PROT_READ | PROT_WRITE));
memcpy(addr1, "BBBB", 5);
TEST_RES(strcmp(addr1, "BBBB"), _ret == 0);
TEST_RES(strcmp(addr2, "AAAA"), _ret == 0);
TEST_SUCC(close(fd));
TEST_SUCC(unlink(filename));
}
END_TEST()

View File

@ -28,6 +28,7 @@ hello_world/hello_world
itimer/setitimer
itimer/timer_create
mmap/mmap_and_fork
mmap/mmap_and_mprotect
mmap/mmap_and_mremap
mmap/mmap_beyond_the_file
mmap/mmap_shared_filebacked