Re: [bug report] media: imx-jpeg: Add V4L2 driver for i.MX8 JPEG Encoder/Decoder

[Mirela Rabulea <mirela.rabulea@nxp.com>]

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.

Thanks,
Mirela

> 
>     1071                                 return -EINVAL;
>     1072                 }
>     1073                 return 0;
>     1074         }
>     1075
>     1076         /* Handle REQBUFS situation */
>     1077         *nplanes = q_data->fmt->colplanes;
>     1078         for (i = 0; i < *nplanes; i++)
>     1079                 sizes[i] = q_data->sizeimage[i];
>     1080
>     1081         return 0;
>     1082 }
> 
> regards,
> dan carpenter