Skip to content

Commit 2a2b444

Browse files
committed
fixup! snapshot: handle the scratch region correctly
Be slightly more careful about handling offsets from the guest in snapshot.rs: previously, a maliciously-constructed sandbox could possibly cause underflow panics, and another function could be misused in a way that would result in a slice out-of-bounds panic (although it was not currently being misused in such a way). Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com>
1 parent 638bcb7 commit 2a2b444

File tree

1 file changed

+12
-7
lines changed

1 file changed

+12
-7
lines changed

src/hyperlight_host/src/sandbox/snapshot.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,14 @@ fn access_gpa<'a>(
177177
scratch: &'a ExclusiveSharedMemory,
178178
scratch_size: usize,
179179
gpa: u64,
180-
) -> (&'a ExclusiveSharedMemory, usize) {
180+
) -> Option<(&'a ExclusiveSharedMemory, usize)> {
181181
let scratch_base = scratch_base_gpa(scratch_size);
182182
if gpa >= scratch_base {
183-
(scratch, (gpa - scratch_base) as usize)
183+
Some((scratch, (gpa - scratch_base) as usize))
184+
} else if gpa >= SandboxMemoryLayout::BASE_ADDRESS as u64 {
185+
Some((snap, gpa as usize - SandboxMemoryLayout::BASE_ADDRESS))
184186
} else {
185-
(snap, gpa as usize - SandboxMemoryLayout::BASE_ADDRESS)
187+
None
186188
}
187189
}
188190

@@ -222,8 +224,8 @@ impl<'a> hyperlight_common::vmem::TableOps for SharedMemoryPageTableBuffer<'a> {
222224
addr + offset
223225
}
224226
unsafe fn read_entry(&self, addr: u64) -> u64 {
225-
let (mem, off) = access_gpa(self.snap, self.scratch, self.scratch_size, addr);
226-
let Some(pte_bytes) = mem.as_slice().get(off..off + 8) else {
227+
let memoff = access_gpa(self.snap, self.scratch, self.scratch_size, addr);
228+
let Some(pte_bytes) = memoff.and_then(|(mem, off)| mem.as_slice().get(off..off + 8)) else {
227229
// Attacker-controlled data pointed out-of-bounds. We'll
228230
// default to returning 0 in this case, which, for most
229231
// architectures (including x86-64 and arm64, the ones we
@@ -303,9 +305,12 @@ unsafe fn guest_page<'a>(
303305
scratch_size: usize,
304306
gpa: u64,
305307
) -> Option<&'a [u8]> {
308+
if !gpa.is_multiple_of(PAGE_SIZE as u64) {
309+
return None;
310+
}
306311
let gpa_u = gpa as usize;
307312
for rgn in regions {
308-
if gpa_u >= rgn.guest_region.start && gpa_u < rgn.guest_region.end {
313+
if gpa_u >= rgn.guest_region.start && gpa_u + PAGE_SIZE <= rgn.guest_region.end {
309314
let off = gpa_u - rgn.guest_region.start;
310315
#[allow(clippy::useless_conversion)]
311316
let host_region_base: usize = rgn.host_region.start.into();
@@ -314,7 +319,7 @@ unsafe fn guest_page<'a>(
314319
});
315320
}
316321
}
317-
let (mem, off) = access_gpa(snap, scratch, scratch_size, gpa);
322+
let (mem, off) = access_gpa(snap, scratch, scratch_size, gpa)?;
318323
if off + PAGE_SIZE <= mem.as_slice().len() {
319324
Some(&mem.as_slice()[off..off + PAGE_SIZE])
320325
} else {

0 commit comments

Comments
 (0)