[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/9] virtio-9p: handle handle_9p_output() err
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/9] virtio-9p: handle handle_9p_output() error |
Date: |
Fri, 23 Sep 2016 13:54:17 +0100 |
User-agent: |
Mutt/1.7.0 (2016-08-17) |
On Wed, Sep 21, 2016 at 06:57:12PM +0200, Greg Kurz wrote:
> A broken guest may send a request with only non-empty out buffers
> or only non-empty in buffers, virtqueue_pop() will then return a
> VirtQueueElement with out_num == 0 or in_num == 0 respectively.
>
> All 9P requests are expected to start with the following 7-byte header:
>
> uint32_t size_le;
> uint8_t id;
> uint16_t tag_le;
>
> If iov_to_buf() fails to return these 7 bytes, then something is wrong in
> the guest.
>
> In both cases, it is wrong to crash QEMU, since the root cause lies in the
> guest. Let's switch the device to the broken state instead.
>
> Signed-off-by: Greg Kurz <address@hidden>
> ---
> v2: - added out_free_pdu: label for errors or when virtqueue is empty
> ---
> hw/9pfs/virtio-9p-device.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index e7ea0e45f3dd..5f3a67cfc717 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -52,17 +52,24 @@ static void handle_9p_output(VirtIODevice *vdev,
> VirtQueue *vq)
>
> elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> if (!elem) {
> - pdu_free(pdu);
> - break;
> + goto out_free_pdu;
> }
>
> - BUG_ON(elem->out_num == 0 || elem->in_num == 0);
> + if (elem->out_num == 0 || elem->in_num == 0) {
> + virtio_error(vdev,
> + "The guest sent a VirtFS request without headers");
> + goto out_free_pdu;
> + }
I'm not sure this check is even necessary since we check
iov_to_buf(elem->out_sg[]) and pdu_marshall() avoids overflowing
elem->in_sg[].
Either way, your change is correct:
Reviewed-by: Stefan Hajnoczi <address@hidden>
signature.asc
Description: PGP signature
- [Qemu-devel] [PATCH v2 0/9] virtio: avoid inappropriate QEMU termination, Greg Kurz, 2016/09/21
- [Qemu-devel] [PATCH v2 1/9] virtio-9p: add parentheses to sizeof operator, Greg Kurz, 2016/09/21
- [Qemu-devel] [PATCH v2 2/9] virtio-blk: turn virtio_blk_handle_request() into a static function, Greg Kurz, 2016/09/21
- [Qemu-devel] [PATCH v2 3/9] virtio-9p: handle handle_9p_output() error, Greg Kurz, 2016/09/21
- [Qemu-devel] [PATCH v2 4/9] virtio-blk: handle virtio_blk_handle_request() errors, Greg Kurz, 2016/09/21
- [Qemu-devel] [PATCH v2 5/9] virtio-net: handle virtio_net_handle_ctrl() error, Greg Kurz, 2016/09/21
- [Qemu-devel] [PATCH v2 7/9] virtio-net: handle virtio_net_flush_tx() errors, Greg Kurz, 2016/09/21
- [Qemu-devel] [PATCH v2 8/9] virtio-scsi: convert virtio_scsi_bad_req() to use virtio_error(), Greg Kurz, 2016/09/21