[PATCH 0/2] vhost-vdpa: fix a use-after-free on the config eventfd

[PATCH 0/2] vhost-vdpa: fix a use-after-free on the config eventfd

[Yu Zhang]

vhost_vdpa_config_cb() reads v->config_ctx with no reference and no lock
while VHOST_VDPA_SET_CONFIG_CALL frees the old context, so a config
interrupt delivered at the wrong moment signals freed memory.

Patch 1 stops the fdget error value from ever being installed in
config_ctx.  It is a fix in its own right -- the callback tests only for
NULL and would signal the ERR_PTR -- and it is also what lets patch 2's
lock establish "non-NULL implies valid".

Patch 2 adds the lock that closes the use-after-free.

This is not the reopen refcount underflow fixed by f6bbf0010ba0
("vhost-vdpa: fix use-after-free of v->config_ctx"), and it is a
different bug from the vq call fd one I sent on Aug 6
(https://lore.kernel.org/all/20260806150323.2154-1-yuz08559@gmail.com/),
which is about the parent caching an eventfd_ctx it does not own.  The
three touch different functions.

Tested on v7.2-rc6 with KASAN under qemu, VDUSE as the parent: one thread
swaps the config call fd on the vhost-vdpa fd while another injects
VDUSE_DEV_INJECT_CONFIG_IRQ on the VDUSE fd.  9 of 10 30-second runs hit
the UAF; 0 of 5 with the series applied, with the workload counters
unchanged either way (~30k swaps and ~2.6M injects per run), so the race
is still being exercised rather than merely not reached.  Also clean over
5 runs with PROVE_LOCKING, and vdpa_sim shows no change on the shared
VHOST_VDPA_SET_CONFIG_CALL path (20000 install/unbind cycles plus the
bad-fd path, identical before and after).

For patch 1 specifically: with a working config fd installed, 250 injects
deliver 250 signals; after a VHOST_VDPA_SET_CONFIG_CALL with a bad fd,
delivery stops on an unpatched kernel and continues with the patch.

Yu Zhang (2):
  vhost-vdpa: don't install the eventfd_ctx_fdget() error in config_ctx
  vhost-vdpa: protect config_ctx from being freed under the config
    callback

 drivers/vhost/vdpa.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

-- 
2.43.0

[PATCH 1/2] vhost-vdpa: don't install the eventfd_ctx_fdget() error in config_ctx

[Yu Zhang]

vhost_vdpa_set_config_call() swaps the eventfd_ctx_fdget() return value
into v->config_ctx before checking it, so on failure the field briefly
holds an ERR_PTR:

	ctx = fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(fd);
	swap(ctx, v->config_ctx);

	if (!IS_ERR_OR_NULL(ctx))
		eventfd_ctx_put(ctx);

	if (IS_ERR(v->config_ctx)) {
		long ret = PTR_ERR(v->config_ctx);

		v->config_ctx = NULL;
		return ret;
	}

Commit 0bde59c1723a ("vhost-vdpa: set v->config_ctx to NULL if
eventfd_ctx_fdget() fails") added that clearing, and spelled out the
invariant the rest of the file relies on: "we consider 'v->config_ctx'
valid if it is not NULL".  The window between the swap and the clearing
still breaks it.  vhost_vdpa_config_cb() only tests for NULL, so a config
interrupt delivered inside the window hands the ERR_PTR to
eventfd_signal().

Check the fd before installing it instead.  That closes the window and
matches how vhost_vring_ioctl() handles the same failure for the vq call
fd.

It also stops a rejected fd from tearing down a config interrupt that was
working: until now the swap replaced the live context and put it, so
after an EBADF the device silently stopped delivering config interrupts
until userspace installed a new fd.

Fixes: 776f395004d8 ("vhost_vdpa: Support config interrupt in vdpa")
Signed-off-by: Yu Zhang <yuz08559@gmail.com>
---
 drivers/vhost/vdpa.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index ac55275..e5e47f6 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -529,18 +529,14 @@ static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp)
 		return  -EFAULT;
 
 	ctx = fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(fd);
+	if (IS_ERR(ctx))
+		return PTR_ERR(ctx);
+
 	swap(ctx, v->config_ctx);
 
-	if (!IS_ERR_OR_NULL(ctx))
+	if (ctx)
 		eventfd_ctx_put(ctx);
 
-	if (IS_ERR(v->config_ctx)) {
-		long ret = PTR_ERR(v->config_ctx);
-
-		v->config_ctx = NULL;
-		return ret;
-	}
-
 	v->vdpa->config->set_config_cb(v->vdpa, &cb);
 
 	return 0;
-- 
2.43.0

Re: [PATCH 1/2] vhost-vdpa: don't install the eventfd_ctx_fdget() error in config_ctx

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Lack of synchronization between `vhost_vdpa_set_config_call()` and the config interrupt handler `vhost_vdpa_config_cb()` leads to a Data Race and a Use-After-Free on `v->config_ctx`.
--

--- Patch [1]: [PATCH 1/2] vhost-vdpa: don't install the eventfd_ctx_fdget() error in config_ctx ---
commit 97c24916f7205e9db996343c66b6d1658f9125bd
Author: Yu Zhang <yuz08559@gmail.com>

vhost-vdpa: don't install the eventfd_ctx_fdget() error in config_ctx

This commit avoids storing an error pointer in the configuration context on
failure. It validates the file descriptor prior to swapping it, preventing
config interrupts from encountering an invalid pointer.

> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -536,18 +536,14 @@ static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp)
>  		return  -EFAULT;
>  
>  	ctx = fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(fd);
> +	if (IS_ERR(ctx))
> +		return PTR_ERR(ctx);
> +
>  	swap(ctx, v->config_ctx);
>  
> -	if (!IS_ERR_OR_NULL(ctx))
> +	if (ctx)
>  		eventfd_ctx_put(ctx);

