[RFC PATCH 12/12] hw/virtio: Display error if vring flag field is not aligned

Philippe Mathieu-Daudé posted 12 patches 4 years, 1 month ago
[RFC PATCH 12/12] hw/virtio: Display error if vring flag field is not aligned
Posted by Philippe Mathieu-Daudé 4 years, 1 month ago
Per the virtio spec [*] the vring is aligned, so the 'flag' field
is also 16-bit aligned. If it is not, this is a guest error, and
we'd rather report any malicious activity.

Enforce the transaction alignment by setting the 'aligned' attribute
and log unaligned addresses.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/virtio/virtio.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 1b8bc194ce1..f19d12fc71e 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -291,15 +291,24 @@ static inline bool vring_avail_flags(VirtQueue *vq, uint16_t *val)
 {
     VRingMemoryRegionCaches *caches = vring_get_region_caches(vq);
     hwaddr pa = offsetof(VRingAvail, flags);
+    MemTxAttrs attrs = { .aligned = 1 };
+    MemTxResult res;
 
     if (!caches) {
         *val = 0;
         return true;
     }
 
-    *val = virtio_lduw_phys_cached_with_attrs(vq->vdev, &caches->avail, pa);
+    *val = virtio_lduw_phys_cached_with_attrs(vq->vdev, &caches->avail,
+                                              pa, attrs, &res);
+    if (res == MEMTX_UNALIGNED_ERROR) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "virtio: vring flag address 0x%" HWADDR_PRIX " "
+                      "is not aligned\n", pa);
+        return false;
+    }
 
-    return true;
+    return res == MEMTX_OK;
 }
 
 /* Called within rcu_read_lock().  */
-- 
2.26.3


Re: [RFC PATCH 12/12] hw/virtio: Display error if vring flag field is not aligned
Posted by Stefan Hajnoczi 4 years ago
On Thu, May 20, 2021 at 01:09:19PM +0200, Philippe Mathieu-Daudé wrote:
>  {
>      VRingMemoryRegionCaches *caches = vring_get_region_caches(vq);
>      hwaddr pa = offsetof(VRingAvail, flags);
> +    MemTxAttrs attrs = { .aligned = 1 };
> +    MemTxResult res;
>  
>      if (!caches) {
>          *val = 0;
>          return true;
>      }
>  
> -    *val = virtio_lduw_phys_cached_with_attrs(vq->vdev, &caches->avail, pa);
> +    *val = virtio_lduw_phys_cached_with_attrs(vq->vdev, &caches->avail,
> +                                              pa, attrs, &res);
> +    if (res == MEMTX_UNALIGNED_ERROR) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "virtio: vring flag address 0x%" HWADDR_PRIX " "
> +                      "is not aligned\n", pa);
> +        return false;
> +    }

Performance-critical code paths could validate the cache and offset
ahead of time to avoid taking the more expensive code path that checks
MemTxAttrs.

The guest driver configures the vring addresses by writing to
virtio-pci/virtio-mmio registers. The alignment check can be performed
at that time (while/before creating the cache).

Stefan