Honor GRATE_MEMORY_FLAG in mmap_syscall and brk_syscall#1185
Open
rennergade wants to merge 25 commits into
Open
Honor GRATE_MEMORY_FLAG in mmap_syscall and brk_syscall#1185rennergade wants to merge 25 commits into
rennergade wants to merge 25 commits into
Conversation
`mmap_syscall` used to do `useraddr = addr as u32` unconditionally, truncating the high 32 bits of any host pointer the grate had supplied via GRATE_MEMORY_FLAG. The truncated low bits then got re-translated through the calling cage's vmmap, landing the mapping at an arbitrary cage address — typically the end of the empty vmmap gap. The cage's subsequent memcpy clobbered whatever was already in that region (stack, locals, fds) and downstream syscalls failed with EBADF or crashes that didn't trace back to the mmap itself. `brk_syscall` had the inverse gap: it always interprets the address as a cage uaddr, so a grate that wanted to set the program break via a host sysaddr had no way to do it. Add two helpers in `typemap::datatype_conversion`: - `sc_convert_addr_to_sys(arg, arg_cageid, cageid)`: if `arg_cageid & GRATE_MEMORY_FLAG` is set, the grate has already computed a host system address — use it directly. Otherwise the arg is a uaddr in the calling cage's memory; translate via that cage's vmmap base. - `sc_convert_sys_to_user(sysaddr, cageid)`: inverse, for return-value bookkeeping and the brk case. Wire both into mmap_syscall (both MAP_FIXED and find_map_space paths) and brk_syscall (sysaddr → uaddr when the flag is set). Add `GRATE_MEMORY_FLAG` and `LIND_ARG_CAGEID_MASK` constants in sysdefs/lind_platform_const.rs to mirror glibc's addr_translation.h. Scope: only `mmap`/`mmap64` (uaddr convention per glibc `mmap.c`) and `brk` (raw uaddr per glibc `brk.c`) need this. `munmap`, `mprotect`, `shmat`, `shmdt` already receive pre-translated sysaddrs from their glibc wrappers (via `TRANSLATE_GUEST_POINTER_TO_HOST`), so the runtime already uses them correctly regardless of the flag — leaving them untouched avoids double-translating the cage path. Adds a grate test in `tests/grate-tests/simple-tests/` (mmap-flag.c + mmap-flag_grate.c): the grate registers an mmap handler that forwards to RawPOSIX with `GRATE_MEMORY_FLAG` set on the addr's cageid; the cage mmap-write-readback-munmap's a page. Pre-patch this trips the truncation bug.
When the runtime's own early_init_stack calls mmap(addr=0, MAP_FIXED, ...) on a cage whose mmap was registered to a grate (e.g. by a grate test), the grate forwards the call with GRATE_MEMORY_FLAG set. My helper treated FLAG-set + addr=0 as "the grate handed us a NULL host pointer", returned sysaddr=0, and the inverse sys_to_user(0) returned EINVAL because 0 < cage.base_address. Symptom: "failed to allocate stack" during cage exec. A NULL host pointer is meaningless for mmap. Treat arg=0 as cage-relative regardless of the flag — that matches the pre-flag behavior of `useraddr = addr as u32 (= 0); sysaddr = user_to_sys(0) = base`.
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
I had the semantic wrong. Looking at imfs-grate's actual usage (rust-grates/imfs-grate/src/imfs/mod.rs around the cage-side mmap forward): the grate passes target_addr = a uaddr in its OWN linear memory (returned from a prior SYS_MMAP that allocated the imfs content region) along with arg_cage = grate_id | GRATE_MEMORY_FLAG. The runtime is supposed to read this as "uaddr in the named cage's memory" and resolve via that cage's base — not as "use as-is". Update the helper so: - flag unset → owner = calling cage (existing cage-side path) - flag set → owner = arg_cageid & LIND_ARG_CAGEID_MASK - sysaddr = base(owner) + arg This matches Sanchit's "all within addr_translation.h" framing — the generalization of the glibc-side __lind_translate_uaddr_to_host to arbitrary cageid, which only the runtime can do. Removes the temporary diagnostic eprintln.
The earlier GRATE_MEMORY_FLAG-aware branches in mmap_syscall and brk_syscall were dead code: by the time a syscall reaches the runtime, glibc's TRANSLATE_ARG_TO_HOST (invoked by make_threei_call for every arg) has already consumed the flag — translating uaddr → host pointer when the flag was set and stripping the flag bit from the cageid. The runtime always sees `(post-translation arg, flag-stripped cageid)`. For byte-buffer args (paths, read/write bufs, etc.) the distinction is invisible — the runtime dereferences the host pointer either way. For mmap/brk the runtime has to *interpret* the address, so it has to distinguish a uaddr (translate via calling cage's base) from a host sysaddr (use as-is). Use value range as the discriminator: wasm32 uaddrs are ≤ 4GB by definition; host base addresses (and therefore sysaddrs) are typically far above 4GB. The ranges don't overlap. - sc_convert_addr_to_sys now keys on `arg <= u32::MAX`. - brk_syscall now keys on the same range check, dropping the GRATE_MEMORY_FLAG check. - Removed unused GRATE_MEMORY_FLAG import from fs_calls.rs and the temporary diagnostic eprintln from mmap_syscall.
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Anonymous / fd=-1 mmaps (including the runtime's pre-main early_init_stack call) are forwarded by the grate unchanged. Tagging those with GRATE_MEMORY_FLAG would resolve addr against the grate's base instead of the cage's, breaking unrelated runtime mmaps. Cage test switched from MAP_ANONYMOUS|MAP_PRIVATE to an fd-backed MAP_SHARED mmap so the FLAG path is the one being exercised.
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
… id) The first cageid param to a grate handler is the GRATE's id (3i's _call_grate_func passes grateid there), not the calling cage. Forwarding with target=cageid mmap'd at the grate's base, clobbering the grate's function table — the cage's instance setup completed normally, then the grate's next call_indirect trapped with 'uninitialized element'. imfs already documents this: arg5 (fd) is an integer whose cage tag is reliably the original caller (handlers.rs:1219-1223). Use arg5cage as the forwarding target.
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rennergade
requested review from
Yaxuan-w,
drapl0n,
qianxichen233 and
stupendoussuperpowers
and removed request for
Yaxuan-w
| /// - `Ok(sysaddr)` host system address. | ||
| /// - `Err(Errno::EINVAL)` if the calling cage can't be looked up or its vmmap | ||
| /// has no base address yet (only on the uaddr branch). | ||
| pub fn sc_convert_addr_to_sys(arg: u64, arg_cageid: u64, cageid: u64) -> Result<usize, Errno> { |
Contributor
There was a problem hiding this comment.
I think we already have a sc_convert_addr_to_host ? We can just update that directly, they are not used anywhere else.
stupendoussuperpowers
previously approved these changes
May 11, 2026
Adds stderr-flushed eprintlns at every decision point in mmap_syscall so we can identify which call kills the runtime mid-printf. Drop before merge — same shape as the previous c4ba2a7/ef4846fb3 cycle.
This reverts commit 3829f3a.
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…new helpers Per Sanchit's review on #1185 + issue #1186: the existing unused sc_convert_uaddr_to_host already takes (uaddr, addr_cageid) and uses addr_cageid to look up the right cage's base — which is exactly the shape we need for both cage-userland calls and grate-forwarded calls (where glibc strips FLAG and leaves addr_cageid as the grate's id). Changes: - sc_convert_uaddr_to_host: drop the NULL early-return (so uaddr=0 anchors to the cage's base — needed for early_init_stack-style MAP_FIXED at 0); change the 'uaddr >= base_addr' panic into a passthrough (already-translated host pointer). - Delete sc_convert_addr_to_sys; mmap_syscall and brk_syscall now call sc_convert_uaddr_to_host directly. - Rename sc_convert_sys_to_user to sc_convert_host_to_uaddr for naming symmetry with the forward direction. Keep it as the inverse helper for return-value bookkeeping (mmap result -> cage uaddr, brk arg passed as host sysaddr -> cage uaddr). No behavior change vs the prior PR commits — same value-range disambiguation, same passthrough semantics, just routed through the existing helper.
check_addr_read / check_addr_rw assume addr is a pre-translated host pointer and do 'addr - base_addr' inside calculate_page_range. When a grate calls copy_data_between_cages(..., cage_uaddr, cage_id, ...) with a uaddr returned from a previous mmap (POSIX doesn't translate return values, and glibc's TRANSLATE_UADDR_TO_HOST only fires when arg_cageid == __lind_cageid — i.e. when the addr is in the caller's own cage), the uaddr arrives untranslated, addr - base_addr underflows in u64, the page_num is garbage, and validation reports 'range invalid'. End-effect for imfs's mmap interposition: copy_data_between_cages fails the validation when seeding a freshly-mmap'd cage region with file contents, so the cage reads zeros. Fix mirrors the mmap_syscall / sc_convert_uaddr_to_host change: if 'addr >= base_addr' treat as host pointer (subtract base); otherwise treat as cage uaddr (use directly). Same passthrough disambiguation.
Issue #1186 part 2. copy_data_between_cages did 'let host_src_addr = srcaddr' without actually translating — it assumed glibc's TRANSLATE_UADDR_TO_HOST had already converted the addr to a host pointer. That only happens for args owned by the calling cage; cross- cage args (e.g. a uaddr returned from a previous mmap that the caller is passing back in for someone else's cage) arrive untranslated. End-effect: when imfs's grate calls copy_data_between_cages(..., cage_uaddr, cage_id, ...) to seed a freshly-mmap'd cage region with file contents, the cage_uaddr reached the runtime as a small integer. On main, _validate_range_* underflowed and returned 'range invalid'; the copy never ran. Loosening calculate_page_range to accept the small value let validation pass, but the subsequent std::ptr::copy_nonoverlapping at the same small value segfaulted. Fix: translate srcaddr and destaddr at the top of copy_data_between_cages via sc_convert_uaddr_to_host, which handles both forms — host-pointer passthrough, cage uaddr gets the named cage's base added. After that, _validate_range_* and the actual copy_nonoverlapping use translated host pointers and work normally. Reverts the calculate_page_range loosening from the previous commit; that was the wrong layer to fix this at.
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let base_addr = vmmap.base_address.unwrap() as u64; | ||
|
|
||
| // Only convert to host if not already converted. | ||
| if uaddr >= base_addr { |
Member
There was a problem hiding this comment.
I think this should check if the top bits match the base_addr, not this.
In practice, you should have a helper function for this to handle situations where we do not have 32 bit aligned 4GB regions.
| use std::collections::{hash_map::Entry, HashMap}; | ||
| use std::collections::HashMap; | ||
| use std::sync::Mutex; | ||
| use sysdefs::constants::lind_platform_const; |
| /// - If no entry exists for `self_cageid`. | ||
| /// - If no entry exists for `syscall_num`. | ||
| pub fn _get_handler(self_cageid: u64, syscall_num: u64, target_cageid: u64) -> Option<(u64, u64)> { | ||
| pub fn _get_handler(self_cageid: u64, syscall_num: u64, _target_cageid: u64) -> Option<(u64, u64)> { |
Member
There was a problem hiding this comment.
Are you making the linter happy, or...?
Comment on lines
+35
to
+39
| /// MSB of a syscall arg's cageid: signals that the arg should be treated as a | ||
| /// host-side reference into the named cage's linear memory, not as a uaddr | ||
| /// in the calling cage's memory. Mirrors `LIND_ARG_TRANSLATE_FLAG` in | ||
| /// `src/glibc/lind_syscall/addr_translation.h`. | ||
| pub const GRATE_MEMORY_FLAG: u64 = 1u64 << 63; |
Member
There was a problem hiding this comment.
I don't really understand this. Need to discuss
| /// `src/glibc/lind_syscall/addr_translation.h`. | ||
| pub const GRATE_MEMORY_FLAG: u64 = 1u64 << 63; | ||
| /// Mask to recover the actual cageid by clearing `GRATE_MEMORY_FLAG`. | ||
| pub const LIND_ARG_CAGEID_MASK: u64 = !GRATE_MEMORY_FLAG; |
Member
There was a problem hiding this comment.
Would it be better to make a macro that uses this?
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
End-to-End Test ReportTest Previewgrate harnessGrate Test Report
Cases
static harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
wasm harnessTest ReportDeterministic TestsSummary
Test Results by Category
Fail TestsSummary
Test Results by Category
C++ harnessSummary
Cases
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mmap_syscallused to douseraddr = addr as u32unconditionally, truncating the high 32 bits of any host pointer a grate had supplied viaGRATE_MEMORY_FLAG. The truncated low bits then got re-translated through the calling cage's vmmap, landing the mapping at an arbitrary cage address. The cage's subsequent memcpy clobbered whatever lived there (stack, locals, fds) and downstream syscalls failed in confusing ways that didn't trace back to the mmap.brk_syscallhad the inverse gap: it always interprets the address as a cage uaddr, so a grate that wanted to set the program break via a host sysaddr had no way to do it.Implementation note (subtle):
GRATE_MEMORY_FLAGis consumed glibc-side insidemake_threei_call/TRANSLATE_ARG_TO_HOST— by the time the runtime sees the call, the flag bit has been stripped fromarg_cageidand (if it was set) the arg is the already-resolved host pointer. The runtime therefore can't check the flag directly. It distinguishes the two cases by value range: cage linear memory occupies[base, base+4GB]on the host, and host bases are far above 4GB, so any arg aboveu32::MAXmust be a pre-translated host sysaddr.Adds two helpers in
typemap::datatype_conversion:sc_convert_addr_to_sys(arg, arg_cageid, cageid): returns the host sysaddr.arg == 0anchors to the calling cage's base (soearly_init_stack-styleaddr=0,MAP_FIXEDcalls still work).arg ≤ u32::MAXis treated as a uaddr and translated through the calling cage's vmmap.arg > u32::MAXis used as-is (grate already translated it).sc_convert_sys_to_user(sysaddr, cageid): inverse, for return-value bookkeeping and the brk case.Wired into both branches of
mmap_syscall(find_map_space hint resolution and the MAP_FIXED path) and intobrk_syscall(sysaddr → uaddr when the arg looks like a host sysaddr). AddsGRATE_MEMORY_FLAGandLIND_ARG_CAGEID_MASKconstants insysdefs/lind_platform_const.rsto mirror glibc'saddr_translation.h.Scope: only
mmap/mmap64(uaddr convention per glibcmmap.c) andbrk(raw uaddr per glibcbrk.c) need this.munmap,mprotect,shmat,shmdtalready receive pre-translated sysaddrs from their glibc wrappers (viaTRANSLATE_GUEST_POINTER_TO_HOST), so the runtime already uses them correctly regardless of the flag — leaving them untouched avoids double-translating the cage path.Test
tests/grate-tests/simple-tests/mmap-flag.c+mmap-flag_grate.c. The grate registers an mmap handler that forwards fd-backed mmaps to RawPOSIX withGRATE_MEMORY_FLAGonarg1cage, exercising the runtime's value-range path. Anonymous mmaps (including the cage's runtime-internalinstance.rssetup mmap) are forwarded unchanged so they don't trip the FLAG path. The cage opens a file, mmaps it MAP_SHARED, writes/reads back a page, munmaps.