Re: [PATCH v6 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:
- [Medium] Accessing MIPI CSI-2 specific union fields without verifying the parsed bus type.
- [Medium] `it6625_set_fmt()` fails to update `format->format.colorspace` for the `V4L2_SUBDEV_FORMAT_ACTIVE` branch.
- [Medium] `it6625_s_edid` violates the V4L2 `VIDIOC_SUBDEV_S_EDID` specification for `start_block` bounds handling.
--

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

This commit adds a new V4L2 subdevice driver for the ITE IT6625 and IT6626
HDMI-to-MIPI CSI-2 bridge chips. The driver supports configuring the bridge
over I2C, handles EDID reading and writing, exposes an HDMI CEC adapter,
and supports DV timings and HPD control.

diff --git a/drivers/media/i2c/it6625.c b/drivers/media/i2c/it6625.c
--- a/drivers/media/i2c/it6625.c
+++ b/drivers/media/i2c/it6625.c
[ ... ]
> +	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> +		struct v4l2_mbus_framefmt *fmt;
> +
> +		fmt = v4l2_subdev_state_get_format(sd_state, format->pad);
> +		fmt->code = format->format.code;
> +		fmt->colorspace = format_to_colorspace(it6625_formats[ret].csi_format);
> +		format->format.colorspace = fmt->colorspace;
> +		v4l2_dbg(1, debug, sd, "%s: try format code = 0x%x",
> +			 __func__, format->format.code);
> +		return 0;
> +	}
> +
> +	scoped_guard(mutex, &it6625->it6625_lock) {
> +		it6625->csi_format = it6625_formats[ret].csi_format;
> +		it6625->mbus_fmt_code = format->format.code;
> +		it6625_enable_stream_locked(it6625, false);
> +		it6625_set_mipi_config_locked(it6625, it6625->csi_format);
> +	}
> +
> +	return 0;
> +}

[Severity: Medium]
Does this branch need to update format->format.colorspace before returning?

The V4L2_SUBDEV_FORMAT_TRY branch above updates format->format.colorspace,
but the active format branch only updates the internal hardware state
variables. This leaves the userspace format object with the old colorspace
value that was populated earlier in the function by it6625_get_fmt().

[ ... ]
> +static int it6625_s_edid(struct v4l2_subdev *sd,
> +			 struct v4l2_subdev_edid *edid)
> +{
[ ... ]
> +	memset(edid->reserved, 0, sizeof(edid->reserved));
> +
> +	if (edid->blocks > EDID_NUM_BLOCKS_MAX) {
> +		v4l2_err(sd, "too many edid blocks: %d", edid->blocks);
> +		edid->blocks = EDID_NUM_BLOCKS_MAX;
> +		return -E2BIG;
> +	}
> +
> +	it6625_disable_hpd(it6625);

[Severity: Medium]
Does this logic correctly handle out-of-bounds start blocks according to the
V4L2 specification?

If start_block is out of bounds, the spec expects blocks to be set to 0.
Additionally, if start_block + blocks exceeds the maximum, shouldn't blocks
be clamped to the remaining capacity (EDID_NUM_BLOCKS_MAX - edid->start_block)
before returning -E2BIG?

[ ... ]
> +static int it6625_parse_endpoint(struct it6625 *it6625)
> +{
[ ... ]
> +	if (endpoint.bus_type == V4L2_MBUS_CSI2_CPHY &&
> +	    it6625->chip_type != IT6626_CHIP) {
> +		dev_err(dev, "IT6625 does not support C-PHY, only IT6626 does");
> +		v4l2_fwnode_endpoint_free(&endpoint);
> +		return -EINVAL;
> +	}
> +
> +	max_lanes = (endpoint.bus_type == V4L2_MBUS_CSI2_CPHY) ? 3 : 4;
> +
> +	if (endpoint.bus.mipi_csi2.num_data_lanes == 0 ||
> +	    endpoint.bus.mipi_csi2.num_data_lanes > max_lanes) {
> +		dev_err(dev,
> +			"invalid number of CSI data lanes: %u (max %u for this bus type)",
> +			endpoint.bus.mipi_csi2.num_data_lanes, max_lanes);

[Severity: Medium]
Can we read from the mipi_csi2 union member here without first verifying the
parsed bus type?

If the device tree is misconfigured and causes v4l2_fwnode_endpoint_alloc_parse()
to infer a different bus type (like V4L2_MBUS_PARALLEL), reading from the
mipi_csi2 struct could yield arbitrary data for the lane count.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-upstream-it6625-it6626-v6-0-6117d4d96574@ite.com.tw?part=2