[PATCH] usb: typec: tcpci: free IRQ before unregistering TCPM port

[PATCH] usb: typec: tcpci: free IRQ before unregistering TCPM port

[Yaohan Li]

tcpci_remove() unregisters the TCPM port before the managed threaded
IRQ has been released. The IRQ handler can still queue work on the
TCPM port worker after unregister has destroyed that worker, causing
a use-after-free.

Free and synchronize the managed IRQ before calling
tcpci_unregister_port(). This ensures no new alert callback can
queue work after the TCPM worker has been torn down.

A KASAN test using a simulated I2C TCPCI device reproduced the
race while unregistering the port. The corresponding teardown
sequence produced no report after the change.

Fixes: ae8a2ca8a2215 ("usb: typec: Group all TCPCI/TCPM code together")
Assisted-by: ZCode:GLM-4
Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
---
 drivers/usb/typec/tcpm/tcpci.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 6717ac914c6a6..580dd5256d0a3 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -993,6 +993,15 @@ static void tcpci_remove(struct i2c_client *client)
 	if (err < 0)
 		dev_warn(&client->dev, "Failed to disable irqs (%pe)\n", ERR_PTR(err));
 
+	/*
+	 * The port's kthread worker is freed by tcpci_unregister_port()
+	 * below, and the threaded IRQ handler queues work on it without
+	 * any port-registered check. Free the IRQ (which also waits for
+	 * an in-flight handler) before the port is unregistered: the devm
+	 * cleanup would only run after .remove() has returned.
+	 */
+	devm_free_irq(&client->dev, client->irq, chip);
+
 	tcpci_unregister_port(chip->tcpci);
 }
 
-- 
2.34.1

Re: [PATCH] usb: typec: tcpci: free IRQ before unregistering TCPM port

[Xu Yang]

On Wed, Nov 04, 2026 at 05:51:32PM +0800, Yaohan Li wrote:
> tcpci_remove() unregisters the TCPM port before the managed threaded
> IRQ has been released. The IRQ handler can still queue work on the
> TCPM port worker after unregister has destroyed that worker, causing
> a use-after-free.

The ALERT_MASK is set to 0 and chip interrupts are masked before unregister
the port. I assume no IRQ should be fired again after port is unregisterd.

> 
> Free and synchronize the managed IRQ before calling
> tcpci_unregister_port(). This ensures no new alert callback can
> queue work after the TCPM worker has been torn down.
> 
> A KASAN test using a simulated I2C TCPCI device reproduced the
> race while unregistering the port. The corresponding teardown
> sequence produced no report after the change.

Can you describe how the race happens?

Thanks,
Xu Yang

> 
> Fixes: ae8a2ca8a2215 ("usb: typec: Group all TCPCI/TCPM code together")
> Assisted-by: ZCode:GLM-4
> Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
> ---
>  drivers/usb/typec/tcpm/tcpci.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index 6717ac914c6a6..580dd5256d0a3 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -993,6 +993,15 @@ static void tcpci_remove(struct i2c_client *client)
>  	if (err < 0)
>  		dev_warn(&client->dev, "Failed to disable irqs (%pe)\n", ERR_PTR(err));
>  
> +	/*
> +	 * The port's kthread worker is freed by tcpci_unregister_port()
> +	 * below, and the threaded IRQ handler queues work on it without
> +	 * any port-registered check. Free the IRQ (which also waits for
> +	 * an in-flight handler) before the port is unregistered: the devm
> +	 * cleanup would only run after .remove() has returned.
> +	 */
> +	devm_free_irq(&client->dev, client->irq, chip);
> +
>  	tcpci_unregister_port(chip->tcpci);
>  }
>  
> -- 
> 2.34.1
> 

Re: [PATCH] usb: typec: tcpci: free IRQ before unregistering TCPM port

[zhang san]

Hi Xu,

Thanks for the review.

You are right that tcpci_remove() clears TCPC_ALERT_MASK before
unregistering the TCPM port. However, this prevents subsequent chip
alerts; it does not synchronize with a threaded IRQ handler that was
already woken or running before the mask write.

In particular, tcpci_irq() can call tcpm_cc_change(), which queues
event_work on port->wq. Meanwhile, tcpm_unregister_port() destroys that
worker. An in-flight IRQ thread can therefore reach
kthread_queue_work() after the worker has been destroyed.

We reproduced this ordering on a KASAN-enabled 7.4-rc1 kernel using a
simulated I2C TCPCI device. A kprobe paused the real threaded handler
immediately before kthread_queue_work() while another CPU unbound the
device.

