ANDROID: Verify virtio queue address ranges are valid am: 2cd328fdf1 am: 0e99dd1e32android-13.0.0_r82android-13.0.0_r81android-13.0.0_r80android-13.0.0_r74android-13.0.0_r73android-13.0.0_r72android-13.0.0_r66android-13.0.0_r65android-13.0.0_r64android-13.0.0_r60android-13.0.0_r59android-13.0.0_r58android13-qpr3-c-s8-releaseandroid13-qpr3-c-s7-releaseandroid13-qpr3-c-s6-releaseandroid13-qpr3-c-s5-releaseandroid13-qpr3-c-s4-releaseandroid13-qpr3-c-s3-releaseandroid13-qpr3-c-s2-releaseandroid13-qpr3-c-s12-releaseandroid13-qpr3-c-s11-releaseandroid13-qpr3-c-s10-releaseandroid13-qpr3-c-s1-release

Original change: https://googleplex-android-review.googlesource.com/c/platform/external/crosvm/+/20586865

Change-Id: I16db721be16ce5fcb3ba0c25d068f5d15c7c015c
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

2 files changed, 24 insertions, 6 deletions

diff --git a/devices/src/virtio/queue.rs b/devices/src/virtio/queue.rs
index a436f748f..1e4d8bdfb 100644
--- a/devices/src/virtio/queue.rs
+++ b/devices/src/virtio/queue.rs
@@ -162,11 +162,8 @@ impl DescriptorChain {
         if self.len > 0 {
             match self.get_mem_regions() {
                 Ok(regions) => {
-                    if regions.iter().any(|r| {
-                        self.mem
-                            .checked_offset(r.gpa, r.len as u64 - 1u64)
-                            .is_none()
-                    }) {
+                    // Each region in `self.regions` must be a contiguous range in `self.mem`.
+                    if !regions.iter().all(|r| self.mem.is_valid_range(r.gpa, r.len as u64)) {
                         return false;
                     }
                 }
diff --git a/vm_memory/src/guest_memory.rs b/vm_memory/src/guest_memory.rs
index 47c3ba4fd..7c5b4d407 100644
--- a/vm_memory/src/guest_memory.rs
+++ b/vm_memory/src/guest_memory.rs
@@ -315,7 +315,10 @@ impl GuestMemory {
             .any(|region| region.start() < end && start < region.end())
     }
 
-    /// Returns the address plus the offset if it is in range.
+    /// Returns an address `addr + offset` if it's in range.
+    ///
+    /// This function doesn't care whether a region `[addr, addr + offset)` is in range or not. To
+    /// guarantee it's a valid range, use `is_valid_range()` instead.
     pub fn checked_offset(&self, addr: GuestAddress, offset: u64) -> Option<GuestAddress> {
         addr.checked_add(offset).and_then(|a| {
             if self.address_in_range(a) {
@@ -326,6 +329,24 @@ impl GuestMemory {
         })
     }
 
+    /// Returns true if the given range `[start, start + length)` is a valid contiguous memory
+    /// range available to the guest and it's backed by a single underlying memory region.
+    pub fn is_valid_range(&self, start: GuestAddress, length: u64) -> bool {
+        if length == 0 {
+            return false;
+        }
+
+        let end = if let Some(end) = start.checked_add(length - 1) {
+            end
+        } else {
+            return false;
+        };
+
+        self.regions
+            .iter()
+            .any(|region| region.start() <= start && end < region.end())
+    }
+
     /// Returns the size of the memory region in bytes.
     pub fn num_regions(&self) -> u64 {
         self.regions.len() as u64