All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] virtio-blk: add missing VIRTIO_BLK_T_SCSI_CMD size check (CVE-2026-48914)
@ 2026-05-26 15:49 Stefan Hajnoczi
  2026-05-28 16:36 ` Paolo Bonzini
  2026-05-29 14:02 ` Kevin Wolf
  0 siblings, 2 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2026-05-26 15:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, qemu-block, qemu-stable, Michael S. Tsirkin,
	Paolo Bonzini, Hanna Reitz, Kevin Wolf, Feifan Qian

Check that the iovec containing struct virtio_scsi_inhdr is large enough
before storing an error value there.

Feifan Qian <bea1e@proton.me> pointed out that this can be used to
corrupt heap memory when the descriptor uses an MMIO address and a
length of 1, forcing QEMU to allocate a 1-byte heap bounce buffer.
virtio_stl_p() stores 4 bytes and therefore corrupts whatever is beyond
the bounce buffer.

Fixes: CVE-2026-48914
Fixes: f34e73cd69bd ("virtio-blk: report non-zero status when failing SG_IO requests")
Reported-by: Feifan Qian <bea1e@proton.me>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/virtio-blk.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 9cb9f1fb2b..6b92066aff 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -199,10 +199,16 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
 
     /*
      * The scsi inhdr is placed in the second-to-last input segment, just
-     * before the regular inhdr.
+     * before the regular inhdr. VIRTIO implementations normally do not rely on
+     * the precise message framing, but legacy implementations did and so we do
+     * too for the legacy virtio-blk SCSI request type.
      *
      * Just put anything nonzero so that the ioctl fails in the guest.
      */
+    if (elem->in_sg[elem->in_num - 2].iov_len != sizeof(*scsi)) {
+        status = VIRTIO_BLK_S_IOERR;
+        goto fail;
+    }
     scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base;
     virtio_stl_p(vdev, &scsi->errors, 255);
     status = VIRTIO_BLK_S_UNSUPP;
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] virtio-blk: add missing VIRTIO_BLK_T_SCSI_CMD size check (CVE-2026-48914)
  2026-05-26 15:49 [PATCH] virtio-blk: add missing VIRTIO_BLK_T_SCSI_CMD size check (CVE-2026-48914) Stefan Hajnoczi
@ 2026-05-28 16:36 ` Paolo Bonzini
  2026-05-29 14:02 ` Kevin Wolf
  1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2026-05-28 16:36 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, qemu-block, qemu-stable, Michael S . Tsirkin,
	Hanna Reitz, Kevin Wolf, Feifan Qian

Queued, thanks.

Paolo



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] virtio-blk: add missing VIRTIO_BLK_T_SCSI_CMD size check (CVE-2026-48914)
  2026-05-26 15:49 [PATCH] virtio-blk: add missing VIRTIO_BLK_T_SCSI_CMD size check (CVE-2026-48914) Stefan Hajnoczi
  2026-05-28 16:36 ` Paolo Bonzini
@ 2026-05-29 14:02 ` Kevin Wolf
  1 sibling, 0 replies; 3+ messages in thread
From: Kevin Wolf @ 2026-05-29 14:02 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, qemu-block, qemu-stable, Michael S. Tsirkin,
	Paolo Bonzini, Hanna Reitz, Feifan Qian

Am 26.05.2026 um 17:49 hat Stefan Hajnoczi geschrieben:
> Check that the iovec containing struct virtio_scsi_inhdr is large enough
> before storing an error value there.
> 
> Feifan Qian <bea1e@proton.me> pointed out that this can be used to
> corrupt heap memory when the descriptor uses an MMIO address and a
> length of 1, forcing QEMU to allocate a 1-byte heap bounce buffer.
> virtio_stl_p() stores 4 bytes and therefore corrupts whatever is beyond
> the bounce buffer.
> 
> Fixes: CVE-2026-48914
> Fixes: f34e73cd69bd ("virtio-blk: report non-zero status when failing SG_IO requests")
> Reported-by: Feifan Qian <bea1e@proton.me>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 9cb9f1fb2b..6b92066aff 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -199,10 +199,16 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
>  
>      /*
>       * The scsi inhdr is placed in the second-to-last input segment, just
> -     * before the regular inhdr.
> +     * before the regular inhdr. VIRTIO implementations normally do not rely on
> +     * the precise message framing, but legacy implementations did and so we do
> +     * too for the legacy virtio-blk SCSI request type.
>       *
>       * Just put anything nonzero so that the ioctl fails in the guest.
>       */
> +    if (elem->in_sg[elem->in_num - 2].iov_len != sizeof(*scsi)) {
> +        status = VIRTIO_BLK_S_IOERR;
> +        goto fail;
> +    }
>      scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base;
>      virtio_stl_p(vdev, &scsi->errors, 255);
>      status = VIRTIO_BLK_S_UNSUPP;

What would the guest do if we didn't update scsi->errors, but just
return VIRTIO_BLK_S_UNSUPP (i.e. remove this whole function)? Shouldn't
that result in an error, too?

Kevin



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-29 14:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-26 15:49 [PATCH] virtio-blk: add missing VIRTIO_BLK_T_SCSI_CMD size check (CVE-2026-48914) Stefan Hajnoczi
2026-05-28 16:36 ` Paolo Bonzini
2026-05-29 14:02 ` Kevin Wolf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.