Hi Dan,
On Mon, 2022-03-07 at 12:44 +0300, Dan Carpenter wrote:
> Caution: EXT Email
>
> On Fri, Mar 04, 2022 at 03:51:16PM +0000, Mirela Rabulea wrote:
> > Hi,
> >
> > On Tue, 2022-03-01 at 15:42 +0300, Dan Carpenter wrote:
> > > Hello Mirela Rabulea,
> > >
> > > The patch 2db16c6ed72c: "media: imx-jpeg: Add V4L2 driver for
> > > i.MX8
> > > JPEG Encoder/Decoder" from Mar 11, 2021, leads to the following
> > > Smatch static checker warning:
> > >
> > > drivers/media/platform/imx-jpeg/mxc-jpeg.c:1070
> > > mxc_jpeg_queue_setup()
> > > warn: potential user controlled iterator 'i' (array size
> > > 2 vs
> > > 7)
> > >
> > > drivers/media/platform/imx-jpeg/mxc-jpeg.c
> > > 1053 static int mxc_jpeg_queue_setup(struct vb2_queue *q,
> > > 1054 unsigned int *nbuffers,
> > > 1055 unsigned int *nplanes,
> > > 1056 unsigned int sizes[],
> > > 1057 struct device
> > > *alloc_ctxs[])
> > > 1058 {
> > > 1059 struct mxc_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > > 1060 struct mxc_jpeg_q_data *q_data = NULL;
> > > 1061 int i;
> > > 1062
> > > 1063 q_data = mxc_jpeg_get_q_data(ctx, q->type);
> > > 1064 if (!q_data)
> > > 1065 return -EINVAL;
> > > 1066
> > > 1067 /* Handle CREATE_BUFS situation - *nplanes != 0
> > > */
> > > 1068 if (*nplanes) {
> > > 1069 for (i = 0; i < *nplanes; i++) {
> > > --> 1070 if (sizes[i] < q_data-
> > > >sizeimage[i])
> > >
> > > Smatch thinks "*nplanes" is controlled by the user in
> > > vb2_create_bufs()
> > > and it can be up to VIDEO_MAX_PLANES(8). Meanwhile the q_data-
> > > > sizeimage[]
> > > array only has MXC_JPEG_MAX_PLANES(2) elements so this looks to
> > > be an
> > > out of bounds access.
> >
> > Thanks for pointing this out. I tried to run smatch (for the first
> > time), and I do not get this warning reported. I'm wondering what
> > am I
> > missing?
> >
> > mirela@fsr-ub1664-134:/workssd/linux-next$
> > /workssd/smatch/smatch_scripts/kchecker drivers/media/platform/imx-
> > jpeg/
> > CHECK scripts/mod/empty.c
> > CALL scripts/checksyscalls.sh
> > CALL scripts/atomic/check-atomics.sh
> > CHECK arch/arm64/kernel/vdso/vgettimeofday.c
> > CHECK drivers/media/platform/imx-jpeg/mxc-jpeg-hw.c
> > CC [M] drivers/media/platform/imx-jpeg/mxc-jpeg.o
> > CHECK drivers/media/platform/imx-jpeg/mxc-jpeg.c
> > LD [M] drivers/media/platform/imx-jpeg/mxc-jpeg-encdec.o
> > mirela@fsr-ub1664-134:/workssd/linux-next$
> >
> > I can induce some errors in the source code, and then I also see
> > CHECK
> > errors.
> >
> > I have built the kernel database with
> > smatch/smatch_scripts/build_kernel_data.sh before runing kchecker.
> >
>
> Oh, sorry. This check hasn't been published yet it's something I've
> just started working on. If the checker is wrong just ignore it, but
> could you give me a hint so I can improve the check?
The check is good, I sent a patch. I see some drivers do handle better
the *nplanes situation in queue_setup. Others will fail the check
(example:drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c)
Thanks,
Mirela
>
> regards,
> dan carpenter
>
