Skip to content

Commit f1bb166

Browse files
bonzinijiangliu
authored andcommitted
guest_memory: avoid multiple side-effecting reads in Bytes<GuestAddress>::write_to
Reading from a GuestMemoryRegion could have side effects if the region is not RAM, and in that case we cannot repeat the read. Therefore, write_to should use dst.write_all() in the else branch. Do it, and add comments explaining the difference between the two. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
1 parent 4e29019 commit f1bb166

File tree

1 file changed

+7
-4
lines changed

1 file changed

+7
-4
lines changed

src/guest_memory.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -650,17 +650,20 @@ impl<T: GuestMemory> Bytes<GuestAddress> for T {
650650
let start = caddr.raw_value() as usize;
651651
let end = start + len;
652652
// It is safe to read from volatile memory. Accessing the guest
653-
// memory as a slice is OK because nothing assumes another thread
654-
// won't change what is loaded.
653+
// memory as a slice should be OK as long as nothing assumes another
654+
// thread won't change what is loaded; however, we may want to introduce
655+
// VolatileRead and VolatileWrite traits in the future.
655656
let bytes_written = dst.write(&src[start..end]).map_err(Error::IOError)?;
656657
Ok(bytes_written)
657658
} else {
658659
let len = std::cmp::min(len, MAX_ACCESS_CHUNK);
659660
let mut buf = vec![0u8; len].into_boxed_slice();
660661
let bytes_read = region.read(&mut buf, caddr)?;
661662
assert_eq!(bytes_read, len);
662-
let bytes_written = dst.write(&buf).map_err(Error::IOError)?;
663-
Ok(bytes_written)
663+
// For a non-RAM region, reading could have side effects, so we
664+
// must use write_all().
665+
dst.write_all(&buf).map_err(Error::IOError)?;
666+
Ok(len)
664667
}
665668
})
666669
}

0 commit comments

Comments
 (0)