[Qemu-devel] Re: [PATCH 14/15] virtio-serial: Handle scatter-gather buffers for control messages

[Amit Shah]

On (Tue) Mar 30 2010 [15:44:21], Juan Quintela wrote:
> Amit Shah <address@hidden> wrote:
> > Current control messages are small enough to not be split into multiple
> > buffers but we could run into such a situation in the future or a
> > malicious guest could cause such a situation.
> >
> > So handle the entire iov request for control messages.
> >
> > Also ensure the size of the control request is >= what we expect
> > otherwise we risk accessing memory that we don't own.
> >
> > Signed-off-by: Amit Shah <address@hidden>
> > CC: Avi Kivity <address@hidden>
> > Reported-by: Avi Kivity <address@hidden>
> > ---
> >  hw/virtio-serial-bus.c |   34 +++++++++++++++++++++++++++++++---
> >  1 files changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > index bd1223e..3edfeca 100644
> >      vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
> >  
> > +    len = 0;
> > +    buf = NULL;
> >      while (virtqueue_pop(vq, &elem)) {
> > -        handle_control_message(vser, elem.out_sg[0].iov_base);
> > -        virtqueue_push(vq, &elem, elem.out_sg[0].iov_len);
> > +        size_t cur_len, copied;
> > +
> > +        cur_len = iov_size(elem.out_sg, elem.out_num);
> > +        /*
> > +         * Allocate a new buf only if we didn't have one previously or
> > +         * if the size of the buf differs
> > +         */
> > +        if (cur_len != len) {
> > +            if (len) {
> > +                qemu_free(buf);
> > +            }
> > +            buf = qemu_malloc(cur_len);
> > +            len = cur_len;
> > +        }
> 
> This can be simplified to only allocate the buffer if it is less no?

Currently all the control messages are the same size, sizeof(struct
virtio_console_control), so it wouldn't matter.

But I guess this could be done, just have to ensure we don't leak data
meant for one control message to another.

                Amit