diff mbox series

[v1,1/1] leds: pca955x: Avoid potential overflow when filling default_label

Message ID 20250404162849.3650361-1-andriy.shevchenko@linux.intel.com
State New
Headers show
Series [v1,1/1] leds: pca955x: Avoid potential overflow when filling default_label | expand

Commit Message

Andy Shevchenko April 4, 2025, 4:28 p.m. UTC
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>
---
 drivers/leds/leds-pca955x.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

kernel test robot April 4, 2025, 6:58 p.m. UTC | #1
Hi Andy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v6.14]
[cannot apply to lee-leds/for-leds-next linus/master next-20250404]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/leds-pca955x-Avoid-potential-overflow-when-filling-default_label/20250405-003054
base:   v6.14
patch link:    https://lore.kernel.org/r/20250404162849.3650361-1-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 1/1] leds: pca955x: Avoid potential overflow when filling default_label
config: powerpc-randconfig-003-20250405 (https://download.01.org/0day-ci/archive/20250405/202504050256.SYq06TxB-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250405/202504050256.SYq06TxB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504050256.SYq06TxB-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/leds/leds-pca955x.c: In function 'pca955x_probe':
>> drivers/leds/leds-pca955x.c:554:53: warning: '%u' directive output may be truncated writing between 1 and 10 bytes into a region of size 8 [-Wformat-truncation=]
        snprintf(default_label, sizeof(default_label), "%u", i);
                                                        ^~
   drivers/leds/leds-pca955x.c:554:52: note: directive argument in the range [0, 4294967294]
        snprintf(default_label, sizeof(default_label), "%u", i);
                                                       ^~~~
   drivers/leds/leds-pca955x.c:554:5: note: 'snprintf' output between 2 and 11 bytes into a destination of size 8
        snprintf(default_label, sizeof(default_label), "%u", i);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +554 drivers/leds/leds-pca955x.c

   449	
   450	static int pca955x_probe(struct i2c_client *client)
   451	{
   452		struct pca955x *pca955x;
   453		struct pca955x_led *pca955x_led;
   454		const struct pca955x_chipdef *chip;
   455		struct led_classdev *led;
   456		struct led_init_data init_data;
   457		struct i2c_adapter *adapter;
   458		struct pca955x_platform_data *pdata;
   459		bool set_default_label = false;
   460		bool keep_pwm = false;
   461		char default_label[8];
   462		unsigned int i;
   463		int err;
   464	
   465		chip = i2c_get_match_data(client);
   466		if (!chip)
   467			return dev_err_probe(&client->dev, -ENODEV, "unknown chip\n");
   468	
   469		adapter = client->adapter;
   470		pdata = dev_get_platdata(&client->dev);
   471		if (!pdata) {
   472			pdata =	pca955x_get_pdata(client, chip);
   473			if (IS_ERR(pdata))
   474				return PTR_ERR(pdata);
   475		}
   476	
   477		/* Make sure the slave address / chip type combo given is possible */
   478		if ((client->addr & ~((1 << chip->slv_addr_shift) - 1)) !=
   479		    chip->slv_addr) {
   480			dev_err(&client->dev, "invalid slave address %02x\n",
   481				client->addr);
   482			return -ENODEV;
   483		}
   484	
   485		dev_info(&client->dev, "Using %s %u-bit LED driver at slave address 0x%02x\n",
   486			 client->name, chip->bits, client->addr);
   487	
   488		if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
   489			return -EIO;
   490	
   491		if (pdata->num_leds != chip->bits) {
   492			dev_err(&client->dev,
   493				"board info claims %d LEDs on a %u-bit chip\n",
   494				pdata->num_leds, chip->bits);
   495			return -ENODEV;
   496		}
   497	
   498		pca955x = devm_kzalloc(&client->dev, sizeof(*pca955x), GFP_KERNEL);
   499		if (!pca955x)
   500			return -ENOMEM;
   501	
   502		pca955x->leds = devm_kcalloc(&client->dev, chip->bits,
   503					     sizeof(*pca955x_led), GFP_KERNEL);
   504		if (!pca955x->leds)
   505			return -ENOMEM;
   506	
   507		i2c_set_clientdata(client, pca955x);
   508	
   509		mutex_init(&pca955x->lock);
   510		pca955x->client = client;
   511		pca955x->chipdef = chip;
   512	
   513		init_data.devname_mandatory = false;
   514		init_data.devicename = "pca955x";
   515	
   516		for (i = 0; i < chip->bits; i++) {
   517			pca955x_led = &pca955x->leds[i];
   518			pca955x_led->led_num = i;
   519			pca955x_led->pca955x = pca955x;
   520			pca955x_led->type = pdata->leds[i].type;
   521	
   522			switch (pca955x_led->type) {
   523			case PCA955X_TYPE_NONE:
   524			case PCA955X_TYPE_GPIO:
   525				break;
   526			case PCA955X_TYPE_LED:
   527				led = &pca955x_led->led_cdev;
   528				led->brightness_set_blocking = pca955x_led_set;
   529				led->brightness_get = pca955x_led_get;
   530	
   531				if (pdata->leds[i].default_state == LEDS_DEFSTATE_OFF) {
   532					err = pca955x_led_set(led, LED_OFF);
   533					if (err)
   534						return err;
   535				} else if (pdata->leds[i].default_state == LEDS_DEFSTATE_ON) {
   536					err = pca955x_led_set(led, LED_FULL);
   537					if (err)
   538						return err;
   539				}
   540	
   541				init_data.fwnode = pdata->leds[i].fwnode;
   542	
   543				if (is_of_node(init_data.fwnode)) {
   544					if (to_of_node(init_data.fwnode)->name[0] ==
   545					    '\0')
   546						set_default_label = true;
   547					else
   548						set_default_label = false;
   549				} else {
   550					set_default_label = true;
   551				}
   552	
   553				if (set_default_label) {
 > 554					snprintf(default_label, sizeof(default_label), "%u", i);
   555					init_data.default_label = default_label;
   556				} else {
   557					init_data.default_label = NULL;
   558				}
   559	
   560				err = devm_led_classdev_register_ext(&client->dev, led,
   561								     &init_data);
   562				if (err)
   563					return err;
   564	
   565				set_bit(i, &pca955x->active_pins);
   566	
   567				/*
   568				 * For default-state == "keep", let the core update the
   569				 * brightness from the hardware, then check the
   570				 * brightness to see if it's using PWM1. If so, PWM1
   571				 * should not be written below.
   572				 */
   573				if (pdata->leds[i].default_state == LEDS_DEFSTATE_KEEP) {
   574					if (led->brightness != LED_FULL &&
   575					    led->brightness != LED_OFF &&
   576					    led->brightness != LED_HALF)
   577						keep_pwm = true;
   578				}
   579			}
   580		}
   581	
   582		/* PWM0 is used for half brightness or 50% duty cycle */
   583		err = pca955x_write_pwm(client, 0, 255 - LED_HALF);
   584		if (err)
   585			return err;
   586	
   587		if (!keep_pwm) {
   588			/* PWM1 is used for variable brightness, default to OFF */
   589			err = pca955x_write_pwm(client, 1, 0);
   590			if (err)
   591				return err;
   592		}
   593	
   594		/* Set to fast frequency so we do not see flashing */
   595		err = pca955x_write_psc(client, 0, 0);
   596		if (err)
   597			return err;
   598		err = pca955x_write_psc(client, 1, 0);
   599		if (err)
   600			return err;
   601
diff mbox series

Patch

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index 94a9f8a54b35..1a2e6996d748 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -71,7 +71,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 */
 };
@@ -455,11 +455,12 @@  static int pca955x_probe(struct i2c_client *client)
 	struct led_classdev *led;
 	struct led_init_data init_data;
 	struct i2c_adapter *adapter;
-	int i, err;
 	struct pca955x_platform_data *pdata;
 	bool set_default_label = false;
 	bool keep_pwm = false;
 	char default_label[8];
+	unsigned int i;
+	int err;
 
 	chip = i2c_get_match_data(client);
 	if (!chip)
@@ -481,16 +482,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;
 	}
@@ -551,8 +551,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;