Re: [PATCH v18 23/23] media: i2c: maxim-serdes: add MAX9296A driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: max9296a: undefined behavior from bit shift underflow when data-lane is 0
--

commit 6e4f4e297665312f959a24abeb56d6ec1fb1275e
Author: Cosmin Tanislav <demonsingur@gmail.com>

    media: i2c: maxim-serdes: add MAX9296A driver
    
    Add a new MAX9296A driver that also supports MAX96714, MAX96714F,
    MAX96714R, MAX96716A and MAX96792A.

> diff --git a/drivers/media/i2c/maxim-serdes/max9296a.c b/drivers/media/i2c/maxim-serdes/max9296a.c
> new file mode 100644
> index 0000000000000..7b25b54e63deb
> --- /dev/null
> +++ b/drivers/media/i2c/maxim-serdes/max9296a.c
> @@ -0,0 +1,1461 @@
[ ... ]
> +static int max9296a_init_phy(struct max_des *des, struct max_des_phy *phy)
> +{
[ ... ]
> +	for (i = 0; i < num_hw_data_lanes ; i++) {
> +		unsigned int map;
> +
> +		if (i < num_data_lanes)
> +			map = phy->mipi.data_lanes[i] - 1;
> +		else
> +			map = ffz(used_data_lanes);
> +
> +		val |= map << (i * 2);
> +		used_data_lanes |= BIT(map);
> +	}

[Severity: Medium]
If the device tree configures a data lane with index 0, could this underflow?

The V4L2 core parses the data-lanes property from the device tree, which can
contain a 0 index depending on the DT configuration. The driver's endpoint
parsing helper max_des_parse_src_dt_endpoint() verifies that lane indices are
<= num_data_lanes, but doesn't appear to explicitly check if the index is 0.

Since map is unsigned, phy->mipi.data_lanes[i] - 1 would underflow to
UINT_MAX when the index is 0. This could result in unpredictable state
corruption or a kernel panic via UBSAN when performing the bitwise shifts.

[ ... ]
> +	for (i = 0, val = 0; i < num_data_lanes; i++) {
> +		unsigned int map;
> +
> +		if (!phy->mipi.lane_polarities[i + 1])
> +			continue;
> +
> +		/*
> +		 * The numbers inside the data_lanes array specify the hardware
> +		 * lane each logical lane maps to.
> +		 * If polarity is set for the physical lanes, retrieve the
> +		 * physical lane matching the logical lane from data_lanes.
> +		 * Otherwise, when polarity is set for the logical lanes
> +		 * the index of the polarity can be used.
> +		 */
> +
> +		if (priv->info->polarity_on_physical_lanes)
> +			map = phy->mipi.data_lanes[i] - 1;
> +		else
> +			map = i;
> +
> +		val |= BIT(map);
> +	}

[Severity: Medium]
Similarly, does this code also trigger an invalid shift when evaluating
polarities if the data-lane index is 0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-gmsl2-3_serdes-v18-0-43117818f499@analog.com?part=23