Re: [PATCH v17 20/22] media: i2c: maxim-serdes: add MAX96724 driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] TOCTOU race in stream enablement leads to a kernel stack out-of-bounds read and info leak via Virtual Channel mapping.
--

commit 09e380cc5beec37cc922ac65a1a89a282d99562a
Author: Cosmin Tanislav <demonsingur@gmail.com>
Subject: media: i2c: maxim-serdes: add MAX96724 driver

This commit adds a new MAX96724 driver that supports MAX96712, MAX96724F
and MAX96724R quad deserializers. It integrates the devices with the common
deserializer framework.

> diff --git a/drivers/media/i2c/maxim-serdes/max96724.c b/drivers/media/i2c/maxim-serdes/max96724.c
> new file mode 100644
> index 0000000000000..458e78f285111
> --- /dev/null
> +++ b/drivers/media/i2c/maxim-serdes/max96724.c

[ ... ]

> +static int max96724_set_pipe_remap(struct max_des *des,
> +				   struct max_des_pipe *pipe,
> +				   unsigned int i,
> +				   struct max_des_remap *remap)
> +{
> +	struct max96724_priv *priv = des_to_priv(des);
> +	struct max_des_phy *phy = &des->phys[remap->phy];
> +	unsigned int phy_id = max96724_phy_id(des, phy);
> +	unsigned int index = pipe->index;
> +	int ret;

[ ... ]

> +	/* Set destination Data Type and Virtual Channel. */
> +	/* TODO: implement extended Virtual Channel. */
> +	ret = regmap_write(priv->regmap, MAX96724_MIPI_TX14(index, i),
> +			   FIELD_PREP(MAX96724_MIPI_TX14_MAP_DST_DT,
> +				      remap->to_dt) |
> +			   FIELD_PREP(MAX96724_MIPI_TX14_MAP_DST_VC,
> +				      remap->to_vc));

[Severity: High]
Can a TOCTOU race in stream enablement lead to a kernel stack out-of-bounds
read and an information leak here?

When max_des_update_streams() is called, it fetches the remote sensor's frame
descriptor twice: first in max_des_populate_remap_context(), and later in
max_des_get_pipe_remaps(). If a concurrent V4L2 ioctl on the sensor changes
the Virtual Channel (vc) ID between these calls, a race occurs.

During the second pass, max_des_get_pipe_remaps() invokes the getter without
re-validating the newly fetched src_vc_id:

max_des_get_pipe_remaps() {
    ...
    ret = max_des_get_src_dst_vc_id(context, pipe->index, hw.phy->index,
                                    src_vc_id, &dst_vc_id);
}

Inside max_des_get_src_dst_vc_id(), this results in an out-of-bounds read
on the stack-allocated context array if src_vc_id is modified to bypass
bitmask checks (e.g., if it is a multiple of BITS_PER_LONG):

max_des_get_src_dst_vc_id() {
    ...
    *dst_vc_id = context->vc_ids_map[pipe_id][phy_id][src_vc_id];
}

The out-of-bounds stack data is then assigned to remap->to_vc and passed to
this hardware driver. By programming remap->to_vc into the hardware register
here, is it possible that up to 2 bits of kernel stack memory per race could
be leaked onto the observable MIPI CSI-2 bus?

> +	if (ret)
> +		return ret;
> +
> +	/* Set destination PHY. */
> +	return regmap_update_bits(priv->regmap, MAX96724_MIPI_TX45(index, i),
> +				  MAX96724_MIPI_TX45_MAP_DPHY_DEST(i),
> +				  field_prep(MAX96724_MIPI_TX45_MAP_DPHY_DEST(i),
> +					     phy_id));
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-gmsl2-3_serdes-v17-0-002499e534e8@analog.com?part=20