Skip to content

Commit f6dc77e

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 0eeb18c commit f6dc77e

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
@@ -540,7 +540,7 @@ pub trait GuestMemory {
540540
let len = std::cmp::min(cap, (count - total) as GuestUsize);
541541
match f(total, len as usize, start, region) {
542542
// no more data
543-
Ok(0) => break,
543+
Ok(0) => return Ok(total),
544544
// made some progress
545545
Ok(len) => {
546546
total += len;
@@ -738,7 +738,7 @@ impl<T: GuestMemory> Bytes<GuestAddress> for T {
738738
{
739739
self.try_access(count, addr, |offset, len, caddr, region| -> Result<usize> {
740740
// Check if something bad happened before doing unsafe things.
741-
assert!(offset < count);
741+
assert!(offset <= count);
742742
if let Some(dst) = unsafe { region.as_mut_slice() } {
743743
// This is safe cause `start` and `len` are within the `region`.
744744
let start = caddr.raw_value() as usize;
@@ -816,7 +816,7 @@ impl<T: GuestMemory> Bytes<GuestAddress> for T {
816816
{
817817
self.try_access(count, addr, |offset, len, caddr, region| -> Result<usize> {
818818
// Check if something bad happened before doing unsafe things.
819-
assert!(offset < count);
819+
assert!(offset <= count);
820820
if let Some(src) = unsafe { region.as_slice() } {
821821
// This is safe cause `start` and `len` are within the `region`.
822822
let start = caddr.raw_value() as usize;
@@ -1030,4 +1030,45 @@ mod tests {
10301030
fn test_non_atomic_access() {
10311031
non_atomic_access_helper::<u16>()
10321032
}
1033+
1034+
#[cfg(feature = "backend-mmap")]
1035+
#[test]
1036+
fn test_zero_length_accesses() {
1037+
#[derive(Default, Clone, Copy)]
1038+
#[repr(C)]
1039+
struct ZeroSizedStruct {
1040+
dummy: [u32; 0],
1041+
}
1042+
1043+
unsafe impl ByteValued for ZeroSizedStruct {}
1044+
1045+
let addr = GuestAddress(0x1000);
1046+
let mem = GuestMemoryMmap::from_ranges(&[(addr, 0x1000)]).unwrap();
1047+
let obj = ZeroSizedStruct::default();
1048+
let mut image = make_image(0x80);
1049+
1050+
assert_eq!(mem.write(&[], addr).unwrap(), 0);
1051+
assert_eq!(mem.read(&mut [], addr).unwrap(), 0);
1052+
1053+
assert!(mem.write_slice(&[], addr).is_ok());
1054+
assert!(mem.read_slice(&mut [], addr).is_ok());
1055+
1056+
assert!(mem.write_obj(obj, addr).is_ok());
1057+
assert!(mem.read_obj::<ZeroSizedStruct>(addr).is_ok());
1058+
1059+
assert_eq!(mem.read_from(addr, &mut Cursor::new(&image), 0).unwrap(), 0);
1060+
1061+
assert!(mem
1062+
.read_exact_from(addr, &mut Cursor::new(&image), 0)
1063+
.is_ok());
1064+
1065+
assert_eq!(
1066+
mem.write_to(addr, &mut Cursor::new(&mut image), 0).unwrap(),
1067+
0
1068+
);
1069+
1070+
assert!(mem
1071+
.write_all_to(addr, &mut Cursor::new(&mut image), 0)
1072+
.is_ok());
1073+
}
10331074
}

0 commit comments

Comments
 (0)