Re: [PATCH] vhost-vdpa: drop the parent's vq callback before the call fd is released

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Aug 07, 2026 at 03:04:04PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Torn read causing NULL pointer dereference and incomplete UAF fix for hardware VDPA drivers.
> --
> 
> --- Patch [1]: [PATCH] vhost-vdpa: drop the parent's vq callback before the call fd is released ---
> commit 59fc02f21fdc99bd2ce2389a383eab4badff2c47
> Author: Yu Zhang <yuz08559@gmail.com>
> 
> vhost-vdpa: drop the parent's vq callback before the call fd is released
> 
> This commit clears the vdpa callback pointers before vhost_vring_ioctl()
> releases the eventfd to prevent a use-after-free regression with VDUSE. It
> aims to ensure the parent driver drops references to the callback context
> before it is destroyed.
> 
> > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> > index c3d913bd7cac7..63ffa94bb4793 100644
> > --- a/drivers/vhost/vdpa.c
> > +++ b/drivers/vhost/vdpa.c
> > @@ -721,13 +721,34 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
> >  			if (ops->get_status(vdpa) &
> >  			    VIRTIO_CONFIG_S_DRIVER_OK)
> >  				vhost_vdpa_unsetup_vq_irq(v, idx);
> > +			/*
> > +			 * The parent caches call_ctx.ctx in cb.trigger without
> > +			 * holding a reference, so it has to stop using it
> > +			 * before vhost_vring_ioctl() drops the last one.
> > +			 */
> > +			cb.callback = NULL;
> > +			cb.private = NULL;
> > +			cb.trigger = NULL;
> > +			ops->set_vq_cb(vdpa, idx, &cb);
> 
> [Severity: Critical]
> Can this non-atomic update cause a NULL pointer dereference for hardware
> drivers?
> 
> For hardware drivers like vp_vdpa, the interrupt handler reads the callback
> struct without locks. If a concurrent interrupt occurs during ops->set_vq_cb(),
> could it read a torn state where it sees the old valid cb.callback but the
> new NULL cb.private?
> 
> drivers/vdpa/virtio_pci/vp_vdpa.c:vp_vdpa_vq_handler() {
>     ...
> 	if (vring->cb.callback)
> 		return vring->cb.callback(vring->cb.private);
>     ...
> }
> 
> If this happens, it would call vhost_vdpa_virtqueue_cb() with a NULL pointer,
> causing a panic when attempting to dereference it.
> 
> Additionally, since there is no synchronize_irq() here, could an
> already-executing hardware interrupt handler still race with the subsequent
> vhost_vring_ioctl() freeing the eventfd, leaving the original use-after-free
> unfixed for hardware drivers?

Yu Zhang any plans to address these concerns? Seem legit.


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