[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250501120725.GJ1567507@google.com>
Date: Thu, 1 May 2025 13:07:25 +0100
From: Lee Jones <lee@...nel.org>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
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 Thu, 24 Apr 2025, Andy Shevchenko wrote:
> 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.
Done.
--
Lee Jones [李琼斯]
Powered by blists - more mailing lists