Re: [PATCH v2] usb: typec: tcpci: quiesce the IRQ before unregistering the TCPM port

[Xu Yang]

On Fri, Nov 06, 2026 at 05:17:25PM +0800, Yaohan Li wrote:
> tcpci_remove() unregisters the TCPM port before the managed threaded
> IRQ has been released.  tcpci_unregister_port() destroys the TCPM
> kthread worker, while tcpci_irq() can still call the TCPM event entry
> points (tcpm_cc_change() and friends), which queue work on port->wq
> without checking whether the port is still registered.  Writing
> TCPC_ALERT_MASK = 0 prevents new chip alerts, but it does not
> synchronize with a threaded handler that was already woken or running.
> 
> Quiesce the handler before unregistering the port:
> 
> - Set chip->stopping before synchronize_irq(), so handlers that already
>   entered tcpci_irq() are waited for, while handlers that start later
>   return IRQ_NONE from _tcpci_irq() without touching the TCPM port.
> - The IRQ remains devm-managed and is released on driver detach, as
>   suggested by Xu Yang; no custom early free_irq is needed.
> - synchronize_irq() alone would not be enough because the IRQ is shared:
>   another device on the same line can trigger the handler after
>   synchronize_irq() returns; the stopping flag makes such late calls
>   harmless.

It's unnecessary to add chip->stopping to return IRQ_NONE, because the
ALERT is already masked at this point. In shared IRQ case, this device's
irq handler may be called if another device trigger it, but it will do
nothing (status will be 0) and return IRQ_NONE at the end.

> 
> The same ordering issue exists on the probe error path: if the final
> TCPC_ALERT_MASK enable write fails after devm_request_threaded_irq()
> succeeded, the error path unregisters the port before devres frees the
> IRQ.  Route that failure through a quiesce_irq label that sets stopping
> and synchronizes the IRQ before unregistering.  If the IRQ request
> itself failed, the error goes directly to unregister_port.

It's unnecessary as well. Firstly, TCPC_ALERT_MASK is set to 0 before it
request threded irq. If it fails to set TCPC_ALERT_MASK, it may follow
above behavior if this device's irq handler is called in shared IRQ case.

> 
> This issue was investigated and the fix was developed with AI
> assistance (ZCode/GLM-4).
> 
> Testing with QEMU, a simulated I2C TCPCI device and KASAN (7.4-rc1):
> 
> - Remove path with a real threaded handler parked at
>   kthread_queue_work() while the device is unbound: synchronize_irq()
>   waits for the handler before tcpci_unregister_port() destroys the
>   worker; no report.  Without synchronization the worker is freed first
>   and KASAN reports a slab-use-after-free in kthread_queue_work().
> - Probe failure path (the final ALERT_MASK enable write is forced to
>   fail): the handler woken by the injected alert is rejected by the
>   stopping flag, and the teardown reports nothing.
> - A shared IRQ raised at tcpci_unregister_port() after
>   synchronize_irq() returns enters _tcpci_irq(), returns IRQ_NONE due
>   to chip->stopping, and never reaches tcpm_cc_change(); no report.
> - tcpci_remove() with the ALERT_MASK = 0 write forced to fail (the
>   driver only warns) while a continuous alert stream is active: the
>   teardown completes, no tcpm_* entry point is reached after the TCPM
>   worker is destroyed, and no report appears.
> 
> These are injection-assisted reproductions; they do not measure how
> often the race occurs on physical hardware.

Above words should not go to commit message.

Thanks,
Xu Yang