Skip to content

Commit

Permalink
vhost-user: fix VirtQ notifier cleanup
Browse files Browse the repository at this point in the history
When vhost-user device cleanup, remove notifier MR and munmaps notifier
address in the event-handling thread, VM CPU thread writing the notifier
in concurrent fails with an error of accessing invalid address. It
happens because MR is still being referenced and accessed in another
thread while the underlying notifier mmap address is being freed and
becomes invalid.

This patch calls RCU and munmap notifiers in the callback after the
memory flatview update finish.

Fixes: 4486652 ("vhost-user: support registering external host notifiers")
Cc: qemu-stable@nongnu.org
Signed-off-by: Xueming Li <xuemingl@nvidia.com>
Message-Id: <20220207071929.527149-3-xuemingl@nvidia.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
  • Loading branch information
steevenlee authored and mstsirkin committed Mar 4, 2022
1 parent e867144 commit 0b0af4d
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 19 deletions.
48 changes: 29 additions & 19 deletions hw/virtio/vhost-user.c
Expand Up @@ -25,6 +25,7 @@
#include "migration/migration.h"
#include "migration/postcopy-ram.h"
#include "trace.h"
#include "exec/ramblock.h"

#include <sys/ioctl.h>
#include <sys/socket.h>
Expand Down Expand Up @@ -1162,15 +1163,26 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev,
return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring);
}

static void vhost_user_host_notifier_remove(struct vhost_dev *dev,
int queue_idx)
static void vhost_user_host_notifier_free(VhostUserHostNotifier *n)
{
struct vhost_user *u = dev->opaque;
VhostUserHostNotifier *n = &u->user->notifier[queue_idx];
VirtIODevice *vdev = dev->vdev;
assert(n && n->unmap_addr);
munmap(n->unmap_addr, qemu_real_host_page_size);
n->unmap_addr = NULL;
}

static void vhost_user_host_notifier_remove(VhostUserState *user,
VirtIODevice *vdev, int queue_idx)
{
VhostUserHostNotifier *n = &user->notifier[queue_idx];

if (n->addr) {
virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false);
if (vdev) {
virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false);
}
assert(!n->unmap_addr);
n->unmap_addr = n->addr;
n->addr = NULL;
call_rcu(n, vhost_user_host_notifier_free, rcu);
}
}

Expand Down Expand Up @@ -1219,8 +1231,9 @@ static int vhost_user_get_vring_base(struct vhost_dev *dev,
.payload.state = *ring,
.hdr.size = sizeof(msg.payload.state),
};
struct vhost_user *u = dev->opaque;

vhost_user_host_notifier_remove(dev, ring->index);
vhost_user_host_notifier_remove(u->user, dev->vdev, ring->index);

ret = vhost_user_write(dev, &msg, NULL, 0);
if (ret < 0) {
Expand Down Expand Up @@ -1506,12 +1519,7 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,

n = &user->notifier[queue_idx];

if (n->addr) {
virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false);
object_unparent(OBJECT(&n->mr));
munmap(n->addr, page_size);
n->addr = NULL;
}
vhost_user_host_notifier_remove(user, vdev, queue_idx);

if (area->u64 & VHOST_USER_VRING_NOFD_MASK) {
return 0;
Expand All @@ -1530,9 +1538,12 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,

name = g_strdup_printf("vhost-user/host-notifier@%p mmaps[%d]",
user, queue_idx);
if (!n->mr.ram) /* Don't init again after suspend. */
if (!n->mr.ram) { /* Don't init again after suspend. */
memory_region_init_ram_device_ptr(&n->mr, OBJECT(vdev), name,
page_size, addr);
} else {
n->mr.ram_block->host = addr;
}
g_free(name);

if (virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true)) {
Expand Down Expand Up @@ -2505,17 +2516,16 @@ bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
void vhost_user_cleanup(VhostUserState *user)
{
int i;
VhostUserHostNotifier *n;

if (!user->chr) {
return;
}
memory_region_transaction_begin();
for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
if (user->notifier[i].addr) {
object_unparent(OBJECT(&user->notifier[i].mr));
munmap(user->notifier[i].addr, qemu_real_host_page_size);
user->notifier[i].addr = NULL;
}
n = &user->notifier[i];
vhost_user_host_notifier_remove(user, NULL, i);
object_unparent(OBJECT(&n->mr));
}
memory_region_transaction_commit();
user->chr = NULL;
Expand Down
2 changes: 2 additions & 0 deletions include/hw/virtio/vhost-user.h
Expand Up @@ -12,8 +12,10 @@
#include "hw/virtio/virtio.h"

typedef struct VhostUserHostNotifier {
struct rcu_head rcu;
MemoryRegion mr;
void *addr;
void *unmap_addr;
} VhostUserHostNotifier;

typedef struct VhostUserState {
Expand Down

0 comments on commit 0b0af4d

Please sign in to comment.