* [PATCH] vhost: Check for valid vdev in vhost_backend_handle_iotlb_msg
@ 2021-01-29 9:07 Eugenio Pérez
2021-01-29 9:11 ` Eugenio Perez Martin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Eugenio Pérez @ 2021-01-29 9:07 UTC (permalink / raw)
To: qemu-devel; +Cc: maxime.coquelin, Jason Wang, qemu-stable, Michael S. Tsirkin
Not checking this can lead to invalid dev->vdev member access in
vhost_device_iotlb_miss if backend issue an iotlb message in a bad
timing, either maliciously or by a bug.
Reproduced rebooting a guest with testpmd in txonly forward mode.
#0 0x0000559ffff94394 in vhost_device_iotlb_miss (
dev=dev@entry=0x55a0012f6680, iova=10245279744, write=1)
at ../hw/virtio/vhost.c:1013
#1 0x0000559ffff9ac31 in vhost_backend_handle_iotlb_msg (
imsg=0x7ffddcfd32c0, dev=0x55a0012f6680)
at ../hw/virtio/vhost-backend.c:411
#2 vhost_backend_handle_iotlb_msg (dev=dev@entry=0x55a0012f6680,
imsg=imsg@entry=0x7ffddcfd32c0)
at ../hw/virtio/vhost-backend.c:404
#3 0x0000559fffeded7b in slave_read (opaque=0x55a0012f6680)
at ../hw/virtio/vhost-user.c:1464
#4 0x000055a0000c541b in aio_dispatch_handler (
ctx=ctx@entry=0x55a0010a2120, node=0x55a0012d9e00)
at ../util/aio-posix.c:329
Fixes: 6dcdd06e3b ("spec/vhost-user spec: Add IOMMU support")
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
hw/virtio/vhost-backend.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 222bbcc62d..31b33bde37 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -406,6 +406,11 @@ int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
{
int ret = 0;
+ if (unlikely(!dev->vdev)) {
+ error_report("Unexpected IOTLB message when virtio device is stopped");
+ return -EINVAL;
+ }
+
switch (imsg->type) {
case VHOST_IOTLB_MISS:
ret = vhost_device_iotlb_miss(dev, imsg->iova,
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost: Check for valid vdev in vhost_backend_handle_iotlb_msg
2021-01-29 9:07 [PATCH] vhost: Check for valid vdev in vhost_backend_handle_iotlb_msg Eugenio Pérez
@ 2021-01-29 9:11 ` Eugenio Perez Martin
2021-02-01 6:06 ` Jason Wang
2021-02-01 11:59 ` Stefano Garzarella
2 siblings, 0 replies; 6+ messages in thread
From: Eugenio Perez Martin @ 2021-01-29 9:11 UTC (permalink / raw)
To: qemu-level; +Cc: Maxime Coquelin, Michael S. Tsirkin, Jason Wang, qemu-stable
Version 2 of ("vhost-user: Check for iotlb callback in iotlb_miss") [1].
Starting a new series, since the title does not reflect the changes anymore.
Thanks!
[1] https://patchew.org/QEMU/20210127204449.745365-1-eperezma@redhat.com/
On Fri, Jan 29, 2021 at 10:08 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Not checking this can lead to invalid dev->vdev member access in
> vhost_device_iotlb_miss if backend issue an iotlb message in a bad
> timing, either maliciously or by a bug.
>
> Reproduced rebooting a guest with testpmd in txonly forward mode.
> #0 0x0000559ffff94394 in vhost_device_iotlb_miss (
> dev=dev@entry=0x55a0012f6680, iova=10245279744, write=1)
> at ../hw/virtio/vhost.c:1013
> #1 0x0000559ffff9ac31 in vhost_backend_handle_iotlb_msg (
> imsg=0x7ffddcfd32c0, dev=0x55a0012f6680)
> at ../hw/virtio/vhost-backend.c:411
> #2 vhost_backend_handle_iotlb_msg (dev=dev@entry=0x55a0012f6680,
> imsg=imsg@entry=0x7ffddcfd32c0)
> at ../hw/virtio/vhost-backend.c:404
> #3 0x0000559fffeded7b in slave_read (opaque=0x55a0012f6680)
> at ../hw/virtio/vhost-user.c:1464
> #4 0x000055a0000c541b in aio_dispatch_handler (
> ctx=ctx@entry=0x55a0010a2120, node=0x55a0012d9e00)
> at ../util/aio-posix.c:329
>
> Fixes: 6dcdd06e3b ("spec/vhost-user spec: Add IOMMU support")
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
> hw/virtio/vhost-backend.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 222bbcc62d..31b33bde37 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -406,6 +406,11 @@ int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
> {
> int ret = 0;
>
> + if (unlikely(!dev->vdev)) {
> + error_report("Unexpected IOTLB message when virtio device is stopped");
> + return -EINVAL;
> + }
> +
> switch (imsg->type) {
> case VHOST_IOTLB_MISS:
> ret = vhost_device_iotlb_miss(dev, imsg->iova,
> --
> 2.27.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost: Check for valid vdev in vhost_backend_handle_iotlb_msg
2021-01-29 9:07 [PATCH] vhost: Check for valid vdev in vhost_backend_handle_iotlb_msg Eugenio Pérez
2021-01-29 9:11 ` Eugenio Perez Martin
@ 2021-02-01 6:06 ` Jason Wang
2021-02-01 11:59 ` Stefano Garzarella
2 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2021-02-01 6:06 UTC (permalink / raw)
To: Eugenio Pérez, qemu-devel
Cc: maxime.coquelin, qemu-stable, Michael S. Tsirkin
On 2021/1/29 下午5:07, Eugenio Pérez wrote:
> Not checking this can lead to invalid dev->vdev member access in
> vhost_device_iotlb_miss if backend issue an iotlb message in a bad
> timing, either maliciously or by a bug.
>
> Reproduced rebooting a guest with testpmd in txonly forward mode.
> #0 0x0000559ffff94394 in vhost_device_iotlb_miss (
> dev=dev@entry=0x55a0012f6680, iova=10245279744, write=1)
> at ../hw/virtio/vhost.c:1013
> #1 0x0000559ffff9ac31 in vhost_backend_handle_iotlb_msg (
> imsg=0x7ffddcfd32c0, dev=0x55a0012f6680)
> at ../hw/virtio/vhost-backend.c:411
> #2 vhost_backend_handle_iotlb_msg (dev=dev@entry=0x55a0012f6680,
> imsg=imsg@entry=0x7ffddcfd32c0)
> at ../hw/virtio/vhost-backend.c:404
> #3 0x0000559fffeded7b in slave_read (opaque=0x55a0012f6680)
> at ../hw/virtio/vhost-user.c:1464
> #4 0x000055a0000c541b in aio_dispatch_handler (
> ctx=ctx@entry=0x55a0010a2120, node=0x55a0012d9e00)
> at ../util/aio-posix.c:329
>
> Fixes: 6dcdd06e3b ("spec/vhost-user spec: Add IOMMU support")
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
> ---
> hw/virtio/vhost-backend.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 222bbcc62d..31b33bde37 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -406,6 +406,11 @@ int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
> {
> int ret = 0;
>
> + if (unlikely(!dev->vdev)) {
> + error_report("Unexpected IOTLB message when virtio device is stopped");
> + return -EINVAL;
> + }
> +
> switch (imsg->type) {
> case VHOST_IOTLB_MISS:
> ret = vhost_device_iotlb_miss(dev, imsg->iova,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost: Check for valid vdev in vhost_backend_handle_iotlb_msg
2021-01-29 9:07 [PATCH] vhost: Check for valid vdev in vhost_backend_handle_iotlb_msg Eugenio Pérez
2021-01-29 9:11 ` Eugenio Perez Martin
2021-02-01 6:06 ` Jason Wang
@ 2021-02-01 11:59 ` Stefano Garzarella
2021-02-01 13:06 ` Eugenio Perez Martin
2 siblings, 1 reply; 6+ messages in thread
From: Stefano Garzarella @ 2021-02-01 11:59 UTC (permalink / raw)
To: Eugenio Pérez
Cc: Jason Wang, maxime.coquelin, Michael S. Tsirkin, qemu-devel, qemu-stable
On Fri, Jan 29, 2021 at 10:07:28AM +0100, Eugenio Pérez wrote:
>Not checking this can lead to invalid dev->vdev member access in
>vhost_device_iotlb_miss if backend issue an iotlb message in a bad
>timing, either maliciously or by a bug.
>
>Reproduced rebooting a guest with testpmd in txonly forward mode.
> #0 0x0000559ffff94394 in vhost_device_iotlb_miss (
> dev=dev@entry=0x55a0012f6680, iova=10245279744, write=1)
> at ../hw/virtio/vhost.c:1013
> #1 0x0000559ffff9ac31 in vhost_backend_handle_iotlb_msg (
> imsg=0x7ffddcfd32c0, dev=0x55a0012f6680)
> at ../hw/virtio/vhost-backend.c:411
> #2 vhost_backend_handle_iotlb_msg (dev=dev@entry=0x55a0012f6680,
> imsg=imsg@entry=0x7ffddcfd32c0)
> at ../hw/virtio/vhost-backend.c:404
> #3 0x0000559fffeded7b in slave_read (opaque=0x55a0012f6680)
> at ../hw/virtio/vhost-user.c:1464
> #4 0x000055a0000c541b in aio_dispatch_handler (
> ctx=ctx@entry=0x55a0010a2120, node=0x55a0012d9e00)
> at ../util/aio-posix.c:329
>
>Fixes: 6dcdd06e3b ("spec/vhost-user spec: Add IOMMU support")
I'm not sure but IIUC vhost_backend_handle_iotlb_msg() was introduced by
commit 020e571b8b, so maybe is better this 'Fixes' line:
Fixes: 020e571b8b ("vhost: rework IOTLB messaging")
>Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>---
> hw/virtio/vhost-backend.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
>index 222bbcc62d..31b33bde37 100644
>--- a/hw/virtio/vhost-backend.c
>+++ b/hw/virtio/vhost-backend.c
>@@ -406,6 +406,11 @@ int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
> {
> int ret = 0;
>
>+ if (unlikely(!dev->vdev)) {
>+ error_report("Unexpected IOTLB message when virtio device is stopped");
>+ return -EINVAL;
>+ }
>+
> switch (imsg->type) {
> case VHOST_IOTLB_MISS:
> ret = vhost_device_iotlb_miss(dev, imsg->iova,
>--
>2.27.0
>
>
The patch LGTM:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost: Check for valid vdev in vhost_backend_handle_iotlb_msg
2021-02-01 11:59 ` Stefano Garzarella
@ 2021-02-01 13:06 ` Eugenio Perez Martin
2021-02-01 13:58 ` Stefano Garzarella
0 siblings, 1 reply; 6+ messages in thread
From: Eugenio Perez Martin @ 2021-02-01 13:06 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Jason Wang, Maxime Coquelin, Michael S. Tsirkin, qemu-level, qemu-stable
On Mon, Feb 1, 2021 at 1:00 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Fri, Jan 29, 2021 at 10:07:28AM +0100, Eugenio Pérez wrote:
> >Not checking this can lead to invalid dev->vdev member access in
> >vhost_device_iotlb_miss if backend issue an iotlb message in a bad
> >timing, either maliciously or by a bug.
> >
> >Reproduced rebooting a guest with testpmd in txonly forward mode.
> > #0 0x0000559ffff94394 in vhost_device_iotlb_miss (
> > dev=dev@entry=0x55a0012f6680, iova=10245279744, write=1)
> > at ../hw/virtio/vhost.c:1013
> > #1 0x0000559ffff9ac31 in vhost_backend_handle_iotlb_msg (
> > imsg=0x7ffddcfd32c0, dev=0x55a0012f6680)
> > at ../hw/virtio/vhost-backend.c:411
> > #2 vhost_backend_handle_iotlb_msg (dev=dev@entry=0x55a0012f6680,
> > imsg=imsg@entry=0x7ffddcfd32c0)
> > at ../hw/virtio/vhost-backend.c:404
> > #3 0x0000559fffeded7b in slave_read (opaque=0x55a0012f6680)
> > at ../hw/virtio/vhost-user.c:1464
> > #4 0x000055a0000c541b in aio_dispatch_handler (
> > ctx=ctx@entry=0x55a0010a2120, node=0x55a0012d9e00)
> > at ../util/aio-posix.c:329
> >
> >Fixes: 6dcdd06e3b ("spec/vhost-user spec: Add IOMMU support")
>
> I'm not sure but IIUC vhost_backend_handle_iotlb_msg() was introduced by
> commit 020e571b8b, so maybe is better this 'Fixes' line:
>
> Fixes: 020e571b8b ("vhost: rework IOTLB messaging")
>
Hi Stefano.
Thanks for reviewing it :). Actually yes, you are right, I carried the
previous Fixes line by mistake.
Should I send a new patch?
Thanks!
> >Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> >---
> > hw/virtio/vhost-backend.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> >diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> >index 222bbcc62d..31b33bde37 100644
> >--- a/hw/virtio/vhost-backend.c
> >+++ b/hw/virtio/vhost-backend.c
> >@@ -406,6 +406,11 @@ int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
> > {
> > int ret = 0;
> >
> >+ if (unlikely(!dev->vdev)) {
> >+ error_report("Unexpected IOTLB message when virtio device is stopped");
> >+ return -EINVAL;
> >+ }
> >+
> > switch (imsg->type) {
> > case VHOST_IOTLB_MISS:
> > ret = vhost_device_iotlb_miss(dev, imsg->iova,
> >--
> >2.27.0
> >
> >
>
> The patch LGTM:
>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vhost: Check for valid vdev in vhost_backend_handle_iotlb_msg
2021-02-01 13:06 ` Eugenio Perez Martin
@ 2021-02-01 13:58 ` Stefano Garzarella
0 siblings, 0 replies; 6+ messages in thread
From: Stefano Garzarella @ 2021-02-01 13:58 UTC (permalink / raw)
To: Eugenio Perez Martin
Cc: Jason Wang, Maxime Coquelin, Michael S. Tsirkin, qemu-level, qemu-stable
On Mon, Feb 01, 2021 at 02:06:53PM +0100, Eugenio Perez Martin wrote:
>On Mon, Feb 1, 2021 at 1:00 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>>
>> On Fri, Jan 29, 2021 at 10:07:28AM +0100, Eugenio Pérez wrote:
>> >Not checking this can lead to invalid dev->vdev member access in
>> >vhost_device_iotlb_miss if backend issue an iotlb message in a bad
>> >timing, either maliciously or by a bug.
>> >
>> >Reproduced rebooting a guest with testpmd in txonly forward mode.
>> > #0 0x0000559ffff94394 in vhost_device_iotlb_miss (
>> > dev=dev@entry=0x55a0012f6680, iova=10245279744, write=1)
>> > at ../hw/virtio/vhost.c:1013
>> > #1 0x0000559ffff9ac31 in vhost_backend_handle_iotlb_msg (
>> > imsg=0x7ffddcfd32c0, dev=0x55a0012f6680)
>> > at ../hw/virtio/vhost-backend.c:411
>> > #2 vhost_backend_handle_iotlb_msg (dev=dev@entry=0x55a0012f6680,
>> > imsg=imsg@entry=0x7ffddcfd32c0)
>> > at ../hw/virtio/vhost-backend.c:404
>> > #3 0x0000559fffeded7b in slave_read (opaque=0x55a0012f6680)
>> > at ../hw/virtio/vhost-user.c:1464
>> > #4 0x000055a0000c541b in aio_dispatch_handler (
>> > ctx=ctx@entry=0x55a0010a2120, node=0x55a0012d9e00)
>> > at ../util/aio-posix.c:329
>> >
>> >Fixes: 6dcdd06e3b ("spec/vhost-user spec: Add IOMMU support")
>>
>> I'm not sure but IIUC vhost_backend_handle_iotlb_msg() was introduced by
>> commit 020e571b8b, so maybe is better this 'Fixes' line:
>>
>> Fixes: 020e571b8b ("vhost: rework IOTLB messaging")
>>
>
>Hi Stefano.
>
>Thanks for reviewing it :). Actually yes, you are right, I carried the
>previous Fixes line by mistake.
>
>Should I send a new patch?
Maybe Michael can fix it while applying, but I'm not sure, let's see
what he says :-)
Stefano
>
>Thanks!
>
>
>> >Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>> >---
>> > hw/virtio/vhost-backend.c | 5 +++++
>> > 1 file changed, 5 insertions(+)
>> >
>> >diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
>> >index 222bbcc62d..31b33bde37 100644
>> >--- a/hw/virtio/vhost-backend.c
>> >+++ b/hw/virtio/vhost-backend.c
>> >@@ -406,6 +406,11 @@ int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
>> > {
>> > int ret = 0;
>> >
>> >+ if (unlikely(!dev->vdev)) {
>> >+ error_report("Unexpected IOTLB message when virtio device is stopped");
>> >+ return -EINVAL;
>> >+ }
>> >+
>> > switch (imsg->type) {
>> > case VHOST_IOTLB_MISS:
>> > ret = vhost_device_iotlb_miss(dev, imsg->iova,
>> >--
>> >2.27.0
>> >
>> >
>>
>> The patch LGTM:
>>
>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-01 13:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-29 9:07 [PATCH] vhost: Check for valid vdev in vhost_backend_handle_iotlb_msg Eugenio Pérez
2021-01-29 9:11 ` Eugenio Perez Martin
2021-02-01 6:06 ` Jason Wang
2021-02-01 11:59 ` Stefano Garzarella
2021-02-01 13:06 ` Eugenio Perez Martin
2021-02-01 13:58 ` Stefano Garzarella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).