media: imx-jpeg: avoid array overflow
PATCH 1/1 media: imx-jpeg: avoid array overflow
From: Arnd Bergmann <[email protected]>
gcc-9 (unlike newer versions) reports a possible array overflow in mxc_jpeg_dec_irq():
drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c: In function 'mxc_jpeg_dec_irq':drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c:641:28: error: array subscript 2 is above array bounds of 'u32[2]' {aka 'unsigned int[2]'} [-Werror=array-bounds]
641 | size += q_data->sizeimage[i];
| ~~~~~~~~~~~~~~~~~^~~drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c:641:28: error: array subscript 3 is above array bounds of 'u32[2]' {aka 'unsigned int[2]'} [-Werror=array-bounds]
The compiler clearly deduces that fmt->mem_planes is at least '2' if this code line is reached, and that fmt->comp_planes must be at least one more for the loop to make sense. However, this does not actually seem to be the case in the initialized values, so I would guess that this part of the function is never reached in practice.
As a workaround, add a compile-time condition that skips any out-of-range array indices.
--- drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c +2 -1
@@ -638,7 +638,8 @@ static u32 mxc_jpeg_get_plane_size(struct mxc_jpeg_q_data *q_data, u32 plane_no) size = q_data->sizeimage[fmt->mem_planes -1 ];for (i = fmt->mem_planes; i < fmt->comp_planes; i++) - size += q_data->sizeimage[i]; +if (i <MXC_JPEG_MAX_PLANES ) + size += q_data->sizeimage[i];return size; }
Signature · 1 line
2.39.1
Hi Arnd,
Thank you for the patch.
Quoted 31 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
gcc-9 (unlike newer versions) reports a possible array overflow in
mxc_jpeg_dec_irq():
drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c: In function 'mxc_jpeg_dec_irq':drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c:641:28: error: array subscript 2 is above array bounds of 'u32[2]' {aka 'unsigned int[2]'} [-Werror=array-bounds]641 | size += q_data->sizeimage[i]; | ~~~~~~~~~~~~~~~~~^~~
drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c:641:28: error: array subscript 3 is above array bounds of 'u32[2]' {aka 'unsigned int[2]'} [-Werror=array-bounds]The compiler clearly deduces that
fmt->mem_planesis at least '2' if this code line is reached, and thatfmt->comp_planesmust be at least one more for the loop to make sense. However, this does not actually seem to be the case in the initialized values, so I would guess that this part of the function is never reached in practice.As a workaround, add a compile-time condition that skips any out-of-range array indices.
Fixes ccc9f1db9c6b ("media: imx-jpeg: Support contiguous and non contiguous format")Signed-off-by Arnd Bergmann <[email protected]>--- drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c +0
-0
drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c static u32 mxc_jpeg_get_plane_size(struct mxc_jpeg_q_data *q_data, u32 plane_no) · line 642
Quoted 6 lines
drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c +2
-1@@ -638,7 +638,8 @@ static u32 mxc_jpeg_get_plane_size(struct mxc_jpeg_q_data *q_data, u32 plane_no) size = q_data->sizeimage[fmt->mem_planes -1 ];for (i = fmt->mem_planes; i < fmt->comp_planes; i++) - size += q_data->sizeimage[i]; +if (i <MXC_JPEG_MAX_PLANES ) + size += q_data->sizeimage[i];
There's a risk someone will try to optimize this later, realizing that the check isn't needed, and we'll get the warning back. A comment would thus be useful.
Also, how about writing it as
for (i = fmt->mem_planes; i < min(fmt->comp_planes, MXC_JPEG_MAX_PLANES); i++) size += q_data->sizeimage[i];
Quoted 3 lines
drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c +0
-0return size; }
Signature · 3 lines
Regards, Laurent Pinchart