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