Re: [PATCH] media: nxp: isi: Check whether pad is non-NULL before access

[Kieran Bingham <kieran.bingham@ideasonboard.com>]

Quoting Laurent Pinchart (2023-12-03 17:00:57)
> On Sat, Dec 02, 2023 at 07:44:47AM +0100, Marek Vasut wrote:
> > On 12/2/23 03:27, Fabio Estevam wrote:
> > > On Fri, Dec 1, 2023 at 12:06 PM Marek Vasut <marex@denx.de> wrote:
> > > 
> > >>          pad = media_pad_remote_pad_first(&xbar->pads[sink_pad]);
> > >> -       sd = media_entity_to_v4l2_subdev(pad->entity);
> > >> +       if (!pad) {
> > >> +               dev_dbg(xbar->isi->dev,
> > > 
> > > Maybe dev_err() here instead?
> > 
> > That dev_dbg() is aligned with the prints in the rest of the function 
> > and it's not like kernel should do dev_err() into kernel log every time 
> > userspace does something wrong.
> 
> We usually use dev_dbg() for errors that can be easily triggered from
> userspace, to avoid giving unpriviledged processes an easy way to flood
> the kernel log.

When this call returns it will also report a dev_err "Failed to enable
pipe":

[   56.806321] mxc-isi 32e00000.isi: no pad connecxted to crossbar input 0
[   56.813655] mxc-isi 32e00000.isi: Failed to enable pipe 0

Personally - I'd make this print dev_err so the /reason/ we failed to
enable the pipe is also stated.

But I can confirm this is a bug and fairly easily trigged by attempting
to capture from the ISI through the libcamera simple pipeline handler,
which (I believe) does not do any route handling at present.

Which ever log level:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


And this needs to go to stable, as this caused a null-pointer-deref for
me on a v6.6 tree.

--
Kieran


> 
> > > Anyway:
> > > 
> > > Reviewed-by: Fabio Estevam <festevam@gmail.com>
> > 
> > Thanks !
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel