[PATCH] mfd: lpc_ich: Fix ARRAY_SIZE usage for apl_gpio_resources

[PATCH] mfd: lpc_ich: Fix ARRAY_SIZE usage for apl_gpio_resources

[Purva Yeshi]

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

Use of the ARRAY_SIZE macro on the two-dimensional array apl_gpio_resources
led to incorrect calculations of num_resources, as ARRAY_SIZE only works
for one-dimensional arrays. It attempts to determine the size of the inner
array but does not correctly compute the total number of elements, leading
to incorrect indexing and potential out-of-bounds access.

This resulted in incorrect resource allocation, causing errors.

Replace ARRAY_SIZE(apl_gpio_resources[APL_GPIO_NORTH]) with ARRAY2D_SIZE,
which correctly calculates the number of elements in a 2D array by
considering both rows and columns, ensuring num_resources is assigned the
correct value.

Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com>
---
 drivers/mfd/lpc_ich.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index 4b7d0cb9340f..f0b8ff9ed177 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -187,32 +187,34 @@ static struct resource *apl_gpio_mem_resources[APL_GPIO_NR_RESOURCES] = {
 	[APL_GPIO_SOUTHWEST] = &apl_gpio_resources[APL_GPIO_SOUTHWEST][0],
 };
 
+#define ARRAY2D_SIZE(arr) (sizeof(arr) / sizeof((arr)[0][0]) / (sizeof((arr)[0]) / sizeof((arr)[0][0])))
+
 static const struct mfd_cell apl_gpio_devices[APL_GPIO_NR_DEVICES] = {
 	[APL_GPIO_NORTH] = {
 		.name = "apollolake-pinctrl",
 		.id = APL_GPIO_NORTH,
-		.num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_NORTH]),
+		.num_resources = ARRAY2D_SIZE(apl_gpio_resources),
 		.resources = apl_gpio_resources[APL_GPIO_NORTH],
 		.ignore_resource_conflicts = true,
 	},
 	[APL_GPIO_NORTHWEST] = {
 		.name = "apollolake-pinctrl",
 		.id = APL_GPIO_NORTHWEST,
-		.num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_NORTHWEST]),
+		.num_resources = ARRAY2D_SIZE(apl_gpio_resources),
 		.resources = apl_gpio_resources[APL_GPIO_NORTHWEST],
 		.ignore_resource_conflicts = true,
 	},
 	[APL_GPIO_WEST] = {
 		.name = "apollolake-pinctrl",
 		.id = APL_GPIO_WEST,
-		.num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_WEST]),
+		.num_resources = ARRAY2D_SIZE(apl_gpio_resources),
 		.resources = apl_gpio_resources[APL_GPIO_WEST],
 		.ignore_resource_conflicts = true,
 	},
 	[APL_GPIO_SOUTHWEST] = {
 		.name = "apollolake-pinctrl",
 		.id = APL_GPIO_SOUTHWEST,
-		.num_resources = ARRAY_SIZE(apl_gpio_resources[APL_GPIO_SOUTHWEST]),
+		.num_resources = ARRAY2D_SIZE(apl_gpio_resources),
 		.resources = apl_gpio_resources[APL_GPIO_SOUTHWEST],
 		.ignore_resource_conflicts = true,
 	},
-- 
2.34.1

Re: (subset) [PATCH] mfd: lpc_ich: Fix ARRAY_SIZE usage for apl_gpio_resources

[Lee Jones]

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

--
Lee Jones [李琼斯]

Re: (subset) [PATCH] mfd: lpc_ich: Fix ARRAY_SIZE usage for apl_gpio_resources

[Andy Shevchenko]

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

Re: (subset) [PATCH] mfd: lpc_ich: Fix ARRAY_SIZE usage for apl_gpio_resources

[Lee Jones]

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 [李琼斯]

Re: (subset) [PATCH] mfd: lpc_ich: Fix ARRAY_SIZE usage for apl_gpio_resources

[Andy Shevchenko]

Thu, May 01, 2025 at 01:07:25PM +0100, Lee Jones kirjoitti:
> 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.

I still see it as commit
https://web.git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=c6c07f8ea2cbb0dca0e529f9ed16df71276515a4

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH] mfd: lpc_ich: Fix ARRAY_SIZE usage for apl_gpio_resources

[Lee Jones]

On Fri, 02 May 2025, Andy Shevchenko wrote:

> Thu, May 01, 2025 at 01:07:25PM +0100, Lee Jones kirjoitti:
> > 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.
> 
> I still see it as commit
> https://web.git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=c6c07f8ea2cbb0dca0e529f9ed16df71276515a4

-ENOPUSH

Try again.

-- 
Lee Jones [李琼斯]

Re: (subset) [PATCH] mfd: lpc_ich: Fix ARRAY_SIZE usage for apl_gpio_resources

[Andy Shevchenko]

On Fri, May 02, 2025 at 08:27:32AM +0100, Lee Jones wrote:
> On Fri, 02 May 2025, Andy Shevchenko wrote:
> > Thu, May 01, 2025 at 01:07:25PM +0100, Lee Jones kirjoitti:
> > > 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:

[...]

> > > > > 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.
> > 
> > I still see it as commit
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git/commit/?h=for-mfd-next&id=c6c07f8ea2cbb0dca0e529f9ed16df71276515a4
> 
> -ENOPUSH
> 
> Try again.

Thanks, now it's not there.

-- 
With Best Regards,
Andy Shevchenko