Re: [PATCH v2 2/6] media: nxp: imx8-isi: Add stream ID validation for crossbar routing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver uses the `BIT()` macro on 64-bit stream masks (`source_streams` and `sink_streams`), which will cause undefined behavior on 32-bit platforms if the stream ID exceeds 31.
--

--- Patch [2]: [PATCH v2 2/6] media: nxp: imx8-isi: Add stream ID validation for crossbar routing ---
commit d756aed8e216cd683009378d45743e6351a90389
Author: Guoniu Zhou <guoniu.zhou@oss.nxp.com>

media: nxp: imx8-isi: Add stream ID validation for crossbar routing

This commit adds validation to enforce hardware constraints in the crossbar
routing configuration. It ensures that the SOURCE stream is strictly set to
0, as ISI pipes are hardcoded to stream 0. It also ensures memory input
respects routing limitations to the first pipeline.

> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> index 84871bceb31db..328d08a278eaf 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> @@ -106,8 +106,20 @@ static int __mxc_isi_crossbar_set_routing(struct v4l2_subdev *sd,
>  	if (ret)
>  		return ret;
>  
> -	/* The memory input can be routed to the first pipeline only. */
> +	/*
> +	 * Validate routes against hardware constraints:
> +	 * - SOURCE stream must be 0 (pipes are hardcoded to stream 0)
> +	 * - Memory input can only route to the first pipeline
> +	 */
>  	for_each_active_route(routing, route) {
> +		if (route->source_stream != 0) {

[Severity: High]
This is a pre-existing issue, but while reviewing this stream ID validation,
I noticed a potential bit shift overflow regarding stream IDs in the related
mxc_isi_crossbar_xlate_streams() function:

drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c:mxc_isi_crossbar_xlate_streams() {
    ...
    if (!(source_streams & BIT(route->source_stream)))
        ...
    sink_streams |= BIT(route->sink_stream);
    ...
}

Does this code cause undefined behavior on 32-bit platforms if userspace
provides a V4L2 routing configuration with a sink_stream or source_stream
greater than or equal to 32?

Since the BIT() macro evaluates to a 32-bit value on 32-bit architectures,
shifting it by 32 or more results in a bit shift overflow. This could allow
the compiler to make unsafe optimization assumptions, potentially corrupting
the 64-bit stream mask logic. 

Should this use BIT_ULL() instead?

> +			dev_dbg(xbar->isi->dev,
> +				"route to pipe %u must use source_stream=0, got %u\n",
> +				route->source_pad - xbar->num_sinks,
> +				route->source_stream);
> +			return -ENXIO;
> +		}
> +
>  		if (route->sink_pad == xbar->num_sinks - 1 &&
>  		    route->source_pad != xbar->num_sinks) {
>  			dev_dbg(xbar->isi->dev,
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-isi-v2-0-45845bc5d4fa@oss.nxp.com?part=2