linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: linux-kernel@vger.kernel.org, Yong Zhi <yong.zhi@intel.com>,
	Bingbu Cao <bingbu.cao@intel.com>,
	Tianshu Qiu <tian.shu.qiu@intel.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-media@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-hardening@vger.kernel.org,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH v2 1/2] media: staging/intel-ipu3: css: Fix wrong size comparison
Date: Mon, 2 Aug 2021 09:05:46 +0300	[thread overview]
Message-ID: <20210802060546.GL3@paasikivi.fi.intel.com> (raw)
In-Reply-To: <184d96f95d6261b1a91704eb68adbd0a2e1c2cc2.1627646101.git.gustavoars@kernel.org>

Hi Gustavo,

I missed you already had sent v2...

On Fri, Jul 30, 2021 at 07:08:13AM -0500, Gustavo A. R. Silva wrote:
> There is a wrong comparison of the total size of the loaded firmware
> css->fw->size with the size of a pointer to struct imgu_fw_header.
> 
> Fix this by using the right operand 'struct imgu_fw_header' for
> sizeof, instead of 'struct imgu_fw_header *' and turn binary_header
> into a flexible-array member. Also, adjust the relational operator
> to be '<=' instead of '<', as it seems that the intention of the
> comparison is to determine if the loaded firmware contains any
> 'struct imgu_fw_info' items in the binary_header[] array than merely
> the file_header (struct imgu_fw_bi_file_h).
> 
> The replacement of the one-element array with a flexible-array member
> also help with the ongoing efforts to globally enable -Warray-bounds
> and get us closer to being able to tighten the FORTIFY_SOURCE routines
> on memcpy().
> 
> Link: https://github.com/KSPP/linux/issues/79
> Link: https://github.com/KSPP/linux/issues/109
> Fixes: 09d290f0ba21 ("media: staging/intel-ipu3: css: Add support for firmware management")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> 
> It'd be just great if someone that knows this code better can confirm
> these changes are correct. In particular the adjustment of the
> relational operator. Thanks!
> 
> Changes in v2:
>  - Use flexible array and adjust relational operator, accordingly.

The operator was just correct. The check is just there to see the firmware
is at least as large as the struct as which it is being accessed.

>  - Update changelog text.
> 
>  drivers/staging/media/ipu3/ipu3-css-fw.c | 2 +-
>  drivers/staging/media/ipu3/ipu3-css-fw.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/ipu3/ipu3-css-fw.c b/drivers/staging/media/ipu3/ipu3-css-fw.c
> index 45aff76198e2..630cb5186b48 100644
> --- a/drivers/staging/media/ipu3/ipu3-css-fw.c
> +++ b/drivers/staging/media/ipu3/ipu3-css-fw.c
> @@ -124,7 +124,7 @@ int imgu_css_fw_init(struct imgu_css *css)
>  	/* Check and display fw header info */
>  
>  	css->fwp = (struct imgu_fw_header *)css->fw->data;
> -	if (css->fw->size < sizeof(struct imgu_fw_header *) ||
> +	if (css->fw->size <= sizeof(struct imgu_fw_header) ||
>  	    css->fwp->file_header.h_size != sizeof(struct imgu_fw_bi_file_h))
>  		goto bad_fw;
>  	if (sizeof(struct imgu_fw_bi_file_h) +
> diff --git a/drivers/staging/media/ipu3/ipu3-css-fw.h b/drivers/staging/media/ipu3/ipu3-css-fw.h
> index 3c078f15a295..c0bc57fd678a 100644
> --- a/drivers/staging/media/ipu3/ipu3-css-fw.h
> +++ b/drivers/staging/media/ipu3/ipu3-css-fw.h
> @@ -171,7 +171,7 @@ struct imgu_fw_bi_file_h {
>  
>  struct imgu_fw_header {
>  	struct imgu_fw_bi_file_h file_header;
> -	struct imgu_fw_info binary_header[1];	/* binary_nr items */
> +	struct imgu_fw_info binary_header[];	/* binary_nr items */
>  };
>  
>  /******************* Firmware functions *******************/

-- 
Regards,

Sakari Ailus

  reply	other threads:[~2021-08-02  6:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-30 12:07 [PATCH v2 0/2] Fix size comparison bug and use flexible array Gustavo A. R. Silva
2021-07-30 12:08 ` [PATCH v2 1/2] media: staging/intel-ipu3: css: Fix wrong size comparison Gustavo A. R. Silva
2021-08-02  6:05   ` Sakari Ailus [this message]
2021-08-02 13:46     ` Gustavo A. R. Silva
2021-08-04 12:37       ` Dan Carpenter
2021-08-10 15:18       ` Sakari Ailus
2021-08-10 16:26         ` Gustavo A. R. Silva
2021-08-10 16:30           ` Sakari Ailus
2021-08-10 16:40             ` Gustavo A. R. Silva
2021-07-30 12:09 ` [PATCH v2 2/2] media: staging/intel-ipu3: css: Use the struct_size() helper Gustavo A. R. Silva

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=20210802060546.GL3@paasikivi.fi.intel.com \
    --to=sakari.ailus@linux.intel.com \
    --cc=bingbu.cao@intel.com \
    --cc=dan.carpenter@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=gustavoars@kernel.org \
    --cc=linux-hardening@vger.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=tian.shu.qiu@intel.com \
    --cc=yong.zhi@intel.com \
    --subject='Re: [PATCH v2 1/2] media: staging/intel-ipu3: css: Fix wrong size comparison' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).