Skip to content

Commit 2e8e8e6

Browse files
alexandruagandreeaflorescu
authored andcommitted
fix zero-length accesses
Relax the assertions from write_to and read_from to allow equality between offset and count, which no longer leads to panics when zero-length accesses happen. This is preferred versus adding an explicit check for count or len == 0 somewhere, because we don't end up with an additional branch on the regular path. Also tweaked the try_access logic a bit, to return Ok(0) when total == 0 but we found a valid region associated with the current address (which indicates a zero-length access). Signed-off-by: Alexandru Agache <[email protected]>
1 parent 82e9349 commit 2e8e8e6

File tree

1 file changed

+44
-3
lines changed

1 file changed

+44
-3
lines changed

src/guest_memory.rs

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ pub trait GuestMemory {
501501
let len = std::cmp::min(cap, (count - total) as GuestUsize);
502502
match f(total, len as usize, start, region) {
503503
// no more data
504-
Ok(0) => break,
504+
Ok(0) => return Ok(total),
505505
// made some progress
506506
Ok(len) => {
507507
total += len;
@@ -687,7 +687,7 @@ impl<T: GuestMemory> Bytes<GuestAddress> for T {
687687
{
688688
self.try_access(count, addr, |offset, len, caddr, region| -> Result<usize> {
689689
// Check if something bad happened before doing unsafe things.
690-
assert!(offset < count);
690+
assert!(offset <= count);
691691
if let Some(dst) = unsafe { region.as_mut_slice() } {
692692
// This is safe cause `start` and `len` are within the `region`.
693693
let start = caddr.raw_value() as usize;
@@ -753,7 +753,7 @@ impl<T: GuestMemory> Bytes<GuestAddress> for T {
753753
{
754754
self.try_access(count, addr, |offset, len, caddr, region| -> Result<usize> {
755755
// Check if something bad happened before doing unsafe things.
756-
assert!(offset < count);
756+
assert!(offset <= count);
757757
if let Some(src) = unsafe { region.as_slice() } {
758758
// This is safe cause `start` and `len` are within the `region`.
759759
let start = caddr.raw_value() as usize;
@@ -962,4 +962,45 @@ mod tests {
962962
fn test_non_atomic_access() {
963963
non_atomic_access_helper::<u16>()
964964
}
965+
966+
#[cfg(feature = "backend-mmap")]
967+
#[test]
968+
fn test_zero_length_accesses() {
969+
#[derive(Default, Clone, Copy)]
970+
#[repr(C)]
971+
struct ZeroSizedStruct {
972+
dummy: [u32; 0],
973+
}
974+
975+
unsafe impl ByteValued for ZeroSizedStruct {}
976+
977+
let addr = GuestAddress(0x1000);
978+
let mem = GuestMemoryMmap::from_ranges(&[(addr, 0x1000)]).unwrap();
979+
let obj = ZeroSizedStruct::default();
980+
let mut image = make_image(0x80);
981+
982+
assert_eq!(mem.write(&[], addr).unwrap(), 0);
983+
assert_eq!(mem.read(&mut [], addr).unwrap(), 0);
984+
985+
assert!(mem.write_slice(&[], addr).is_ok());
986+
assert!(mem.read_slice(&mut [], addr).is_ok());
987+
988+
assert!(mem.write_obj(obj, addr).is_ok());
989+
assert!(mem.read_obj::<ZeroSizedStruct>(addr).is_ok());
990+
991+
assert_eq!(mem.read_from(addr, &mut Cursor::new(&image), 0).unwrap(), 0);
992+
993+
assert!(mem
994+
.read_exact_from(addr, &mut Cursor::new(&image), 0)
995+
.is_ok());
996+
997+
assert_eq!(
998+
mem.write_to(addr, &mut Cursor::new(&mut image), 0).unwrap(),
999+
0
1000+
);
1001+
1002+
assert!(mem
1003+
.write_all_to(addr, &mut Cursor::new(&mut image), 0)
1004+
.is_ok());
1005+
}
9651006
}

0 commit comments

Comments
 (0)