[PATCH] usb: typec: ucsi: ccg: Fix use-after-free of ucsi on remove

[PATCH] usb: typec: ucsi: ccg: Fix use-after-free of ucsi on remove

[Fan Wu]

The threaded IRQ handler ccg_irq_handler() calls ucsi_notify_common(),
which on a connector-change event calls ucsi_connector_change() and
schedules connector work.  In ucsi_ccg_remove(), ucsi_destroy() frees
uc->ucsi (kfree) before free_irq() is called, so a handler invocation
already in flight may access the freed object after ucsi_destroy().

  CPU 0 (remove)            | CPU 1 (threaded IRQ)
    ucsi_destroy(uc->ucsi)  |   ccg_irq_handler()
      kfree(ucsi) // FREE   |     ucsi_notify_common(uc->ucsi) // USE

Move free_irq() before ucsi_destroy() in the remove path.  It is kept
after ucsi_unregister(): ucsi_unregister() cancels connector work whose
handler issues GET_CONNECTOR_STATUS through ucsi_send_command_common(),
which waits for a completion that is signalled from the IRQ handler, so
the IRQ must stay active until that work has been cancelled.

The probe error path already orders free_irq() before ucsi_destroy().

This bug was found by static analysis.

Fixes: e32fd989ac1c ("usb: typec: ucsi: ccg: Move to the new API")
Cc: stable@vger.kernel.org
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
index d83a0051c737..c089000bd448 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -1513,8 +1513,8 @@ static void ucsi_ccg_remove(struct i2c_client *client)
 	cancel_work_sync(&uc->work);
 	pm_runtime_disable(uc->dev);
 	ucsi_unregister(uc->ucsi);
-	ucsi_destroy(uc->ucsi);
 	free_irq(uc->irq, uc);
+	ucsi_destroy(uc->ucsi);
 }

 static const struct of_device_id ucsi_ccg_of_match_table[] = {
--
2.45.2

Re: [PATCH] usb: typec: ucsi: ccg: Fix use-after-free of ucsi on remove

[Heikki Krogerus]

On Tue, Jun 16, 2026 at 01:20:11PM +0000, Fan Wu wrote:
> The threaded IRQ handler ccg_irq_handler() calls ucsi_notify_common(),
> which on a connector-change event calls ucsi_connector_change() and
> schedules connector work.  In ucsi_ccg_remove(), ucsi_destroy() frees
> uc->ucsi (kfree) before free_irq() is called, so a handler invocation
> already in flight may access the freed object after ucsi_destroy().
> 
>   CPU 0 (remove)            | CPU 1 (threaded IRQ)
>     ucsi_destroy(uc->ucsi)  |   ccg_irq_handler()
>       kfree(ucsi) // FREE   |     ucsi_notify_common(uc->ucsi) // USE
> 
> Move free_irq() before ucsi_destroy() in the remove path.  It is kept
> after ucsi_unregister(): ucsi_unregister() cancels connector work whose
> handler issues GET_CONNECTOR_STATUS through ucsi_send_command_common(),
> which waits for a completion that is signalled from the IRQ handler, so
> the IRQ must stay active until that work has been cancelled.
> 
> The probe error path already orders free_irq() before ucsi_destroy().
> 
> This bug was found by static analysis.
> 
> Fixes: e32fd989ac1c ("usb: typec: ucsi: ccg: Move to the new API")
> Cc: stable@vger.kernel.org
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
> index d83a0051c737..c089000bd448 100644
> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -1513,8 +1513,8 @@ static void ucsi_ccg_remove(struct i2c_client *client)
>  	cancel_work_sync(&uc->work);
>  	pm_runtime_disable(uc->dev);
>  	ucsi_unregister(uc->ucsi);
> -	ucsi_destroy(uc->ucsi);
>  	free_irq(uc->irq, uc);
> +	ucsi_destroy(uc->ucsi);
>  }
> 
>  static const struct of_device_id ucsi_ccg_of_match_table[] = {
> --
> 2.45.2

-- 
heikki