Skip to content

Commit

Permalink
mpk: protect memory with PROT_NONE (#7363)
Browse files Browse the repository at this point in the history
This change fixes a bug with `ProtectionKey::protect`: previously it
initialized each stripe with read and write permissions (i.e.,
`pkey_mprotect(..., PROT_READ | PROT_WRITE)` under the mistaken
assumption that these permissions were MPK-specific, "what MPK
permissions will we be allowed to set in the PKRU for these regions in
the future?". This assumption is incorrect: the regions were immediately
made accessible for reading and writing. The fix is to initially protect
the regions with `PROT_NONE` and allow Wasmtime's `memory.grow`
implementation to mark pages with `mprotect(..., PROT_READ |
PROT_WRITE)` as usual. Whether a store can access a slice is still
determined by the CPU state set in `mpk::allow`.
  • Loading branch information
abrown authored Oct 25, 2023
1 parent 1c81b5c commit d58f526
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 9 deletions.
4 changes: 2 additions & 2 deletions crates/runtime/src/mpk/enabled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl ProtectionKey {
pub fn protect(&self, region: &mut [u8]) -> Result<()> {
let addr = region.as_mut_ptr() as usize;
let len = region.len();
let prot = sys::PROT_READ | sys::PROT_WRITE;
let prot = sys::PROT_NONE;
sys::pkey_mprotect(addr, len, prot, self.id).with_context(|| {
format!(
"failed to mark region with pkey (addr = {addr:#x}, len = {len}, prot = {prot:#b})"
Expand Down Expand Up @@ -169,7 +169,7 @@ mod tests {
assert!(result.is_err());
assert_eq!(
result.unwrap_err().to_string(),
"failed to mark region with pkey (addr = 0x1, len = 1, prot = 0b11)"
"failed to mark region with pkey (addr = 0x1, len = 1, prot = 0b0)"
);
}

Expand Down
11 changes: 4 additions & 7 deletions crates/runtime/src/mpk/sys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,10 @@ use crate::page_size;
use anyhow::Result;
use std::io::Error;

/// Protection mask allowing reads of pkey-protected memory (see `prot` in
/// [`pkey_mprotect`]).
pub const PROT_READ: u32 = libc::PROT_READ as u32; // == 0b0001.

/// Protection mask allowing writes of pkey-protected memory (see `prot` in
/// [`pkey_mprotect`]).
pub const PROT_WRITE: u32 = libc::PROT_WRITE as u32; // == 0b0010;
/// Protection mask disallowing reads and writes of pkey-protected memory (see
/// `prot` in [`pkey_mprotect`]); in Wasmtime we expect all MPK-protected memory
/// to start as `PROT_NONE`.
pub const PROT_NONE: u32 = libc::PROT_NONE as u32; // == 0b0000;

/// Allocate a new protection key in the Linux kernel ([docs]); returns the
/// key ID.
Expand Down

0 comments on commit d58f526

Please sign in to comment.