Re: [PATCH] usb: typec: hd3ss3220: disable the interrupt before unregistering the port

[sashiko-bot@kernel.org]

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