From 74d423fb043baa85c98afa1ea74a156e53e2cfd7 Mon Sep 17 00:00:00 2001
From: Daniel Verkamp <dverkamp@chromium.org>
Date: Wed, 29 Mar 2023 16:43:26 -0700
Subject: [PATCH] virtio: iommu: fix inside-out Option references

Change-Id: I4a8ee44f07491825ea346168fa888ba69774b559
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4391796
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
---
 devices/src/virtio/iommu/memory_util.rs |  4 +--
 devices/src/virtio/queue.rs             | 39 +++++++++++++++++--------
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/devices/src/virtio/iommu/memory_util.rs b/devices/src/virtio/iommu/memory_util.rs
index 3c0542d440..9bb0c6ace6 100644
--- a/devices/src/virtio/iommu/memory_util.rs
+++ b/devices/src/virtio/iommu/memory_util.rs
@@ -15,7 +15,7 @@ use crate::virtio::iommu::ExportedRegion;
 /// A wrapper that works with gpa, or iova and an iommu.
 pub fn read_obj_from_addr_wrapper<T: FromBytes>(
     mem: &GuestMemory,
-    exported_region: &Option<ExportedRegion>,
+    exported_region: Option<&ExportedRegion>,
     addr: GuestAddress,
 ) -> anyhow::Result<T> {
     if let Some(exported_region) = exported_region {
@@ -29,7 +29,7 @@ pub fn read_obj_from_addr_wrapper<T: FromBytes>(
 /// A wrapper that works with gpa, or iova and an iommu.
 pub fn write_obj_at_addr_wrapper<T: FromBytes + AsBytes>(
     mem: &GuestMemory,
-    exported_region: &Option<ExportedRegion>,
+    exported_region: Option<&ExportedRegion>,
     val: T,
     addr: GuestAddress,
 ) -> anyhow::Result<()> {
diff --git a/devices/src/virtio/queue.rs b/devices/src/virtio/queue.rs
index b9254e41fc..8168f71d3e 100644
--- a/devices/src/virtio/queue.rs
+++ b/devices/src/virtio/queue.rs
@@ -142,8 +142,9 @@ impl DescriptorChain {
         let desc_head = desc_table
             .checked_add((index as u64) * 16)
             .context("integer overflow")?;
-        let desc: Desc = read_obj_from_addr_wrapper(mem, &exported_desc_table, desc_head)
-            .with_context(|| format!("failed to read desc {:x}", desc_head.offset()))?;
+        let desc: Desc =
+            read_obj_from_addr_wrapper(mem, exported_desc_table.as_ref(), desc_head)
+                .with_context(|| format!("failed to read desc {:x}", desc_head.offset()))?;
 
         let addr = GuestAddress(desc.addr.into());
         let len = desc.len.to_native();
@@ -573,7 +574,8 @@ impl Queue {
 
         let avail_index_addr = self.avail_ring.unchecked_add(2);
         let avail_index: u16 =
-            read_obj_from_addr_wrapper(mem, &self.exported_avail_ring, avail_index_addr).unwrap();
+            read_obj_from_addr_wrapper(mem, self.exported_avail_ring.as_ref(), avail_index_addr)
+                .unwrap();
 
         Wrapping(avail_index)
     }
@@ -590,7 +592,7 @@ impl Queue {
         let avail_event_addr = self.used_ring.unchecked_add(4 + 8 * u64::from(self.size));
         write_obj_at_addr_wrapper(
             mem,
-            &self.exported_used_ring,
+            self.exported_used_ring.as_ref(),
             avail_index.0,
             avail_event_addr,
         )
@@ -604,7 +606,8 @@ impl Queue {
         fence(Ordering::SeqCst);
 
         let avail_flags: u16 =
-            read_obj_from_addr_wrapper(mem, &self.exported_avail_ring, self.avail_ring).unwrap();
+            read_obj_from_addr_wrapper(mem, self.exported_avail_ring.as_ref(), self.avail_ring)
+                .unwrap();
 
         avail_flags & flag == flag
     }
@@ -621,7 +624,8 @@ impl Queue {
 
         let used_event_addr = self.avail_ring.unchecked_add(4 + 2 * u64::from(self.size));
         let used_event: u16 =
-            read_obj_from_addr_wrapper(mem, &self.exported_avail_ring, used_event_addr).unwrap();
+            read_obj_from_addr_wrapper(mem, self.exported_avail_ring.as_ref(), used_event_addr)
+                .unwrap();
 
         Wrapping(used_event)
     }
@@ -634,8 +638,13 @@ impl Queue {
         fence(Ordering::SeqCst);
 
         let used_index_addr = self.used_ring.unchecked_add(2);
-        write_obj_at_addr_wrapper(mem, &self.exported_used_ring, used_index.0, used_index_addr)
-            .unwrap();
+        write_obj_at_addr_wrapper(
+            mem,
+            self.exported_used_ring.as_ref(),
+            used_index.0,
+            used_index_addr,
+        )
+        .unwrap();
     }
 
     /// Get the first available descriptor chain without removing it from the queue.
@@ -661,7 +670,8 @@ impl Queue {
 
         // This index is checked below in checked_new.
         let descriptor_index: u16 =
-            read_obj_from_addr_wrapper(mem, &self.exported_avail_ring, desc_idx_addr).unwrap();
+            read_obj_from_addr_wrapper(mem, self.exported_avail_ring.as_ref(), desc_idx_addr)
+                .unwrap();
 
         let iommu = self.iommu.as_ref().map(Arc::clone);
         DescriptorChain::checked_new(
@@ -734,11 +744,16 @@ impl Queue {
         let used_elem = used_ring.unchecked_add((4 + next_used * 8) as u64);
 
         // These writes can't fail as we are guaranteed to be within the descriptor ring.
-        write_obj_at_addr_wrapper(mem, &self.exported_used_ring, desc_index as u32, used_elem)
-            .unwrap();
         write_obj_at_addr_wrapper(
             mem,
-            &self.exported_used_ring,
+            self.exported_used_ring.as_ref(),
+            desc_index as u32,
+            used_elem,
+        )
+        .unwrap();
+        write_obj_at_addr_wrapper(
+            mem,
+            self.exported_used_ring.as_ref(),
             len,
             used_elem.unchecked_add(4),
         )