[PATCH v3 1/1] leds: pca955x: Avoid potential overflow when filling default_label

[PATCH v3 1/1] leds: pca955x: Avoid potential overflow when filling default_label

[Andy Shevchenko]

GCC compiler (Debian 14.2.0-17) is not happy about printing
into a too short buffer (when build with `make W=1`):

  drivers/leds/leds-pca955x.c:554:33: note: ‘snprintf’ output between 2 and 12 bytes into a destination of size 8

Indeed, the buffer size is chosen based on some assumptions,
while in general the assigned value might not fit (GCC can't
prove it does).

Fix this by changing the bits field in the struct pca955x_chipdef to u8,
with a positive side effect of the better memory footprint, and convert
loop iterator to be unsigned. With that done, update format specifiers
accordingly.

In one case join back string literal as it improves the grepping over the code
based on the message and remove duplicating information (the driver name is
printed as pert of the dev_*() output [1]) as we touch the same line anyway.

Link: https://lore.kernel.org/r/4ac527f2-c59e-70a2-efd4-da52370ea557@dave.eu/ [1]
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

v3: reworked again to satisfy both compilers (LKP)
v2: updated format specifier once again (LKP)

 drivers/leds/leds-pca955x.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index e9cfde9fe4b1..24a40a1cdb15 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -73,7 +73,7 @@ enum pca955x_type {
 };
 
 struct pca955x_chipdef {
-	int			bits;
+	u8			bits;
 	u8			slv_addr;	/* 7-bit slave address mask */
 	int			slv_addr_shift;	/* Number of bits to ignore */
 	int			blink_div;	/* PSC divider */
@@ -142,13 +142,13 @@ struct pca955x_platform_data {
 };
 
 /* 8 bits per input register */
-static inline int pca955x_num_input_regs(int bits)
+static inline u8 pca955x_num_input_regs(u8 bits)
 {
 	return (bits + 7) / 8;
 }
 
 /* 4 bits per LED selector register */
-static inline int pca955x_num_led_regs(int bits)
+static inline u8 pca955x_num_led_regs(u8 bits)
 {
 	return (bits + 3)  / 4;
 }
@@ -581,14 +581,14 @@ static int pca955x_probe(struct i2c_client *client)
 	struct led_classdev *led;
 	struct led_init_data init_data;
 	struct i2c_adapter *adapter;
-	int i, bit, err, nls, reg;
+	u8 i, nls, psc0;
 	u8 ls1[4];
 	u8 ls2[4];
 	struct pca955x_platform_data *pdata;
-	u8 psc0;
 	bool keep_psc0 = false;
 	bool set_default_label = false;
 	char default_label[8];
+	int bit, err, reg;
 
 	chip = i2c_get_match_data(client);
 	if (!chip)
@@ -610,16 +610,15 @@ static int pca955x_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
-	dev_info(&client->dev, "leds-pca955x: Using %s %d-bit LED driver at "
-		 "slave address 0x%02x\n", client->name, chip->bits,
-		 client->addr);
+	dev_info(&client->dev, "Using %s %u-bit LED driver at slave address 0x%02x\n",
+		 client->name, chip->bits, client->addr);
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -EIO;
 
 	if (pdata->num_leds != chip->bits) {
 		dev_err(&client->dev,
-			"board info claims %d LEDs on a %d-bit chip\n",
+			"board info claims %d LEDs on a %u-bit chip\n",
 			pdata->num_leds, chip->bits);
 		return -ENODEV;
 	}
@@ -694,8 +693,7 @@ static int pca955x_probe(struct i2c_client *client)
 			}
 
 			if (set_default_label) {
-				snprintf(default_label, sizeof(default_label),
-					 "%d", i);
+				snprintf(default_label, sizeof(default_label), "%u", i);
 				init_data.default_label = default_label;
 			} else {
 				init_data.default_label = NULL;
-- 
2.47.2

Re: (subset) [PATCH v3 1/1] leds: pca955x: Avoid potential overflow when filling default_label

[Lee Jones]

On Mon, 07 Apr 2025 18:13:26 +0300, Andy Shevchenko wrote:
> GCC compiler (Debian 14.2.0-17) is not happy about printing
> into a too short buffer (when build with `make W=1`):
> 
>   drivers/leds/leds-pca955x.c:554:33: note: ‘snprintf’ output between 2 and 12 bytes into a destination of size 8
> 
> Indeed, the buffer size is chosen based on some assumptions,
> while in general the assigned value might not fit (GCC can't
> prove it does).
> 
> [...]

Applied, thanks!

[1/1] leds: pca955x: Avoid potential overflow when filling default_label
      commit: 1463b1c5bb231a8d46dc9db1773a31167614f84b

--
Lee Jones [李琼斯]