Re: [PATCH v2 3/6] media: nxp: imx8-isi: Fix per-stream reference counting for multiplexed streams

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unchecked return value from `media_pad_remote_pad_first()` causes a NULL pointer dereference if the sink pad has no enabled link.
- [Medium] Use of `BIT()` macro for 64-bit stream IDs causes undefined behavior and bitwise truncation on 32-bit platforms.
--

--- Patch [3]: [PATCH v2 3/6] media: nxp: imx8-isi: Fix per-stream reference counting for multiplexed streams ---
commit 976517cc78165ba12439fb004220b58271a0beea
Author: Guoniu Zhou <guoniu.zhou@oss.nxp.com>

media: nxp: imx8-isi: Fix per-stream reference counting for multiplexed streams

The driver uses a single enable_count per input to track the input state.
This fails to properly enable multiple streams from different virtual
channels on the same input pad. This patch tracks per-stream state using
enabled_streams bitmask and enabled_count[] array.

> 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 328d08a278eaf..64576f6bd45c2 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
> @@ -337,6 +337,8 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
>  	struct mxc_isi_crossbar *xbar = to_isi_crossbar(sd);
>  	struct v4l2_subdev *remote_sd;
>  	struct mxc_isi_input *input;
> +	u64 streams_to_enable;
> +	unsigned long stream;
>  	u64 sink_streams;
>  	u32 sink_pad;
>  	u32 remote_pad;
> @@ -350,30 +352,47 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
>  
>  	input = &xbar->inputs[sink_pad];

[Severity: High]
This is a pre-existing issue, but can this lead to a NULL pointer dereference
if the remote link is disabled?

When userspace configures a V4L2 subdev route connecting a source pad to a
sink pad on the crossbar, but leaves the incoming media link to that sink pad
disabled, media_pad_remote_pad_first() will return NULL.

In drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c:
mxc_isi_crossbar_xlate_streams() {
    ...
	pad = media_pad_remote_pad_first(&xbar->pads[sink_pad]);
	sd = media_entity_to_v4l2_subdev(pad->entity);
	if (!sd) {
    ...
}

The media_entity_to_v4l2_subdev() macro immediately dereferences pad to
access pad->entity before any NULL check is performed.

[Severity: Medium]
This is also a pre-existing issue, but might there be bitwise truncation and
undefined behavior on 32-bit platforms when computing sink_streams?

When userspace configures a routing table using VIDIOC_SUBDEV_S_ROUTING with
a sink_stream ID greater than or equal to 32, using BIT() will cause issues
on 32-bit systems where BIT() operates on a 32-bit unsigned long.

In drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c:
mxc_isi_crossbar_xlate_streams() {
    ...
	for_each_active_route(&state->routing, route) {
		...
		sink_streams |= BIT(route->sink_stream);
		sink_pad = route->sink_pad;
	}
    ...
}

Could BIT_ULL() be used instead to safely construct the u64 sink_streams
bitmask?

> -	/*
> -	 * TODO: Track per-stream enable counts to support multiplexed
> -	 * streams.
> -	 */

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