virtio: queue: Enforce DescriptorChain ordering

The virtio spec requires that all read-only descriptors appear in the
chain before any write-only descriptors.  Enforce this in the
`checked_new` function by adding a new `required_flags` parameter.  The
`next_descriptor` function will set this to `VIRTQ_DESC_F_WRITE` if the
current descriptor is write-only.  This ensures that once we see a
write-only descriptor, all following descriptors must be write-only.

BUG=b:136127316
TEST=none

Change-Id: Id8f942a4236a20f62f35439f3648dbec17e14c00
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1757239
Auto-Submit: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>

2 files changed, 17 insertions, 8 deletions

diff --git a/devices/src/virtio/descriptor_utils.rs b/devices/src/virtio/descriptor_utils.rs
index 2233931..409134e 100644
--- a/devices/src/virtio/descriptor_utils.rs
+++ b/devices/src/virtio/descriptor_utils.rs
@@ -517,7 +517,8 @@ pub fn create_descriptor_chain(
         );
     }
 
-    DescriptorChain::checked_new(memory, descriptor_array_addr, 0x100, 0).ok_or(Error::InvalidChain)
+    DescriptorChain::checked_new(memory, descriptor_array_addr, 0x100, 0, 0)
+        .ok_or(Error::InvalidChain)
 }
 
 #[cfg(test)]
diff --git a/devices/src/virtio/queue.rs b/devices/src/virtio/queue.rs
index ea894a2..32ffa2c 100644
--- a/devices/src/virtio/queue.rs
+++ b/devices/src/virtio/queue.rs
@@ -75,6 +75,7 @@ impl<'a> DescriptorChain<'a> {
         desc_table: GuestAddress,
         queue_size: u16,
         index: u16,
+        required_flags: u16,
     ) -> Option<DescriptorChain> {
         if index >= queue_size {
             return None;
@@ -104,7 +105,7 @@ impl<'a> DescriptorChain<'a> {
             next,
         };
 
-        if chain.is_valid() {
+        if chain.is_valid() && chain.flags & required_flags == required_flags {
             Some(chain)
         } else {
             None
@@ -154,12 +155,19 @@ impl<'a> DescriptorChain<'a> {
     /// the head of the next _available_ descriptor chain.
     pub fn next_descriptor(&self) -> Option<DescriptorChain<'a>> {
         if self.has_next() {
-            DescriptorChain::checked_new(self.mem, self.desc_table, self.queue_size, self.next).map(
-                |mut c| {
-                    c.ttl = self.ttl - 1;
-                    c
-                },
+            // Once we see a write-only descriptor, all subsequent descriptors must be write-only.
+            let required_flags = self.flags & VIRTQ_DESC_F_WRITE;
+            DescriptorChain::checked_new(
+                self.mem,
+                self.desc_table,
+                self.queue_size,
+                self.next,
+                required_flags,
             )
+            .map(|mut c| {
+                c.ttl = self.ttl - 1;
+                c
+            })
         } else {
             None
         }
@@ -303,7 +311,7 @@ impl Queue {
         let descriptor_index: u16 = mem.read_obj_from_addr(desc_idx_addr).unwrap();
 
         let descriptor_chain =
-            DescriptorChain::checked_new(mem, self.desc_table, queue_size, descriptor_index);
+            DescriptorChain::checked_new(mem, self.desc_table, queue_size, descriptor_index, 0);
         if descriptor_chain.is_some() {
             self.next_avail += Wrapping(1);
         }