[PATCH] media: atomisp: reject frame dimensions that overflow the size calculation

[PATCH] media: atomisp: reject frame dimensions that overflow the size calculation

[Doruk Tan Ozturk]

ia_css_frame_allocate() computes the allocation size for a frame in the
frame_init_*_planes() helpers as width/padded_width * height *
bytes-per-pixel * plane-count. frame->data_bytes is a u32 and the
helpers use plain unsigned arithmetic with no overflow check; the
result is then passed to hmm_alloc().

width, height and padded_width are user-controlled: the
v4l2_framebuffer ioctl path reaches this via
atomisp_v4l2_framebuffer_to_css_frame(), which forwards arg->fmt.width
and arg->fmt.height straight into ia_css_frame_allocate() and then
copies arg->fmt.sizeimage bytes into the resulting buffer with
hmm_store(). A sufficiently large width/height makes the size
calculation wrap, so hmm_alloc() returns an undersized buffer that the
following copy overflows.

This is a memory-safety bug confined to the ISP-private hmm buffer
object (a separate bo allocator), not the kmalloc slab; it corrupts
ISP device memory rather than granting a kmalloc-heap primitive.

Reject up front, in ia_css_frame_allocate() (which already returns
-EINVAL for bad arguments), any dimensions whose worst-case byte count
cannot be represented in the u32 data_bytes field. The factor 16
conservatively bounds the largest per-pixel multiplier across all
supported formats (up to 6 planes, or 3x RGB planes with up to 4 bytes
per element).

Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).
Found by static analysis; not yet runtime-reproduced (Intel atomisp
hardware required).

Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
 .../atomisp/pci/runtime/frame/src/frame.c     | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
index 8614efc28b19..dea7b11ccc90 100644
--- a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
+++ b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
@@ -6,6 +6,7 @@
 
 #include <linux/bitops.h>
 #include <linux/math.h>
+#include <linux/overflow.h>
 
 #include "assert_support.h"
 #include "atomisp_internal.h"
@@ -106,10 +107,30 @@ int ia_css_frame_allocate(struct ia_css_frame **frame,
 				      unsigned int raw_bit_depth)
 {
 	int err = 0;
+	u32 bytes;
 
 	if (!frame || width == 0 || height == 0)
 		return -EINVAL;
 
+	/*
+	 * The frame_init_*_planes() helpers compute frame->data_bytes (a u32)
+	 * as width/padded_width * height * bytes-per-pixel * plane-count using
+	 * unmodulated unsigned arithmetic, with no overflow check, and the
+	 * result is then handed to hmm_alloc(). width, height and padded_width
+	 * are user-controlled (e.g. via the v4l2_framebuffer ioctl path in
+	 * atomisp_v4l2_framebuffer_to_css_frame()). A large width/height pair
+	 * makes the size calculation wrap, producing an undersized hmm buffer
+	 * that a subsequent copy then overflows.
+	 *
+	 * Reject up front any dimensions whose worst-case byte count cannot be
+	 * represented in the u32 data_bytes field. The factor 16 conservatively
+	 * bounds the largest per-pixel multiplier across all supported formats
+	 * (up to 6 planes / 3x RGB planes with up to 4 bytes per element).
+	 */
+	if (check_mul_overflow(max(width, padded_width), height, &bytes) ||
+	    check_mul_overflow(bytes, 16u, &bytes))
+		return -EINVAL;
+
 	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
 			    "ia_css_frame_allocate() enter: width=%d, height=%d, format=%d, padded_width=%d, raw_bit_depth=%d\n",
 			    width, height, format, padded_width, raw_bit_depth);
-- 
2.53.0

Re: [PATCH] media: atomisp: reject frame dimensions that overflow the size calculation

[Dan Carpenter]

On Sat, Jun 27, 2026 at 08:55:56AM +0200, Doruk Tan Ozturk wrote:
> @@ -106,10 +107,30 @@ int ia_css_frame_allocate(struct ia_css_frame **frame,
>  				      unsigned int raw_bit_depth)
>  {
>  	int err = 0;
> +	u32 bytes;
>  
>  	if (!frame || width == 0 || height == 0)
>  		return -EINVAL;
>  
> +	/*
> +	 * The frame_init_*_planes() helpers compute frame->data_bytes (a u32)
> +	 * as width/padded_width * height * bytes-per-pixel * plane-count using
> +	 * unmodulated unsigned arithmetic, with no overflow check, and the
> +	 * result is then handed to hmm_alloc(). width, height and padded_width
> +	 * are user-controlled (e.g. via the v4l2_framebuffer ioctl path in
> +	 * atomisp_v4l2_framebuffer_to_css_frame()). A large width/height pair
> +	 * makes the size calculation wrap, producing an undersized hmm buffer
> +	 * that a subsequent copy then overflows.
> +	 *
> +	 * Reject up front any dimensions whose worst-case byte count cannot be
> +	 * represented in the u32 data_bytes field. The factor 16 conservatively
> +	 * bounds the largest per-pixel multiplier across all supported formats
> +	 * (up to 6 planes / 3x RGB planes with up to 4 bytes per element).
> +	 */

AI likes to add comments to every line which it changes.  That information
is already there in the commit message.  Everyone knows what
check_mul_overflow() is for.

It's like the ToS when you buy software, there might be some interesting
information in there but we'll never know because it's too much.  The same
thing applies to comments.  Don't comment on things which are obvious.

(You might wonder why, if this is obvious, wasn't it done in the original
code.  drivers/staging/ is for code which is obviously bad).

regards,
dan carpenter

> +	if (check_mul_overflow(max(width, padded_width), height, &bytes) ||
> +	    check_mul_overflow(bytes, 16u, &bytes))
> +		return -EINVAL;
> +
>  	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
>  			    "ia_css_frame_allocate() enter: width=%d, height=%d, format=%d, padded_width=%d, raw_bit_depth=%d\n",
>  			    width, height, format, padded_width, raw_bit_depth);
> -- 
> 2.53.0