[PATCH] vhost: do not crash on ring map failure

Michael S. Tsirkin posted 1 patch 1 day, 20 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/71961a7dc157f552303aeea8c99a75c5e1ce904e.1784898432.git.mst@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>
hw/virtio/vhost.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] vhost: do not crash on ring map failure
Posted by Michael S. Tsirkin 1 day, 20 hours ago
When vhost_commit() rebuilds the memory region table after a flatview
change, it revalidates cached host virtual addresses for active vring
parts. If a mapping is stale, QEMU abort().

This is not a security problem - only the priviledged guest
can control make it invalid - but not nice e.g. for driver debugging.

Let's call virtio_error() instead, marking the device as broken.

Fixes: 0ca1fd2d68 ("vhost: Simplify ring verification checks")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3783
Cc: Stefano Garzarella <sgarzare@redhat.com>
Cc: Dr. David Alan Gilbert <dave@treblig.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index af41841b52..371dca17dd 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -755,8 +755,9 @@ static void vhost_commit(MemoryListener *listener)
                        (void *)(uintptr_t)dev->mem->regions[i].userspace_addr,
                        dev->mem->regions[i].guest_phys_addr,
                        dev->mem->regions[i].memory_size)) {
-            error_report("Verify ring failure on region %d", i);
-            abort();
+            virtio_error(dev->vdev,
+                         "Verify ring failure on region %d", i);
+            goto out;
         }
     }
 
-- 
MST
Re: [PATCH] vhost: do not crash on ring map failure
Posted by Dr. David Alan Gilbert 12 hours ago
* Michael S. Tsirkin (mst@redhat.com) wrote:
> When vhost_commit() rebuilds the memory region table after a flatview
> change, it revalidates cached host virtual addresses for active vring
> parts. If a mapping is stale, QEMU abort().
> 
> This is not a security problem - only the priviledged guest
> can control make it invalid - but not nice e.g. for driver debugging.
> 
> Let's call virtio_error() instead, marking the device as broken.
> 
> Fixes: 0ca1fd2d68 ("vhost: Simplify ring verification checks")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3783
> Cc: Stefano Garzarella <sgarzare@redhat.com>
> Cc: Dr. David Alan Gilbert <dave@treblig.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>

> ---
>  hw/virtio/vhost.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index af41841b52..371dca17dd 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -755,8 +755,9 @@ static void vhost_commit(MemoryListener *listener)
>                         (void *)(uintptr_t)dev->mem->regions[i].userspace_addr,
>                         dev->mem->regions[i].guest_phys_addr,
>                         dev->mem->regions[i].memory_size)) {
> -            error_report("Verify ring failure on region %d", i);
> -            abort();
> +            virtio_error(dev->vdev,
> +                         "Verify ring failure on region %d", i);
> +            goto out;
>          }
>      }
>  
> -- 
> MST
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/