When hotplug and hotunplug vhost-user-net device quickly.
qemu will crash. BT is as below:

0  __pthread_kill_implementation () at /usr/lib64/libc.so.6
1  raise () at /usr/lib64/libc.so.6
2  abort () at /usr/lib64/libc.so.6
3  try_dequeue () at ../util/rcu.c:235
4  call_rcu_thread (opaque=opaque@entry=0x0) at ../util/rcu.c:288
5  qemu_thread_start (args=0x55b10d9ceaa0) at ../util/qemu-thread-posix.c:541
6  start_thread () at /usr/lib64/libc.so.6
7  clone3 () at /usr/lib64/libc.so.6

1. device_del qmp process

virtio_set_status
  vhost_dev_stop
    vhost_user_get_vring_base
      vhost_user_host_notifier_remove

vhost_user_slave_handle_vring_host_notifier maybe called asynchronous after
vhost_user_host_notifier_remove. vhost_user_host_notifier_remove will not
all call_rcu because of notifier->addr is NULL at this time.

2. netdev_del qmp process

vhost_user_cleanup
       vhost_user_host_notifier_remove
       g_free_rcu

vhost_user_host_notifier_remove and g_free_rcu will sumbit same rcu_head
to rcu node list. rcu_call_count add twice but only one node is added.
rcu thread will abort when calling try_dequeue with node list is empty.
Fix this by moving g_free(n) to vhost_user_host_notifier_free.

Fixes: 503e355465 ("virtio/vhost-user: dynamically assign 
VhostUserHostNotifiers")
Signed-off-by: yaozhenguo <yaozhen...@jd.com>
---
 hw/virtio/vhost-user.c         | 23 +++++++++++------------
 include/hw/virtio/vhost-user.h |  1 +
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 00561daa06..7ab37c0da2 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1188,6 +1188,12 @@ static void 
vhost_user_host_notifier_free(VhostUserHostNotifier *n)
     assert(n && n->unmap_addr);
     munmap(n->unmap_addr, qemu_real_host_page_size());
     n->unmap_addr = NULL;
+    if (n->need_free) {
+        memory_region_transaction_begin();
+        object_unparent(OBJECT(&n->mr));
+        memory_region_transaction_commit();
+        g_free(n);
+    }
 }
 
 /*
@@ -1195,7 +1201,7 @@ static void 
vhost_user_host_notifier_free(VhostUserHostNotifier *n)
  * under rcu.
  */
 static void vhost_user_host_notifier_remove(VhostUserHostNotifier *n,
-                                            VirtIODevice *vdev)
+                                            VirtIODevice *vdev, bool free)
 {
     if (n->addr) {
         if (vdev) {
@@ -1204,6 +1210,7 @@ static void 
vhost_user_host_notifier_remove(VhostUserHostNotifier *n,
         assert(!n->unmap_addr);
         n->unmap_addr = n->addr;
         n->addr = NULL;
+        n->need_free = free;
         call_rcu(n, vhost_user_host_notifier_free, rcu);
     }
 }
@@ -1280,7 +1287,7 @@ static int vhost_user_get_vring_base(struct vhost_dev 
*dev,
 
     VhostUserHostNotifier *n = fetch_notifier(u->user, ring->index);
     if (n) {
-        vhost_user_host_notifier_remove(n, dev->vdev);
+        vhost_user_host_notifier_remove(n, dev->vdev, false);
     }
 
     ret = vhost_user_write(dev, &msg, NULL, 0);
@@ -1562,7 +1569,7 @@ static int 
vhost_user_backend_handle_vring_host_notifier(struct vhost_dev *dev,
      * new mapped address.
      */
     n = fetch_or_create_notifier(user, queue_idx);
-    vhost_user_host_notifier_remove(n, vdev);
+    vhost_user_host_notifier_remove(n, vdev, false);
 
     if (area->u64 & VHOST_USER_VRING_NOFD_MASK) {
         return 0;
@@ -2737,13 +2744,7 @@ static void vhost_user_state_destroy(gpointer data)
 {
     VhostUserHostNotifier *n = (VhostUserHostNotifier *) data;
     if (n) {
-        vhost_user_host_notifier_remove(n, NULL);
-        object_unparent(OBJECT(&n->mr));
-        /*
-         * We can't free until vhost_user_host_notifier_remove has
-         * done it's thing so schedule the free with RCU.
-         */
-        g_free_rcu(n, rcu);
+        vhost_user_host_notifier_remove(n, NULL, true);
     }
 }
 
@@ -2765,9 +2766,7 @@ void vhost_user_cleanup(VhostUserState *user)
     if (!user->chr) {
         return;
     }
-    memory_region_transaction_begin();
     user->notifiers = (GPtrArray *) g_ptr_array_free(user->notifiers, true);
-    memory_region_transaction_commit();
     user->chr = NULL;
 }
 
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index 324cd8663a..a171f29e0b 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -54,6 +54,7 @@ typedef struct VhostUserHostNotifier {
     void *addr;
     void *unmap_addr;
     int idx;
+    bool need_free;
 } VhostUserHostNotifier;
 
 /**
-- 
2.43.0


Reply via email to