[Severity: Critical]
This isn't a bug introduced by this patch, but can this sequence lead to a
use-after-free if a config interrupt occurs concurrently?

In the interrupt handler, the pointer is read locklessly:

drivers/vhost/vdpa.c:vhost_vdpa_config_cb() {
    struct eventfd_ctx *config_ctx = v->config_ctx;

    if (config_ctx)
        eventfd_signal(config_ctx);
    ...
}

If the ioctl thread executing vhost_vdpa_set_config_call() swaps out the old
context and immediately calls eventfd_ctx_put(), it might free the context
while the interrupt handler has already read the old pointer but hasn't yet
called eventfd_signal().

Does this code need synchronization before freeing the old eventfd context to
wait for concurrent interrupt handlers to finish?

> -	if (IS_ERR(v->config_ctx)) {
> -		long ret = PTR_ERR(v->config_ctx);
> -
> -		v->config_ctx = NULL;
> -		return ret;
> -	}
> -
>  	v->vdpa->config->set_config_cb(v->vdpa, &cb);
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807100025.19750-1-yuz08559@gmail.com?part=1

[PATCH 2/2] vhost-vdpa: protect config_ctx from being freed under the config callback

[Yu Zhang]

vhost_vdpa_config_cb() loads v->config_ctx and signals it without taking
a reference and without holding any lock:

	struct eventfd_ctx *config_ctx = v->config_ctx;

	if (config_ctx)
		eventfd_signal(config_ctx);

VHOST_VDPA_SET_CONFIG_CALL replaces that field and drops what is normally
the last reference to the old context:

	swap(ctx, v->config_ctx);

	if (ctx)
		eventfd_ctx_put(ctx);

eventfd_ctx_put() drops the last kref and frees the context immediately,
with no RCU grace period, so a callback that has already loaded the
pointer goes on to dereference freed memory.  The two sides share no
lock: the ioctl runs under vhost_dev.mutex, while the parent invokes the
callback from its own interrupt or workqueue context.

