Linux LED subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 1/1] leds: pca955x: Avoid potential overflow when filling default_label
@ 2025-04-07 10:49 Andy Shevchenko
  2025-04-07 14:49 ` kernel test robot
  0 siblings, 1 reply; 2+ messages in thread
From: Andy Shevchenko @ 2025-04-07 10:49 UTC (permalink / raw)
  To: Eddie James, Andy Shevchenko, Lee Jones, linux-leds, linux-kernel
  Cc: Pavel Machek

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. Note, the choice of the format specifier is made deliberately
like that because some of (old) GCC versions (powerpc-linux-gcc (GCC) 8.5.0)
are not happy otherwise.

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>
---

v2: updated format specifier once again (LKP)

 drivers/leds/leds-pca955x.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index e9cfde9fe4b1..b71c1580f4bb 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 */
@@ -581,7 +581,6 @@ 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 ls1[4];
 	u8 ls2[4];
 	struct pca955x_platform_data *pdata;
@@ -589,6 +588,9 @@ static int pca955x_probe(struct i2c_client *client)
 	bool keep_psc0 = false;
 	bool set_default_label = false;
 	char default_label[8];
+	int bit, nls, reg;
+	unsigned int i;
+	int err;
 
 	chip = i2c_get_match_data(client);
 	if (!chip)
@@ -610,16 +612,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 +695,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), "%hhu", i);
 				init_data.default_label = default_label;
 			} else {
 				init_data.default_label = NULL;
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2 1/1] leds: pca955x: Avoid potential overflow when filling default_label
  2025-04-07 10:49 [PATCH v2 1/1] leds: pca955x: Avoid potential overflow when filling default_label Andy Shevchenko
@ 2025-04-07 14:49 ` kernel test robot
  0 siblings, 0 replies; 2+ messages in thread
From: kernel test robot @ 2025-04-07 14:49 UTC (permalink / raw)
  To: Andy Shevchenko, Eddie James, Lee Jones, linux-leds, linux-kernel
  Cc: llvm, oe-kbuild-all, Pavel Machek

