Linux USB
 help / color / mirror / Atom feed
* [PATCH] usb: typec: hd3ss3220: disable the interrupt before unregistering the port
@ 2026-11-12  6:56 Myeonghun Pak
  2026-11-12  7:07 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Myeonghun Pak @ 2026-11-12  6:56 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Biju Das, linux-usb, linux-kernel, Ijae Kim,
	stable

The devm-managed IRQ remains active while hd3ss3220_remove() unregisters
the Type-C port and releases the role switch. A cable event during unbind
can make hd3ss3220_set_role() access either resource after release.

Disable and synchronize the IRQ before teardown, matching the existing
polling-work cancellation. The ID pin IRQ needs no change: its GPIO and
regulator are acquired before it, so devres releases that IRQ first.

Found by a static analysis tool under development.
Not tested on hardware.

Fixes: 1c48c759ef4b ("usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---

 drivers/usb/typec/hd3ss3220.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
index 60d6de9ba339..19c5dbdc97a1 100644
--- a/drivers/usb/typec/hd3ss3220.c
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -554,6 +554,8 @@ static void hd3ss3220_remove(struct i2c_client *client)
 
 	if (hd3ss3220->poll)
 		cancel_delayed_work_sync(&hd3ss3220->output_poll_work);
+	else
+		disable_irq(client->irq);
 
 	typec_unregister_port(hd3ss3220->port);
 	usb_role_switch_put(hd3ss3220->role_sw);

base-commit: 630059a5a632377b9b7b487baccde92b7ae1dd47
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] usb: typec: hd3ss3220: disable the interrupt before unregistering the port
  2026-11-12  6:56 [PATCH] usb: typec: hd3ss3220: disable the interrupt before unregistering the port Myeonghun Pak
@ 2026-11-12  7:07 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-11-12  7:07 UTC (permalink / raw)
  To: Myeonghun Pak; +Cc: linux-usb

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] usb: typec: hd3ss3220: IRQ race in hd3ss3220_probe() and
  hd3ss3220_remove() [1]
--

commit 2013fda123b6f1830f8ccc3b96427044ac637553
Author: Myeonghun Pak <mhun512@gmail.com>

usb: typec: hd3ss3220: disable the interrupt before unregistering the port

This commit disables client->irq in hd3ss3220_remove() before
unregistering the Type-C port and releasing the USB role switch.

> diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
> index 60d6de9ba3399..19c5dbdc97a1c 100644
> --- a/drivers/usb/typec/hd3ss3220.c
> +++ b/drivers/usb/typec/hd3ss3220.c
> @@ -554,6 +554,8 @@ static void hd3ss3220_remove(struct i2c_client *client)
>  
>  	if (hd3ss3220->poll)
>  		cancel_delayed_work_sync(&hd3ss3220->output_poll_work);
> +	else
> +		disable_irq(client->irq);
>  
>  	typec_unregister_port(hd3ss3220->port);
>  	usb_role_switch_put(hd3ss3220->role_sw);

[Severity: Critical]
This problem wasn't introduced by this patch, but can the same race
[1] still happen on the error path in hd3ss3220_probe()?

drivers/usb/typec/hd3ss3220.c:hd3ss3220_probe() {
    ...
	if (client->irq > 0) {
		ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
					hd3ss3220_irq_handler,
					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
					"hd3ss3220", &client->dev);
		if (ret)
			goto err_unreg_port;
	}
    ...
	ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
	if (ret < 0)
		goto err_unreg_port;
    ...
err_unreg_port:
	typec_unregister_port(hd3ss3220->port);
err_put_role:
	usb_role_switch_put(hd3ss3220->role_sw);
}

If reading HD3SS3220_REG_DEV_REV fails after devm_request_threaded_irq()
succeeds, hd3ss3220_probe() tears down hd3ss3220->port and
hd3ss3220->role_sw before devres frees client->irq:

CPU0                              CPU1
hd3ss3220_probe()
  typec_unregister_port(port)
  usb_role_switch_put(role_sw)
                                  hd3ss3220_irq_handler()
                                    ...
                                      hd3ss3220_set_role()
                                        typec_set_data_role(port, ...)
                                          <-- use-after-free
devres_release_all()

[1] https://sashiko.dev/#/bug/linux-37ab65bb-ba98-4e43-a446-fe1182fefabb

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-12  7:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-12  6:56 [PATCH] usb: typec: hd3ss3220: disable the interrupt before unregistering the port Myeonghun Pak
2026-11-12  7:07 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox