On Tue, Jul 21, 2026 at 10:33 AM Philippe Mathieu-Daudé
<[email protected]> wrote:
>
> On 21/7/26 16:26, Philippe Mathieu-Daudé wrote:
> > On 21/7/26 15:44, Stefan Hajnoczi wrote:
> >> Virtqueue handler functions in device emulation code often look
> >> something like this:
> >>
> >> while (!virtio_queue_empty(vq)) {
> >> ...pop and process virtqueue element...
> >> }
> >>
> >> virtio-blk, virtio-scsi, virtio-crypto, and vhost-shadow-virtqueue use
> >> this pattern.
> >>
> >> The device may break (i.e. hit an error that requires device reset)
> >> during the loop. virtio_queue_empty() returns 1 for broken split vrings
> >> but not for broken packed vrings, leading to an infinite loop.
> >>
> >> Adjust the packed vring behavior to match split vrings and avoid
> >> infinite loops.
> >>
> >> Fixes: CVE-2026-16457
> >> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3968
> >> Reported-by: Anatol Belski <[email protected]>
> >> Signed-off-by: Stefan Hajnoczi <[email protected]>
> >> ---
> >> hw/virtio/virtio.c | 4 ++++
> >> 1 file changed, 4 insertions(+)
> >>
> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >> index f4d86a36553..3795e3c8c68 100644
> >> --- a/hw/virtio/virtio.c
> >> +++ b/hw/virtio/virtio.c
> >> @@ -763,6 +763,10 @@ static int
> >> virtio_queue_packed_empty_rcu(VirtQueue *vq)
> >> struct VRingPackedDesc desc;
> >> VRingMemoryRegionCaches *cache;
> >> + if (virtio_device_disabled(vq->vdev)) {
> >> + return 1;
> >> + }
> >
> > Correct, so:
> > Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
> >
> > But why not run this check in virtio_queue_empty() entry?
>
> To be clear:
>
> -- >8 --
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index f4d86a36553..019c9c9e293 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -786,6 +786,9 @@ static int virtio_queue_packed_empty(VirtQueue *vq)
>
> int virtio_queue_empty(VirtQueue *vq)
> {
> + if (virtio_device_disabled(vq->vdev)) {
> + return 1;
> + }
The _rcu() version of the functions still needs to check this as well,
so I don't think it can be extracted cleanly.
Stefan
> if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> return virtio_queue_packed_empty(vq);
> } else {
> ---
>
> Having:
>
> static inline bool virtio_device_disabled(VirtIODevice *vdev)
> {
> return unlikely(vdev->disabled || vdev->broken);
> }
>
> that would avoid the RCU_READ_LOCK access.
>
