Skip to content

Commit

Permalink
vhost: fix vid allocation race
Browse files Browse the repository at this point in the history
[ upstream commit 9944bdd ]

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: a277c71 ("vhost: refactor code structure")

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>
  • Loading branch information
Fei Chen authored and bluca committed Feb 8, 2021
1 parent b9d6b8b commit 9f014a0
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lib/librte_vhost/vhost.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -653,17 +655,21 @@ 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;
}

dev = rte_zmalloc(NULL, sizeof(struct virtio_net), 0);
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;
Expand Down

0 comments on commit 9f014a0

Please sign in to comment.