This is not the reopen refcount underflow fixed by commit f6bbf0010ba0
("vhost-vdpa: fix use-after-free of v->config_ctx"), which was about
vhost_vdpa_config_put() leaving a stale pointer behind.  Here the pointer
is maintained correctly and it is the read side that is unprotected.

With VDUSE as the parent this is reachable from userspace with access to
/dev/vduse (root by default).  VDUSE_DEV_INJECT_CONFIG_IRQ queues
dev->inject, and vduse_dev_irq_inject() runs the callback under VDUSE's
own dev->irq_lock, which vhost does not hold.  vduse_dev_reset() does
flush_work(&dev->inject), but VHOST_VDPA_SET_CONFIG_CALL never goes
through reset, so an inject already in flight is not waited for.  A
process that injects config interrupts on the VDUSE fd while another
thread swaps the call fd on the vhost-vdpa fd hits it in seconds:

  BUG: KASAN: slab-use-after-free in native_queued_spin_lock_slowpath
  Read of size 4 at addr ffff888107d21808 by task kworker/u17:1/2993
  Workqueue: vduse-irq vduse_dev_irq_inject
  Call Trace:
   native_queued_spin_lock_slowpath+0x97/0x5b0
   _raw_spin_lock_irqsave+0xd4/0xe0
   eventfd_signal_mask+0x69/0x120
   vhost_vdpa_config_cb+0x34/0x50
   vduse_dev_irq_inject+0x46/0x60
   process_one_work+0x468/0x950

  Allocated by task 2992:
   do_eventfd+0x50/0x200
   __x64_sys_eventfd2+0x2e/0x40

  Freed by task 2992:
   eventfd_ctx_put+0xb9/0xc0
   vhost_vdpa_unlocked_ioctl+0x116c/0x2190

Add a spinlock covering every access to config_ctx, so the callback
either signals a context that is still alive or observes NULL, and the
put happens only once no callback can reach the old value.

Clearing the parent's callback before the put would not be enough: of the
in-tree set_config_cb() implementations only VDUSE takes a lock, the rest
store the pointer unlocked, so that would not order against an in-flight
invocation.

Fixes: 776f395004d8 ("vhost_vdpa: Support config interrupt in vdpa")
Signed-off-by: Yu Zhang <yuz08559@gmail.com>
---
 drivers/vhost/vdpa.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index e5e47f6..272d506 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -56,6 +56,8 @@ struct vhost_vdpa {
 	int virtio_id;
 	int minor;
 	struct eventfd_ctx *config_ctx;
+	/* Serialises vhost_vdpa_config_cb() against config_ctx being replaced. */
+	spinlock_t config_lock;
 	int in_batch;
 	struct vdpa_iova_range range;
 	u32 batch_asid;
@@ -187,10 +189,12 @@ static irqreturn_t vhost_vdpa_virtqueue_cb(void *private)
 static irqreturn_t vhost_vdpa_config_cb(void *private)
 {
 	struct vhost_vdpa *v = private;
-	struct eventfd_ctx *config_ctx = v->config_ctx;
+	unsigned long flags;
 
-	if (config_ctx)
-		eventfd_signal(config_ctx);
+	spin_lock_irqsave(&v->config_lock, flags);
+	if (v->config_ctx)
+		eventfd_signal(v->config_ctx);
+	spin_unlock_irqrestore(&v->config_lock, flags);
 
 	return IRQ_HANDLED;
 }
@@ -511,15 +515,22 @@ static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v, u16 __user *argp)
 
 static void vhost_vdpa_config_put(struct vhost_vdpa *v)
 {
-	if (v->config_ctx) {
-		eventfd_ctx_put(v->config_ctx);
-		v->config_ctx = NULL;
-	}
+	struct eventfd_ctx *ctx;
+	unsigned long flags;
+
+	spin_lock_irqsave(&v->config_lock, flags);
+	ctx = v->config_ctx;
+	v->config_ctx = NULL;
+	spin_unlock_irqrestore(&v->config_lock, flags);
+
+	if (ctx)
+		eventfd_ctx_put(ctx);
 }
 
 static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp)
 {
 	struct vdpa_callback cb;
+	unsigned long flags;
 	int fd;
 	struct eventfd_ctx *ctx;
 
@@ -532,8 +543,14 @@ static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp)
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 
+	spin_lock_irqsave(&v->config_lock, flags);
 	swap(ctx, v->config_ctx);
