Re: [PATCH v2 2/2] usb: typec: ucsi: Add ITE IT885x Type-C PD controller driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Race condition during driver removal causing a use-after-free on the `ucsi->connector` array.
- [High] Infinite IRQ storm and CPU lockup due to silently ignored I2C write or memory allocation failures.
--

--- Patch [2]: [PATCH v2 2/2] usb: typec: ucsi: Add ITE IT885x Type-C PD controller driver ---
commit a72e407ddecce41bb76245d04b0009fb6795ba01
Author: Amber Kao <amber.kao@ite.com.tw>

usb: typec: ucsi: Add ITE IT885x Type-C PD controller driver

This commit adds core UCSI support for the ITE IT885x USB Type-C Power
Delivery controller over I2C. It provides port and partner registration,
deferring Alternate Mode support and command translation to future patches.

> diff --git a/drivers/usb/typec/ucsi/ucsi_itepd.c b/drivers/usb/typec/ucsi/ucsi_itepd.c
> new file mode 100644
> index 0000000000000..e364a3bd7df4e
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_itepd.c

[ ... ]

> +static int itepd_write_reg(struct itepd *itepd, u8 reg, const void *data, u32 len)
> +{
> +	struct i2c_client *client = itepd->client;
> +	struct i2c_msg msg[1];
> +	u8 *buf;
> +	int ret;
> +
> +	buf = kzalloc(len + 1, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;

[ ... ]

> +static irqreturn_t itepd_irq_process(struct itepd *itepd)
> +{
> +	u8 msg_in[ITEPD_MSG_IN_MAX_LEN] = {};
> +	__le32 le_cci;
> +	u32 cci = 0;
> +	u8 event;
> +	u8 len;
> +	int ret;

[ ... ]

> +out_clear:
> +	itepd_write_reg(itepd, ITEPD_VENDOR_WC_INT, &event, sizeof(event));
> +
> +	if (!ret && (event & ITEPD_ALERT_UCSI_EVENT))
> +		ucsi_notify_common(itepd->ucsi, cci);
> +
> +	return IRQ_HANDLED;
> +}

[Severity: High]
Could a failure to clear the interrupt result in an infinite IRQ storm?

Looking at itepd_write_reg(), it dynamically allocates memory via kzalloc().
If this allocation fails with -ENOMEM, or if the underlying I2C transfer
fails, the hardware interrupt is not actually cleared on the device.

Because itepd_irq_process() ignores the return value of itepd_write_reg()
and unconditionally returns IRQ_HANDLED, the kernel IRQ subsystem will unmask
the line and immediately trigger the handler again, potentially causing a CPU
lockup under memory pressure or I2C bus errors.

[ ... ]

> +static void itepd_remove(struct i2c_client *client)
> +{
> +	struct itepd *itepd = i2c_get_clientdata(client);
> +
> +	ucsi_unregister(itepd->ucsi);
> +	free_irq(client->irq, itepd);
> +	ucsi_destroy(itepd->ucsi);
> +}

[Severity: High]
Is there a race condition during driver removal that could lead to a
use-after-free?

This teardown sequence calls ucsi_unregister() before free_irq().

When ucsi_unregister() is called, it frees the ucsi->connector array. If a
pending hardware interrupt is processed by the threaded IRQ handler
after ucsi_unregister() completes but before free_irq() is called,
itepd_irq_process() will call ucsi_notify_common().

This sequence would result in ucsi_notify_common() accessing the freed
connector array.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ucsi-itepd-feature-v2-0-41943fd5df38@ite.com.tw?part=2