Re: [PATCH 05/12] hw/virtio: Add support for apple virtio-blk

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

On Wed, Jun 14, 2023 at 10:56:22PM +0000, Alexander Graf wrote:
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 39e7f23fab..76b85bb3cb 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -1120,6 +1120,20 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
>  
>          break;
>      }
> +    case VIRTIO_BLK_T_APPLE1:
> +    {
> +        if (s->conf.x_apple_type) {
> +            /* Only valid on Apple Virtio */
> +            char buf[iov_size(in_iov, in_num)];

I'm concerned that a variable-sized stack buffer could be abused by a
malicious guest. Even if it's harmless in the Apple use case, someone
else might copy this approach and use it where it creates a security
problem. Please either implement iov_memset() or allocate the temporary
buffer using bdrv_blockalign() (and free it with qemu_vfree()).

> +            memset(buf, 0, sizeof(buf));
> +            iov_from_buf(in_iov, in_num, 0, buf, sizeof(buf));
> +            virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]