Skip to content

Commit 45f5bc4

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 dcb9a44 commit 45f5bc4

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;
@@ -804,7 +804,7 @@ impl<T: GuestMemory> Bytes<GuestAddress> for T {
804804
{
805805
self.try_access(count, addr, |offset, len, caddr, region| -> Result<usize> {
806806
// Check if something bad happened before doing unsafe things.
807-
assert!(offset < count);
807+
assert!(offset <= count);
808808
if let Some(src) = unsafe { region.as_slice() } {
809809
// This is safe cause `start` and `len` are within the `region`.
810810
let start = caddr.raw_value() as usize;
@@ -1013,4 +1013,45 @@ mod tests {
10131013
fn test_non_atomic_access() {
10141014
non_atomic_access_helper::<u16>()
10151015
}
1016+
1017+
#[cfg(feature = "backend-mmap")]
1018+
#[test]
1019+
fn test_zero_length_accesses() {
1020+
#[derive(Default, Clone, Copy)]
1021+
#[repr(C)]
1022+
struct ZeroSizedStruct {
1023+
dummy: [u32; 0],
1024+
}
1025+
1026+
unsafe impl ByteValued for ZeroSizedStruct {}
1027+
1028+
let addr = GuestAddress(0x1000);
1029+
let mem = GuestMemoryMmap::from_ranges(&[(addr, 0x1000)]).unwrap();
1030+
let obj = ZeroSizedStruct::default();
1031+
let mut image = make_image(0x80);
1032+
1033+
assert_eq!(mem.write(&[], addr).unwrap(), 0);
1034+
assert_eq!(mem.read(&mut [], addr).unwrap(), 0);
1035+
1036+
assert!(mem.write_slice(&[], addr).is_ok());
1037+
assert!(mem.read_slice(&mut [], addr).is_ok());
1038+
1039+
assert!(mem.write_obj(obj, addr).is_ok());
1040+
assert!(mem.read_obj::<ZeroSizedStruct>(addr).is_ok());
1041+
1042+
assert_eq!(mem.read_from(addr, &mut Cursor::new(&image), 0).unwrap(), 0);
1043+
1044+
assert!(mem
1045+
.read_exact_from(addr, &mut Cursor::new(&image), 0)
1046+
.is_ok());
1047+
1048+
assert_eq!(
1049+
mem.write_to(addr, &mut Cursor::new(&mut image), 0).unwrap(),
1050+
0
1051+
);
1052+
1053+
assert!(mem
1054+
.write_all_to(addr, &mut Cursor::new(&mut image), 0)
1055+
.is_ok());
1056+
}
10161057
}

0 commit comments

Comments
 (0)