Without the patch, TCPM worker teardown completed before the handler
resumed, and KASAN reported a slab-use-after-free in
kthread_queue_work().

With the patch, devm_free_irq() waited for the handler to finish before
tcpci_unregister_port() destroyed the worker, and the same test produced
no KASAN report.

This is an injection-assisted reproduction, not a measurement of how
often the race occurs on physical hardware. It demonstrates that
masking the chip alert does not by itself exclude an already in-flight
threaded handler.

I would be happy to provide further details of the interleaving or test
setup if useful.

Best regards,
Yaohan Li


Xu Yang <xu.yang_2@oss.nxp.com> 于2026年11月6日周五 10:19写道：
>
> On Wed, Nov 04, 2026 at 05:51:32PM +0800, Yaohan Li wrote:
> > tcpci_remove() unregisters the TCPM port before the managed threaded
> > IRQ has been released. The IRQ handler can still queue work on the
> > TCPM port worker after unregister has destroyed that worker, causing
> > a use-after-free.
>
> The ALERT_MASK is set to 0 and chip interrupts are masked before unregister
> the port. I assume no IRQ should be fired again after port is unregisterd.
>
> >
> > Free and synchronize the managed IRQ before calling
> > tcpci_unregister_port(). This ensures no new alert callback can
> > queue work after the TCPM worker has been torn down.
> >
> > A KASAN test using a simulated I2C TCPCI device reproduced the
> > race while unregistering the port. The corresponding teardown
> > sequence produced no report after the change.
>
> Can you describe how the race happens?
>
> Thanks,
> Xu Yang
>
> >
> > Fixes: ae8a2ca8a2215 ("usb: typec: Group all TCPCI/TCPM code together")
> > Assisted-by: ZCode:GLM-4
> > Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
> > ---
> >  drivers/usb/typec/tcpm/tcpci.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> > index 6717ac914c6a6..580dd5256d0a3 100644
> > --- a/drivers/usb/typec/tcpm/tcpci.c
> > +++ b/drivers/usb/typec/tcpm/tcpci.c
> > @@ -993,6 +993,15 @@ static void tcpci_remove(struct i2c_client *client)
> >       if (err < 0)
> >               dev_warn(&client->dev, "Failed to disable irqs (%pe)\n", ERR_PTR(err));
> >
> > +     /*
> > +      * The port's kthread worker is freed by tcpci_unregister_port()
> > +      * below, and the threaded IRQ handler queues work on it without
> > +      * any port-registered check. Free the IRQ (which also waits for
> > +      * an in-flight handler) before the port is unregistered: the devm
> > +      * cleanup would only run after .remove() has returned.
> > +      */
> > +     devm_free_irq(&client->dev, client->irq, chip);
> > +
> >       tcpci_unregister_port(chip->tcpci);
> >  }
> >
> > --
> > 2.34.1
> >

Re: [PATCH] usb: typec: tcpci: free IRQ before unregistering TCPM port

[Xu Yang]

On Fri, Nov 06, 2026 at 12:45:48PM +0800, zhang san wrote:
> Hi Xu,
> 
> Thanks for the review.
> 
> You are right that tcpci_remove() clears TCPC_ALERT_MASK before
> unregistering the TCPM port. However, this prevents subsequent chip
> alerts; it does not synchronize with a threaded IRQ handler that was
> already woken or running before the mask write.
> 
> In particular, tcpci_irq() can call tcpm_cc_change(), which queues
> event_work on port->wq. Meanwhile, tcpm_unregister_port() destroys that
> worker. An in-flight IRQ thread can therefore reach
> kthread_queue_work() after the worker has been destroyed.
> 
> We reproduced this ordering on a KASAN-enabled 7.4-rc1 kernel using a
> simulated I2C TCPCI device. A kprobe paused the real threaded handler
> immediately before kthread_queue_work() while another CPU unbound the
> device.
> 
> Without the patch, TCPM worker teardown completed before the handler
> resumed, and KASAN reported a slab-use-after-free in
> kthread_queue_work().
> 
> With the patch, devm_free_irq() waited for the handler to finish before
> tcpci_unregister_port() destroyed the worker, and the same test produced
> no KASAN report.
> 
> This is an injection-assisted reproduction, not a measurement of how
> often the race occurs on physical hardware. It demonstrates that
> masking the chip alert does not by itself exclude an already in-flight
> threaded handler.
> 
> I would be happy to provide further details of the interleaving or test
> setup if useful.

OK. 

I think synchronize_irq() would be more suitable for this case. Otherwise,
using devm_* would lose its purpose.

Thanks,
Xu Yang