Re: [PATCH v3 2/3] virtio_pci_legacy: drop callback sync on reset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition during system shutdown due to missing callback synchronization after device reset, leading to potential use-after-free or system crash.
--

commit 78e24f18ba4e25cf07c789fb8728c72626386f80
Author: Michael S. Tsirkin <mst@redhat.com>

virtio_pci_legacy: drop callback sync on reset

The virtio core now synchronizes callbacks after reset,
so the legacy PCI transport no longer needs to do it itself.

> diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
> index d9cbb02b35a11..8115aa39e01e9 100644
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -98,8 +98,6 @@ static void vp_reset(struct virtio_device *vdev)
>  	/* Flush out the status write, and flush in device writes,
>  	 * including MSi-X interrupts, if any. */
>  	vp_legacy_get_status(&vp_dev->ldev);
> -	/* Flush pending VQ/configuration callbacks. */
> -	vp_synchronize_vectors(vdev);

[Severity: High]
Does removing this internal synchronization introduce a race condition
during system shutdown?

Unlike virtio_reset_device(), virtio_device_shutdown() in
drivers/virtio/virtio.c does not synchronize callbacks after resetting
the device:

drivers/virtio/virtio.c:virtio_device_shutdown() {
	...
	virtio_synchronize_cbs(dev);
	/*
	 * As IOMMUs are reset on shutdown, this will block device access to memory.
	 * Some devices get wedged if this happens, so reset to make sure it does not.
	 */
	dev->config->reset(dev);
}

When dev->config->reset(dev) calls vp_reset(), the status read can trigger
a final in-flight MSI-X interrupt.

Without vp_synchronize_vectors(), vp_reset() now returns immediately while
the interrupt handler could still execute drv->config_changed(dev)
concurrently on another CPU.

Can this lead to a fatal bus error, use-after-free, or kernel crash as the
system teardown sequence proceeds?

>  }
>  
>  static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789160982.git.mst@redhat.com?part=2