Re: [PATCH RFC 1/3] PCI: Report surprise removal event

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition between `pci_dev_set_disconnected()` and `pci_clear_disconnect_work()` allows a `work_struct` to be queued after the driver teardown completes, leading to use-after-free or execution of unloaded module code.
--

commit 889ac5c66b7f1749706d4c7c756b37926ccf5245
Author: Michael S. Tsirkin <mst@redhat.com>

PCI: Report surprise removal event

This commit introduces a mechanism to report surprise removal events to
drivers via a work_struct. It allows drivers to initiate disconnect cleanup
without waiting on device interrupts that will never arrive.

Link: https://lore.kernel.org/all/fba3d235e38c1c6fcef2a30ed083ad9e25b20fa3.1752094439.git.mst@redhat.com/

> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index ba3c3fddddc23..23b1605e783a3 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -805,6 +805,12 @@ static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
>  	pci_dev_set_io_state(dev, pci_channel_io_perm_failure);
>  	pci_doe_disconnected(dev);
>  
> +	if (READ_ONCE(dev->disconnect_work_enable)) {
> +		/* Make sure work is up to date. */
> +		smp_rmb();
> +		schedule_work(&dev->disconnect_work);

[Severity: High]
Does this lockless check introduce a Time-of-Check to Time-of-Use (TOCTOU)
race window?

If a thread executing pci_dev_set_disconnected() is preempted right after
checking READ_ONCE(dev->disconnect_work_enable) but before calling
schedule_work(), the work might get queued after driver teardown completes.

If a concurrent driver unbind operation calls pci_clear_disconnect_work(),
it will set the flag to 0 and call cancel_work_sync(). Since the work is
not yet queued, cancel_work_sync() would return immediately. The device is
unbound and can be freed, or the module unloaded. When the preempted thread
resumes, it enqueues the work_struct which now resides in freed memory,
potentially causing a Use-After-Free or a crash from executing unmapped
memory.

Note: This issue is not resolved by follow-up patches in this series; the
vulnerable logic is merely moved to schedule_notification_work().

> +	}
> +
>  	return 0;
>  }

[ ... ]

> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d31a8d107b1ef..06d43f57f5094 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h

[ ... ]

> +static inline void pci_clear_disconnect_work(struct pci_dev *pdev)
> +{
> +	WRITE_ONCE(pdev->disconnect_work_enable, 0x0);
> +
> +	/* Make sure to stop using work from now on. */
> +	smp_wmb();
> +
> +	cancel_work_sync(&pdev->disconnect_work);

[Severity: High]
This teardown function assumes cancel_work_sync() will catch all pending
work, but can it synchronize with threads that were preempted just before
calling schedule_work() in pci_dev_set_disconnected()?

> +}

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