Hi Andy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on lee-leds/for-leds-next]
[also build test WARNING on linus/master v6.15-rc1 next-20250407]
[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/20250407-185316
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git for-leds-next
patch link:    https://lore.kernel.org/r/20250407105033.324789-1-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v2 1/1] leds: pca955x: Avoid potential overflow when filling default_label
config: hexagon-randconfig-002-20250407 (https://download.01.org/0day-ci/archive/20250407/202504072230.RoQhKH5y-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250407/202504072230.RoQhKH5y-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/202504072230.RoQhKH5y-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/leds/leds-pca955x.c:698:60: warning: format specifies type 'unsigned char' but the argument has type 'unsigned int' [-Wformat]
                                   snprintf(default_label, sizeof(default_label), "%hhu", i);
                                                                                   ~~~~   ^
                                                                                   %u
   1 warning generated.


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

   575	
   576	static int pca955x_probe(struct i2c_client *client)
   577	{
   578		struct pca955x *pca955x;
   579		struct pca955x_led *pca955x_led;
   580		const struct pca955x_chipdef *chip;
   581		struct led_classdev *led;
   582		struct led_init_data init_data;
   583		struct i2c_adapter *adapter;
   584		u8 ls1[4];
   585		u8 ls2[4];
   586		struct pca955x_platform_data *pdata;
   587		u8 psc0;
   588		bool keep_psc0 = false;
   589		bool set_default_label = false;
   590		char default_label[8];
   591		int bit, nls, reg;
   592		unsigned int i;
   593		int err;
   594	
   595		chip = i2c_get_match_data(client);
   596		if (!chip)
   597			return dev_err_probe(&client->dev, -ENODEV, "unknown chip\n");
   598	
   599		adapter = client->adapter;
   600		pdata = dev_get_platdata(&client->dev);
   601		if (!pdata) {
   602			pdata =	pca955x_get_pdata(client, chip);
   603			if (IS_ERR(pdata))
   604				return PTR_ERR(pdata);
   605		}
   606	
   607		/* Make sure the slave address / chip type combo given is possible */
   608		if ((client->addr & ~((1 << chip->slv_addr_shift) - 1)) !=
   609		    chip->slv_addr) {
   610			dev_err(&client->dev, "invalid slave address %02x\n",
   611				client->addr);
   612			return -ENODEV;
   613		}
   614	
   615		dev_info(&client->dev, "Using %s %u-bit LED driver at slave address 0x%02x\n",
   616			 client->name, chip->bits, client->addr);
   617	
   618		if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
   619			return -EIO;
   620	
   621		if (pdata->num_leds != chip->bits) {
   622			dev_err(&client->dev,
   623				"board info claims %d LEDs on a %u-bit chip\n",
   624				pdata->num_leds, chip->bits);
   625			return -ENODEV;
   626		}
   627	
   628		pca955x = devm_kzalloc(&client->dev, sizeof(*pca955x), GFP_KERNEL);
   629		if (!pca955x)
   630			return -ENOMEM;
   631	
   632		pca955x->leds = devm_kcalloc(&client->dev, chip->bits,
   633					     sizeof(*pca955x_led), GFP_KERNEL);
   634		if (!pca955x->leds)
   635			return -ENOMEM;
   636	
   637		i2c_set_clientdata(client, pca955x);
   638	
   639		mutex_init(&pca955x->lock);
   640		pca955x->client = client;
   641		pca955x->chipdef = chip;
   642		pca955x->blink_period = PCA955X_BLINK_DEFAULT_MS;
   643	
   644		init_data.devname_mandatory = false;
   645		init_data.devicename = "pca955x";
   646	
   647		nls = pca955x_num_led_regs(chip->bits);
   648		/* Use auto-increment feature to read all the LED selectors at once. */
   649		err = i2c_smbus_read_i2c_block_data(client,
   650						    0x10 | (pca955x_num_input_regs(chip->bits) + 4), nls,
   651						    ls1);
   652		if (err < 0)
   653			return err;
   654	
   655		for (i = 0; i < nls; i++)
   656			ls2[i] = ls1[i];
   657	
   658		for (i = 0; i < chip->bits; i++) {
   659			pca955x_led = &pca955x->leds[i];
   660			pca955x_led->led_num = i;
   661			pca955x_led->pca955x = pca955x;
   662			pca955x_led->type = pdata->leds[i].type;
   663	
   664			switch (pca955x_led->type) {
   665			case PCA955X_TYPE_NONE:
   666			case PCA955X_TYPE_GPIO:
   667				break;
   668			case PCA955X_TYPE_LED:
   669				bit = i % 4;
   670				reg = i / 4;
   671				led = &pca955x_led->led_cdev;
   672				led->brightness_set_blocking = pca955x_led_set;
   673				led->brightness_get = pca955x_led_get;
   674				led->blink_set = pca955x_led_blink;
   675	
   676				if (pdata->leds[i].default_state == LEDS_DEFSTATE_OFF)
   677					ls2[reg] = pca955x_ledsel(ls2[reg], bit, PCA955X_LS_LED_OFF);
   678				else if (pdata->leds[i].default_state == LEDS_DEFSTATE_ON)
   679					ls2[reg] = pca955x_ledsel(ls2[reg], bit, PCA955X_LS_LED_ON);
   680				else if (pca955x_ledstate(ls2[reg], bit) == PCA955X_LS_BLINK0) {
   681					keep_psc0 = true;
   682					set_bit(i, &pca955x->active_blink);
   683				}
   684	
   685				init_data.fwnode = pdata->leds[i].fwnode;
   686	
   687				if (is_of_node(init_data.fwnode)) {
   688					if (to_of_node(init_data.fwnode)->name[0] ==
   689					    '\0')
   690						set_default_label = true;
   691					else
   692						set_default_label = false;
   693				} else {
   694					set_default_label = true;
   695				}
   696	
   697				if (set_default_label) {
 > 698					snprintf(default_label, sizeof(default_label), "%hhu", i);
   699					init_data.default_label = default_label;
   700				} else {
   701					init_data.default_label = NULL;
   702				}
   703	
   704				err = devm_led_classdev_register_ext(&client->dev, led,
   705								     &init_data);
   706				if (err)
   707					return err;
   708	
   709				set_bit(i, &pca955x->active_pins);
   710			}
   711		}
   712	
   713		for (i = 0; i < nls; i++) {
   714			if (ls1[i] != ls2[i]) {
   715				err = pca955x_write_ls(pca955x, i, ls2[i]);
   716				if (err)
   717					return err;
   718			}
   719		}
   720	
   721		if (keep_psc0) {
   722			err = pca955x_read_psc(pca955x, 0, &psc0);
   723		} else {
   724			psc0 = pca955x_period_to_psc(pca955x, pca955x->blink_period);
   725			err = pca955x_write_psc(pca955x, 0, psc0);
   726		}
   727	
   728		if (err)
   729			return err;
   730	
   731		pca955x->blink_period = pca955x_psc_to_period(pca955x, psc0);
   732	
   733		/* Set PWM1 to fast frequency so we do not see flashing */
   734		err = pca955x_write_psc(pca955x, 1, 0);
   735		if (err)
   736			return err;
   737	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-04-07 14:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-07 10:49 [PATCH v2 1/1] leds: pca955x: Avoid potential overflow when filling default_label Andy Shevchenko
2025-04-07 14:49 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox