Revert "i2c: core: Allocate temp client on the stack in i2c_detect"

[PATCH] Revert "i2c: core: Allocate temp client on the stack in i2c_detect"

Posted by Arnd Bergmann 9 months, 3 weeks ago

From: Arnd Bergmann <arnd@arndb.de>

struct i2c_client is way too large to be put on the kernel stack, and depending
on the kernel configuration, this can exceed the compile-time warning limit:

drivers/i2c/i2c-core-base.c:1420:12: error: stack frame size (1040) exceeds limit (1024) in 'i2c_do_add_adapter' [-Werror,-Wframe-larger-than]
 1420 | static int i2c_do_add_adapter(struct i2c_driver *driver,
      |            ^

The current version is the result of a cleanup patch that does not appear
to be a requirement for anything else, so address the problem through a
simple revert.

Fixes: 735668f8e5c9 ("i2c: core: Allocate temp client on the stack in i2c_detect")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/i2c/i2c-core-base.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 35a221e2c11c..5c9419e95044 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2506,7 +2506,7 @@ static int i2c_detect_address(struct i2c_client *temp_client,
 static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver)
 {
 	const unsigned short *address_list;
-	struct i2c_client temp_client;
+	struct i2c_client *temp_client;
 	int i, err = 0;
 
 	address_list = driver->address_list;
@@ -2527,19 +2527,22 @@ static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver)
 		return 0;
 
 	/* Set up a temporary client to help detect callback */
-	memset(&temp_client, 0, sizeof(temp_client));
-	temp_client.adapter = adapter;
+	temp_client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
+	if (!temp_client)
+		return -ENOMEM;
+	temp_client->adapter = adapter;
 
 	for (i = 0; address_list[i] != I2C_CLIENT_END; i += 1) {
 		dev_dbg(&adapter->dev,
 			"found normal entry for adapter %d, addr 0x%02x\n",
 			i2c_adapter_id(adapter), address_list[i]);
-		temp_client.addr = address_list[i];
-		err = i2c_detect_address(&temp_client, driver);
+		temp_client->addr = address_list[i];
+		err = i2c_detect_address(temp_client, driver);
 		if (unlikely(err))
 			break;
 	}
 
+	kfree(temp_client);
 	return err;
 }
 
-- 
2.39.5

Re: [PATCH] Revert "i2c: core: Allocate temp client on the stack in i2c_detect"

Posted by Wolfram Sang 9 months, 3 weeks ago

On Fri, Feb 21, 2025 at 09:54:40PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> struct i2c_client is way too large to be put on the kernel stack, and depending
> on the kernel configuration, this can exceed the compile-time warning limit:
> 
> drivers/i2c/i2c-core-base.c:1420:12: error: stack frame size (1040) exceeds limit (1024) in 'i2c_do_add_adapter' [-Werror,-Wframe-larger-than]
>  1420 | static int i2c_do_add_adapter(struct i2c_driver *driver,
>       |            ^
> 
> The current version is the result of a cleanup patch that does not appear
> to be a requirement for anything else, so address the problem through a
> simple revert.
> 
> Fixes: 735668f8e5c9 ("i2c: core: Allocate temp client on the stack in i2c_detect")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thank you, yet Geert was faster and fixed a checkpatch check:

https://lore.kernel.org/r/f9aa39362e918b62aec0567f899b37d8d3c44710.1740064176.git.geert+renesas@glider.be

Re: [PATCH] Revert "i2c: core: Allocate temp client on the stack in i2c_detect"

Posted by Guenter Roeck 9 months, 3 weeks ago

On 2/21/25 12:54, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> struct i2c_client is way too large to be put on the kernel stack, and depending
> on the kernel configuration, this can exceed the compile-time warning limit:
> 
> drivers/i2c/i2c-core-base.c:1420:12: error: stack frame size (1040) exceeds limit (1024) in 'i2c_do_add_adapter' [-Werror,-Wframe-larger-than]
>   1420 | static int i2c_do_add_adapter(struct i2c_driver *driver,
>        |            ^
> 
> The current version is the result of a cleanup patch that does not appear
> to be a requirement for anything else, so address the problem through a
> simple revert.
> 
> Fixes: 735668f8e5c9 ("i2c: core: Allocate temp client on the stack in i2c_detect")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>