Re: [media-ci] Re: [PATCH v12 3/3] media: nxp: Add i.MX95 CSI pixel formatter v4l2 driver

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

On Thu, 16 Jul 2026 07:59:45 -0500
Frank Li <Frank.li@oss.nxp.com> wrote:

> On Thu, Jul 16, 2026 at 07:20:52AM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> > - [High] Missing NULL checks for the return value of `v4l2_subdev_state_get_format()` allow userspace to trigger a kernel panic.
> > - [High] The driver caches a pointer to the remote subdevice when bound, but fails to implement an `.unbind` callback to clear it, leading to a Use-After-Free.
> > - [Medium] Mixing `goto`-based error handling with scope-based cleanup (`__free` and `guard`) in the same function violates kernel cleanup guidelines.
> > - [Medium] `csi_formatter_subdev_set_fmt` partially modifies the subdevice state but returns `-EINVAL` if the stream has no active route.
> > - [Medium] The `csi_formatter_read` wrapper ignores the return value of `regmap_read`, potentially leading to uninitialized stack variable usage.
> > --
> >
> > --- Patch [3]: [PATCH v12 3/3] media: nxp: Add i.MX95 CSI pixel formatter v4l2 driver ---
> > commit 0cff6156940653a6a408d93ee678b339b7218ebb
> > Author: Guoniu Zhou <guoniu.zhou@nxp.com>
> > Subject: media: nxp: Add i.MX95 CSI pixel formatter v4l2 driver
> >
> > The CSI pixel formatter is a module found on i.MX95 used to reformat
> > packet info, pixel and non-pixel data from CSI-2 host controller to
> > match Pixel Link(PL) definition. Add data formatting support.
> >  
> > > diff --git a/drivers/media/platform/nxp/imx95-csi-formatter.c b/drivers/media/platform/nxp/imx95-csi-formatter.c
> > > new file mode 100644
> > > index 0000000000000..cea60327c9729
> > > --- /dev/null
> > > +++ b/drivers/media/platform/nxp/imx95-csi-formatter.c  
> >
> > [ ... ]
> >  
> > > +static int csi_formatter_subdev_enum_mbus_code(struct v4l2_subdev *sd,
> > > +					       struct v4l2_subdev_state *sd_state,
> > > +					       struct v4l2_subdev_mbus_code_enum *code)
> > > +{
> > > +	if (code->pad == CSI_FORMATTER_PAD_SOURCE) {
> > > +		struct v4l2_mbus_framefmt *fmt;
> > > +
> > > +		if (code->index > 0)
> > > +			return -EINVAL;
> > > +
> > > +		fmt = v4l2_subdev_state_get_format(sd_state, code->pad,
> > > +						   code->stream);
> > > +		code->code = fmt->code;  
> >
> > [Severity: High]
> > Can this cause a NULL pointer dereference?
> >
> > The V4L2 core does not validate the stream ID provided by userspace in
> > VIDIOC_SUBDEV_ENUM_MBUS_CODE against the active routing table before calling
> > this function. If a user passes an invalid stream ID,
> > v4l2_subdev_state_get_format() returns NULL, which is then unconditionally
> > dereferenced here.  
> 
> Is it true?

Yes and no ;-)

Technically yes, v4l2_subdev_state_get_format() can return NULL on
several conditions if it can't find a state/pad match or if
state/pad is invalid. However, the call_enum_mbus_code() is always
called via a wrapper, which runs check_state() first, validating
such error conditions.

-

That's said, I bet other static analyzers would also produce the
same warning, as the check logic is indirect, using some complex
macros.

We need to create a custom LLM media prompt to prevent such
warnings from LLMs (or reduce the complexity of the wrapper code).
Perhaps improving comments at v4l2-subdev could also help.

I already pinged earlier today a Sashiko developer to see if we
can get some help to write a prompt.


Thanks,
Mauro