Re: [PATCH v2 1/6] media: nxp: imx8-isi: Fix stream ID validation bypass in crossbar routing

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Undefined behavior from bit shift overflow when using BIT() macro on user-controlled stream IDs on 32-bit architectures.
- [High] Missing V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX flag in routing validation allows stream mixing, violating driver assumptions and causing pipeline state corruption.
--

--- Patch [1]: [PATCH v2 1/6] media: nxp: imx8-isi: Fix stream ID validation bypass in crossbar routing ---
commit c48b83ab851ce7535e8d8249fb85d35c57bb63e1
Author: Guoniu Zhou <guoniu.zhou@oss.nxp.com>

media: nxp: imx8-isi: Fix stream ID validation bypass in crossbar routing

The crossbar routing validation has a bug where it validates the wrong
routing table, allowing userspace to bypass validation entirely. This
commit fixes it by validating the routing table that will actually be
applied.

> 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 c580c831972ec..84871bceb31db 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> @@ -107,7 +107,7 @@ static int __mxc_isi_crossbar_set_routing(struct v4l2_subdev *sd,
>  		return ret;

[Severity: High]
This isn't a bug introduced by this patch, but does the routing validation
here miss the V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX flag?

The validation in __mxc_isi_crossbar_set_routing() does not pass this
flag:

imx8-isi-crossbar.c:__mxc_isi_crossbar_set_routing() {
    ...
	ret = v4l2_subdev_routing_validate(sd, routing,
					   V4L2_SUBDEV_ROUTING_NO_N_TO_1);
    ...
}

This allows stream mixing, violating an explicit assumption in
mxc_isi_crossbar_xlate_streams(), where the driver silently overwrites
sink_pad for matching routes:

imx8-isi-crossbar.c:mxc_isi_crossbar_xlate_streams() {
    ...
	/*
	 * Translate the source pad and streams to the sink side. The routing
	 * validation forbids stream merging, so all matching entries in the
	 * routing table are guaranteed to have the same sink pad.
	 */
    ...
		sink_streams |= BIT(route->sink_stream);
		sink_pad = route->sink_pad;
    ...
}

If userspace configures a routing table via VIDIOC_SUBDEV_S_ROUTING with
multiple routes from different sink pads ending at the same source pad,
mxc_isi_crossbar_enable_streams() will only enable streams on the last
matched remote subdevice, leaving others disabled and leading to hardware
pipeline hangs.

>  
>  	/* The memory input can be routed to the first pipeline only. */
> -	for_each_active_route(&state->routing, route) {
> +	for_each_active_route(routing, route) {

[Severity: High]
This is a pre-existing issue, but can using the BIT() macro on
user-controlled stream IDs cause undefined behavior on 32-bit architectures?

The core framework limits stream IDs to 63 (V4L2_SUBDEV_MAX_STREAM_ID), but
the driver uses BIT() on these IDs in mxc_isi_crossbar_xlate_streams():

imx8-isi-crossbar.c:mxc_isi_crossbar_xlate_streams() {
    ...
		if (route->source_pad != source_pad ||
		    !(source_streams & BIT(route->source_stream)))
			continue;

		sink_streams |= BIT(route->sink_stream);
    ...
}

On 32-bit architectures, unsigned long is 32 bits. If userspace provides a
routing table via VIDIOC_SUBDEV_S_ROUTING with stream IDs >= 32, this
causes a shift out of bounds. This can result in incorrect bitmasks,
causing the driver to start or stop the wrong hardware streams and hang
the pipeline.

>  		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=1