Re: [PATCH] virtio-blk: add missing VIRTIO_BLK_T_SCSI_CMD size check (CVE-2026-48914)

2026-05-29 Thread Kevin Wolf
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  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 
> Cc: Paolo Bonzini 
> Signed-off-by: Stefan Hajnoczi 

Reviewed-by: Kevin Wolf 

> 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




Re: [PATCH] virtio-blk: add missing VIRTIO_BLK_T_SCSI_CMD size check (CVE-2026-48914)

2026-05-28 Thread Paolo Bonzini
Queued, thanks.

Paolo




[PATCH] virtio-blk: add missing VIRTIO_BLK_T_SCSI_CMD size check (CVE-2026-48914)

2026-05-26 Thread Stefan Hajnoczi
Check that the iovec containing struct virtio_scsi_inhdr is large enough
before storing an error value there.

Feifan Qian  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 
Cc: Paolo Bonzini 
Signed-off-by: Stefan Hajnoczi 
---
 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