Re: [PATCH v8 2/2] media: i2c: add driver for ITE IT6625/IT6626

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unconditional return of IRQ_HANDLED in the threaded IRQ handler leads to an IRQ storm if the I2C read fails or the interrupt is spurious.
- [High] The `fps_from_bt_timings()` function computes the denominator as `V4L2_DV_BT_FRAME_HEIGHT(t) * V4L2_DV_BT_FRAME_WIDTH(t)` using 32-bit unsigned arithmetic. Malicious or manipulated timing parameters can cause this product to exactly equal `2^32`, wrapping around to `0` and causing a kernel panic via division-by-zero.
- [Medium] it6625_initial_setup() races with userspace CEC transmissions because it is called after adapter registration and lacks locking.
--

--- Patch [2]: [PATCH v8 2/2] media: i2c: add driver for ITE IT6625/IT6626 ---
commit 053fb4f690d4c8eec60373a939622584d5a96c81
Author: Hermes Wu <Hermes.wu@ite.com.tw>

media: i2c: add driver for ITE IT6625/IT6626

Adds a V4L2 subdevice driver for the ITE IT6625/IT6626 HDMI-to-MIPI CSI-2
bridge chips. The bridge is configured over I2C, exposes an HDMI CEC adapter,
and supports EDID and video timings configuration.

> diff --git a/drivers/media/i2c/it6625.c b/drivers/media/i2c/it6625.c
> new file mode 100644
> index 0000000000000..fd1d0c85e4f3a
> --- /dev/null
> +++ b/drivers/media/i2c/it6625.c
[ ... ]
> +static inline unsigned int fps_from_bt_timings(const struct v4l2_bt_timings *t)
> +{
> +	if (!V4L2_DV_BT_FRAME_HEIGHT(t) || !V4L2_DV_BT_FRAME_WIDTH(t))
> +		return 0;
> +
> +	return DIV_ROUND_CLOSEST((unsigned int)t->pixelclock,
> +				 V4L2_DV_BT_FRAME_HEIGHT(t) *
> +				 V4L2_DV_BT_FRAME_WIDTH(t));

[Severity: High]
Can the denominator calculation overflow and cause a divide-by-zero panic?

The width and height fields are 32-bit unsigned integers. If manipulated
timing parameters or malicious HDMI signals provide large blanking intervals
such that width is 65536 and height is 65536, the check for zero passes.

However, the 32-bit product (65536 * 65536) overflows to exactly 0, which is
then passed as the divisor to the DIV_ROUND_CLOSEST macro.

> +}
[ ... ]
> +static irqreturn_t it6625_irq_handler(int unused, void *data)
> +{
> +	struct it6625 *it6625 = data;
> +
> +	it6625_interrupt_handler(it6625);
> +	return IRQ_HANDLED;

[Severity: High]
Could this lead to an interrupt storm if the I2C read fails or the interrupt
is spurious?

If it6625_read_byte() fails and returns a negative value in
it6625_interrupt_handler(), it returns early without clearing the hardware
interrupt source:

static int it6625_interrupt_handler(struct it6625 *it6625)
{
	...
	val = it6625_read_byte(it6625, REG_MCU_INTERRUPT);
	if (val <= 0)
		return 0;

Since it6625_irq_handler() unconditionally returns IRQ_HANDLED, the kernel's
spurious IRQ detector is bypassed. If the IRQ line remains asserted due to
the early return, this might cause an infinite loop and soft-lockup.

> +}
[ ... ]
> +static int it6625_probe(struct i2c_client *client)
> +{
[ ... ]
> +	err = cec_register_adapter(it6625->cec_adap, &client->dev);
> +	if (err < 0) {
> +		dev_err(it6625->dev, "%s: failed to register the cec device", __func__);
> +		cec_delete_adapter(it6625->cec_adap);
> +		it6625->cec_adap = NULL;
> +		goto err_clean_hdl;
> +	}
> +
> +	it6625_debugfs_init(it6625, client);
> +
> +	it6625_initial_setup(it6625);

[Severity: Medium]
Is there a race condition between it6625_initial_setup() and userspace CEC
transmissions?

By calling cec_register_adapter() before it6625_initial_setup(), the CEC
device node becomes accessible to userspace. A process or udev rule could
immediately open the node and issue a transmission, which sets
B_CEC_SEND_DATA in REG_HOST_CTRL_INT.

Concurrently, it6625_initial_setup() modifies REG_HOST_CTRL_INT without
holding it6625_lock and waits for it to become 0x00. If both bits are set,
the wait might stall waiting for the long CEC transmission to finish, causing
a timeout.

> +	it6625_v4l2_sd_ctrl_update(sd);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-upstream-it6625-it6626-v8-0-2c3b0f9524fc@ite.com.tw?part=2