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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] usb: typec: tcpci: Fixes tag cites file-move commit ae8a2ca8a221
--

commit 86b087596ae2abb991b5f1a00292c87c28b06171
Author: Yaohan Li <shunsheng1218@gmail.com>

> usb: typec: tcpci: quiesce the IRQ before unregistering the TCPM port
>
> 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.
>
> 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.
>
> 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.
>
> Fixes: ae8a2ca8a2215 ("usb: typec: Group all TCPCI/TCPM code together")
> Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>

[Severity: Low]
Commit ae8a2ca8a221 ("usb: typec: Group all TCPCI/TCPM code together") was a
pure file move from drivers/usb/typec/tcpci.c to
drivers/usb/typec/tcpm/tcpci.c.

The tcpci_remove() ordering issue dates back to commit 74e656d6b055 ("staging:
typec: Type-C Port Controller Interface driver (tcpci)"), and the
tcpci_probe() error path after devm_request_threaded_irq() was introduced by
commit 862a9c0f6848 ("usb: typec: tcpci: fix NULL pointer issue on shared irq
case").  Should the Fixes: tags cite those two commits instead?

  Fixes: 74e656d6b055 ("staging: typec: Type-C Port Controller Interface driver (tcpci)")
  Fixes: 862a9c0f6848 ("usb: typec: tcpci: fix NULL pointer issue on shared irq case")

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