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
