Re: [Qemu-devel] [PATCH 1/7] virtio-9p: handle handle_9p_output() error

[Cornelia Huck]

On Wed, 21 Sep 2016 16:38:26 +0200
Greg Kurz <address@hidden> wrote:

> On Wed, 21 Sep 2016 16:16:59 +0200
> Cornelia Huck <address@hidden> wrote:
> 
> > On Wed, 21 Sep 2016 15:14:00 +0200
> > Greg Kurz <address@hidden> 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>
> > > ---
> > >  hw/9pfs/virtio-9p-device.c |   14 ++++++++++++--
> > >  1 file changed, 12 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > > index 009b43f6d045..0f09bef13392 100644
> > > --- a/hw/9pfs/virtio-9p-device.c
> > > +++ b/hw/9pfs/virtio-9p-device.c
> > > @@ -56,13 +56,23 @@ static void handle_9p_output(VirtIODevice *vdev, 
> > > VirtQueue *vq)
> > >              break;
> > >          }
> > > 
> > > -        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");
> > > +            pdu_free(pdu);
> > > +            return;  
> > 
> > Make that 'break;' to be more consistent with the code right above?
> > 
> 
> The code right above isn't an error path, unlike here. Maybe I should
> even add an out_err: label to make it explicit.

I think our tastes differ a bit. But it's your code :)