linux-kernel - Re: [PATCH] staging: media: ipu3: buffer overflow fix in imgu_map

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y7LfAR4QdIzp81yW@paasikivi.fi.intel.com>
Date:   Mon, 2 Jan 2023 13:41:21 +0000
From:   Sakari Ailus <sakari.ailus@...ux.intel.com>
To:     Aleksandr Burakov <a.burakov@...alinux.ru>
Cc:     Bingbu Cao <bingbu.cao@...el.com>,
        Tianshu Qiu <tian.shu.qiu@...el.com>,
        linux-media@...r.kernel.org, linux-staging@...ts.linux.dev,
        linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH] staging: media: ipu3: buffer overflow fix in
 imgu_map_node

Hi Aleksandr,

On Fri, Dec 23, 2022 at 03:30:25PM +0300, Aleksandr Burakov wrote:
> If imgu_node_map[i].css_queue is not equal to css_queue
> then "i" after the loop could be equal to IMGU_NODE_NUM
> that is more than the border value (IMGU_NODE_NUM - 1).
> So imgu_map_node() call may return IMGU_NODE_NUM that is more
> than expected value.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device driver")
> Signed-off-by: Aleksandr Burakov <a.burakov@...alinux.ru>
> ---
>  drivers/staging/media/ipu3/ipu3.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/ipu3/ipu3.c b/drivers/staging/media/ipu3/ipu3.c
> index 0c453b37f8c4..cb09eb3cc227 100644
> --- a/drivers/staging/media/ipu3/ipu3.c
> +++ b/drivers/staging/media/ipu3/ipu3.c
> @@ -60,8 +60,10 @@ unsigned int imgu_map_node(struct imgu_device *imgu, unsigned int css_queue)
>  	for (i = 0; i < IMGU_NODE_NUM; i++)
>  		if (imgu_node_map[i].css_queue == css_queue)
>  			break;
> -
> -	return i;
> +	if (i < IMGU_NODE_NUM)
> +		return i;
> +	else
> +		return (IMGU_NODE_NUM - 1);
>  }
>  
>  /**************** Dummy buffers ****************/

Thanks for the patch. It would require a bug elsewhere in the driver for
this to happen. If some handling for this case is added, it shouldn't be
hiding the issue.

One easy way could be to add WARN_ON() for this, and return some value (as
you do). Zero would do equally well.

I.e.

	return WARN_ON(i >= IMGU_NODE_NUM) ? 0 : i;

-- 
Sakari Ailus