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