linux-kernel - Re: (subset) [PATCH] mfd: lpc_ich: Fix ARRAY_SIZE usage for apl_gpio

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: <aAqKcn25bkrjIiLF@black.fi.intel.com>
Date: Thu, 24 Apr 2025 22:01:06 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Lee Jones <lee@...nel.org>
Cc: Peter Tyser <ptyser@...-inc.com>, Purva Yeshi <purvayeshi550@...il.com>,
	linux-kernel@...r.kernel.org
Subject: Re: (subset) [PATCH] mfd: lpc_ich: Fix ARRAY_SIZE usage for
 apl_gpio_resources

On Fri, Apr 04, 2025 at 02:13:08PM +0100, Lee Jones wrote:
> On Sat, 22 Mar 2025 18:48:41 +0530, Purva Yeshi wrote:
> > Fix warning detected by smatch tool:
> > drivers/mfd/lpc_ich.c:194:34: error: strange non-value function or array
> > drivers/mfd/lpc_ich.c:194:34: error: missing type information
> > drivers/mfd/lpc_ich.c:201:34: error: strange non-value function or array
> > drivers/mfd/lpc_ich.c:201:34: error: missing type information
> > drivers/mfd/lpc_ich.c:208:34: error: strange non-value function or array
> > drivers/mfd/lpc_ich.c:208:34: error: missing type information
> > drivers/mfd/lpc_ich.c:215:34: error: strange non-value function or array
> > drivers/mfd/lpc_ich.c:215:34: error: missing type information

[...]

> Applied, thanks!
> 
> [1/1] mfd: lpc_ich: Fix ARRAY_SIZE usage for apl_gpio_resources
>       commit: 87e172b0fdd3aa4e3d099884e608dbc70ee3e663

Can this be reverted ASAP, please? See below why.

There is no problem with the code. The original author of the change
haven't proved otherwise.

The change made it much worse to read and maintain. By the way, it actually
_added_ the problem as far as I can see with my small test program.

Let's just calculate based on the sizeof(struct foo) taken as 10 for
simplicity and array size as 4x2. The full size of the array is
4 * 2 * 10 bytes. The size of the entry in outer array will be 2 * 10 bytes.
Now, what ARRAY2D_SIZE do is (4 * 2 * 10 / 10 / (2 * 10 / 10) == 4, and
that's WRONG! This will make a out-of-boundary accesses possible.

If smatch can't parse something, it's problem of smatch. No need to "fix"
the working and robust code. The original code even allows (in theory) to have
different amount of resources per entry, however it's quite unlikely to happen.
But at bare minimum it shows the entry taken along with _its_ ARRAY_SIZE()
and not something common over the outer array.

-- 
With Best Regards,
Andy Shevchenko