vhost: fix vid allocation race
authorFei Chen <chenwei.0515@bytedance.com>
Mon, 1 Feb 2021 08:48:44 +0000 (16:48 +0800)
committerFerruh Yigit <ferruh.yigit@intel.com>
Thu, 4 Feb 2021 17:19:36 +0000 (18:19 +0100)
vhost_new_device might be called in different threads at
the same time.

thread 1(config thread)
            rte_vhost_driver_start
               ->vhost_user_start_client
                   ->vhost_user_add_connection
                     -> vhost_new_device

thread 2(vhost-events)
vhost_user_read_cb
           ->vhost_user_msg_handler (return value < 0)
             -> vhost_user_start_client
                 -> vhost_new_device

So there could be a case that a same vid has been allocated
twice, or some vid might be lost in DPDK lib however still
held by the upper applications.

Another place where race would happen is at the func
*vhost_destroy_device*, but after a detailed investigation,
the race does not exist as long as no two devices have the
same vid: Calling vhost_destroy_devices in different
threads with different vids is actually safe.

Fixes: a277c7159876 ("vhost: refactor code structure")
Cc: stable@dpdk.org
Reported-by: Peng He <hepeng.0320@bytedance.com>
Signed-off-by: Fei Chen <chenwei.0515@bytedance.com>
Reviewed-by: Zhihong Wang <wangzhihong.wzh@bytedance.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
lib/librte_vhost/vhost.c

index efb136e..52ab93d 100644 (file)
@@ -26,6 +26,7 @@
 #include "vhost_user.h"
 
 struct virtio_net *vhost_devices[MAX_VHOST_DEVICE];
+pthread_mutex_t vhost_dev_lock = PTHREAD_MUTEX_INITIALIZER;
 
 /* Called with iotlb_lock read-locked */
 uint64_t
@@ -645,6 +646,7 @@ vhost_new_device(void)
        struct virtio_net *dev;
        int i;
 
+       pthread_mutex_lock(&vhost_dev_lock);
        for (i = 0; i < MAX_VHOST_DEVICE; i++) {
                if (vhost_devices[i] == NULL)
                        break;
@@ -653,6 +655,7 @@ vhost_new_device(void)
        if (i == MAX_VHOST_DEVICE) {
                VHOST_LOG_CONFIG(ERR,
                        "Failed to find a free slot for new device.\n");
+               pthread_mutex_unlock(&vhost_dev_lock);
                return -1;
        }
 
@@ -660,10 +663,13 @@ vhost_new_device(void)
        if (dev == NULL) {
                VHOST_LOG_CONFIG(ERR,
                        "Failed to allocate memory for new dev.\n");
+               pthread_mutex_unlock(&vhost_dev_lock);
                return -1;
        }
 
        vhost_devices[i] = dev;
+       pthread_mutex_unlock(&vhost_dev_lock);
+
        dev->vid = i;
        dev->flags = VIRTIO_DEV_BUILTIN_VIRTIO_NET;
        dev->slave_req_fd = -1;