Re: [PATCH] virtio: check for driver remove callback before calling it

[Francesco Valla <francesco@valla.it>]

On Sun, Nov 08, 2026 at 04:50:38PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] virtio: unreset device in virtio_dev_remove() when
>   drv->remove is NULL
> It also noticed 1 related pre-existing issue this patch does not
> introduce:
> - [Critical] virtio: use-after-free in virtio_dev_remove() [1]
> --
> 
> commit ccd4078efa3bba738fda1cc44e8e8955495a84fa
> Author: Francesco Valla <francesco@valla.it>
> 
> virtio: check for driver remove callback before calling it
> 
> This commit checks whether drv->remove is non-NULL before calling it in
> virtio_dev_remove().
> 
> > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> > index a8588d7ad109f..067d32c67b44f 100644
> > --- a/drivers/virtio/virtio.c
> > +++ b/drivers/virtio/virtio.c
> > @@ -374,7 +374,8 @@ static void virtio_dev_remove(struct device *_d)
> >  
> >  	virtio_config_core_disable(dev);
> >  
> > -	drv->remove(dev);
> > +	if (drv->remove)
> > +		drv->remove(dev);
> >  
> >  	/* Driver should have reset device. */
> >  	WARN_ON_ONCE(dev->config->get_status(dev));
> 
> [Severity: Medium]
> When drv->remove is NULL, will WARN_ON_ONCE(dev->config->get_status(dev))
> always fire on unbind or module unload?
> 
> __device_release_driver() calls device_remove() (which runs
> virtio_dev_remove()) before device_unbind_cleanup() runs
> devres_release_all().  Even when a driver such as spi-virtio registers a
> devm callback to reset the device and delete its virtqueues, that cleanup
> has not run yet when virtio_dev_remove() checks get_status() and sets
> VIRTIO_CONFIG_S_ACKNOWLEDGE.  The subsequent devm reset then wipes out
> VIRTIO_CONFIG_S_ACKNOWLEDGE, or if a driver has no devm reset action at
> all, the device is never reset and remains active with live virtqueues and
> DMA after its devm resources are freed.
>

Yes, but that's (IMO) better than a NULL dereference. The spi-virtio
driver will be fixed in a different series, as stated.

> [Severity: Critical]
> This problem wasn't introduced by this patch, but calling
> of_node_put(dev->dev.of_node) at the end of virtio_dev_remove() causes a
> refcount underflow and use-after-free [1].
> 
> virtio_device_of_init() acquires the dev->dev.of_node reference during
> register_virtio_device(), not during driver probe, and virtio_dev_remove()
> drops that reference on driver unbind without clearing dev->dev.of_node.
> Unbinding and rebinding a driver drops the reference multiple times on the
> same pointer, while unregistering a device that never bound to a driver
> leaks the reference.
> 
> [1] https://sashiko.dev/#/bug/linux-cf8a6c4a-49fb-4827-8f82-d189cf36ccdd
> 

That's... interesting. I'll add it to my backlog if no one is already
working on it.

> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261108-virtio_drv_remove_fix-v1-1-23da1a7c3900@valla.it?part=1

Regards,
Francesco