Linux Media Controller development
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: Doruk Tan Ozturk <doruk@0sec.ai>
Cc: Hans de Goede <hansg@kernel.org>,
	Andy Shevchenko <andy@kernel.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	linux-media@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] media: atomisp: reject frame dimensions that overflow the size calculation
Date: Sat, 27 Jun 2026 10:12:34 +0300	[thread overview]
Message-ID: <aj934iSujRmfJ3St@stanley.mountain> (raw)
In-Reply-To: <20260627065556.88673-1-doruk@0sec.ai>

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

      reply	other threads:[~2026-06-27  7:12 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-27  6:55 [PATCH] media: atomisp: reject frame dimensions that overflow the size calculation Doruk Tan Ozturk
2026-06-27  7:12 ` Dan Carpenter [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aj934iSujRmfJ3St@stanley.mountain \
    --to=error27@gmail.com \
    --cc=andy@kernel.org \
    --cc=doruk@0sec.ai \
    --cc=gregkh@linuxfoundation.org \
    --cc=hansg@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox