| Message ID | 1423710317-15832-1-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 12 Feb 2015 11:05:17 +0800 Jason Wang <jasowang@redhat.com> wrote: > We don't validate the existence of handle_output which may let a buggy > guest to trigger a SIGSEV easily. Fix this by validate its existence > before. > > Cc: qemu-stable@nongnu.org > Cc: Anthony Liguori <aliguori@amazon.com> > Cc: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/virtio/virtio.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index d735343..ffc22e8 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -761,6 +761,10 @@ void virtio_queue_notify_vq(VirtQueue *vq) > { > if (vq->vring.desc) { > VirtIODevice *vdev = vq->vdev; > + > + if (!vq->handle_output) { > + return; > + } Maybe better to just change line 762 to: if (vq->vring.desc && vq->handle_output) { -d > trace_virtio_queue_notify(vdev, vq - vdev->vq, vq); > vq->handle_output(vdev, vq); > } > -- > 1.9.1 > >
On 12/02/2015 04:05, Jason Wang wrote: > We don't validate the existence of handle_output which may let a buggy > guest to trigger a SIGSEV easily. Fix this by validate its existence > before. > > Cc: qemu-stable@nongnu.org > Cc: Anthony Liguori <aliguori@amazon.com> > Cc: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Jason Wang <jasowang@redhat.com> Which queue was causing this? Paolo
On Sat, Feb 14, 2015 at 4:18 AM, Don Koch <dkoch@verizon.com> wrote: > On Thu, 12 Feb 2015 11:05:17 +0800 > Jason Wang <jasowang@redhat.com> wrote: > >> We don't validate the existence of handle_output which may let a >> buggy >> guest to trigger a SIGSEV easily. Fix this by validate its existence >> before. >> >> Cc: qemu-stable@nongnu.org >> Cc: Anthony Liguori <aliguori@amazon.com> >> Cc: Michael S. Tsirkin <mst@redhat.com> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> hw/virtio/virtio.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >> index d735343..ffc22e8 100644 >> --- a/hw/virtio/virtio.c >> +++ b/hw/virtio/virtio.c >> @@ -761,6 +761,10 @@ void virtio_queue_notify_vq(VirtQueue *vq) >> { >> if (vq->vring.desc) { >> VirtIODevice *vdev = vq->vdev; >> + >> + if (!vq->handle_output) { >> + return; >> + } > > Maybe better to just change line 762 to: > if (vq->vring.desc && vq->handle_output) { > > -d Yes, better. > > >> trace_virtio_queue_notify(vdev, vq - vdev->vq, vq); >> vq->handle_output(vdev, vq); >> } >> -- >> 1.9.1 >> >>
On Sat, Feb 14, 2015 at 4:50 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 12/02/2015 04:05, Jason Wang wrote: >> We don't validate the existence of handle_output which may let a >> buggy >> guest to trigger a SIGSEV easily. Fix this by validate its existence >> before. >> >> Cc: qemu-stable@nongnu.org >> Cc: Anthony Liguori <aliguori@amazon.com> >> Cc: Michael S. Tsirkin <mst@redhat.com> >> Signed-off-by: Jason Wang <jasowang@redhat.com> > > Which queue was causing this? > > Paolo The queue that was not used by the device. Though qemu does not use them, but it allows guest to do some basic programming. e.g: (for 1q virtio-net) 1) write 10 to queue_sel 2) setup an arbitrary pfn 3) then notify queue 10
On 15/02/2015 03:35, Jason Wang wrote: > > > On Sat, Feb 14, 2015 at 4:50 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> >> On 12/02/2015 04:05, Jason Wang wrote: >>> We don't validate the existence of handle_output which may let a buggy >>> guest to trigger a SIGSEV easily. Fix this by validate its existence >>> before. >>> >>> Cc: qemu-stable@nongnu.org >>> Cc: Anthony Liguori <aliguori@amazon.com> >>> Cc: Michael S. Tsirkin <mst@redhat.com> >>> Signed-off-by: Jason Wang <jasowang@redhat.com> >> >> Which queue was causing this? >> >> Paolo > > The queue that was not used by the device. Though qemu does not use > them, but it allows guest to do some basic programming. e.g: (for 1q > virtio-net) > > 1) write 10 to queue_sel > 2) setup an arbitrary pfn > 3) then notify queue 10 Oh, I see. Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index d735343..ffc22e8 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -761,6 +761,10 @@ void virtio_queue_notify_vq(VirtQueue *vq) { if (vq->vring.desc) { VirtIODevice *vdev = vq->vdev; + + if (!vq->handle_output) { + return; + } trace_virtio_queue_notify(vdev, vq - vdev->vq, vq); vq->handle_output(vdev, vq); }
We don't validate the existence of handle_output which may let a buggy guest to trigger a SIGSEV easily. Fix this by validate its existence before. Cc: qemu-stable@nongnu.org Cc: Anthony Liguori <aliguori@amazon.com> Cc: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/virtio/virtio.c | 4 ++++ 1 file changed, 4 insertions(+)