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 <[email protected]>

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/[email protected]?part=2

Reply via email to