| Message ID | 20210520110919.2483190-13-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | exec/memory: Experimental API to catch unaligned accesses | expand |
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
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(). */
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(-)