+	spin_unlock_irqrestore(&v->config_lock, flags);
 
+	/*
+	 * The callback can no longer reach the old context, so this is the
+	 * last reference to it.
+	 */
 	if (ctx)
 		eventfd_ctx_put(ctx);
 
@@ -1595,6 +1612,7 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
 	}
 
 	atomic_set(&v->opened, 0);
+	spin_lock_init(&v->config_lock);
 	v->minor = minor;
 	v->vdpa = vdpa;
 	v->nvqs = vdpa->nvqs;
-- 
2.43.0

Re: [PATCH 2/2] vhost-vdpa: protect config_ctx from being freed under the config callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] An identical Use-After-Free pattern to the one fixed for `v->config_ctx` exists unpatched for `vq->call_ctx.ctx` in the virtqueue interrupt path.
--

--- Patch [2]: [PATCH 2/2] vhost-vdpa: protect config_ctx from being freed under the config callback ---
commit 8e0bc317868b822a05e8a68785677223512b3be4
Author: Yu Zhang <yuz08559@gmail.com>

vhost-vdpa: protect config_ctx from being freed under the config callback

This commit fixes a use-after-free by adding a spinlock to serialize accesses
to config_ctx in the config callback against its replacement in the
VHOST_VDPA_SET_CONFIG_CALL ioctl.

> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index ae137d5be99c9..cae8089ab3ddf 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -194,10 +196,12 @@ static irqreturn_t vhost_vdpa_virtqueue_cb(void *private)

[Severity: Critical]
This is a pre-existing issue, but does an identical use-after-free pattern
exist for vq->call_ctx.ctx in the virtqueue interrupt path?

It appears vhost_vdpa_virtqueue_cb() reads and signals vq->call_ctx.ctx
locklessly:

drivers/vhost/vdpa.c:vhost_vdpa_virtqueue_cb() {
    struct eventfd_ctx *call_ctx = vq->call_ctx.ctx;
    
    if (call_ctx)
        eventfd_signal(call_ctx);
}

Could this race with VHOST_SET_VRING_CALL in vhost_vring_ioctl(), which
swaps out vq->call_ctx.ctx and immediately frees the old context?

drivers/vhost/vhost.c:vhost_vring_ioctl() {
    swap(ctx, vq->call_ctx.ctx);
    ...
    if (!IS_ERR_OR_NULL(ctx))
        eventfd_ctx_put(ctx);
}

If a userspace process with access to /dev/vduse concurrently invokes
VDUSE_VQ_INJECT_IRQ while VHOST_SET_VRING_CALL is executing, could
vduse_vq_signal_irqfd() end up calling eventfd_signal() on freed memory
since it uses the cached vq->cb.trigger?

drivers/vdpa/vdpa_user/vduse_dev.c:vduse_vq_signal_irqfd() {
    if (vq->ready && vq->cb.trigger) {
        eventfd_signal(vq->cb.trigger);
        signal = true;
    }
}

>  static irqreturn_t vhost_vdpa_config_cb(void *private)
>  {
>  	struct vhost_vdpa *v = private;
> -	struct eventfd_ctx *config_ctx = v->config_ctx;
> +	unsigned long flags;
>  
> -	if (config_ctx)
> -		eventfd_signal(config_ctx);
> +	spin_lock_irqsave(&v->config_lock, flags);
> +	if (v->config_ctx)
> +		eventfd_signal(v->config_ctx);
> +	spin_unlock_irqrestore(&v->config_lock, flags);
>  
>  	return IRQ_HANDLED;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807100025.19750-1-yuz08559@gmail.com?part=2