* [PATCH v1 1/1] leds: pca955x: Avoid potential overflow when filling default_label
@ 2025-04-04 16:28 Andy Shevchenko
2025-04-04 18:58 ` kernel test robot
0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2025-04-04 16:28 UTC (permalink / raw)
To: Andy Shevchenko, linux-leds, linux-kernel; +Cc: Lee Jones, 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. 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(-)
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;
--
2.47.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1 1/1] leds: pca955x: Avoid potential overflow when filling default_label
2025-04-04 16:28 [PATCH v1 1/1] leds: pca955x: Avoid potential overflow when filling default_label Andy Shevchenko
@ 2025-04-04 18:58 ` kernel test robot
2025-04-06 12:37 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: kernel test robot @ 2025-04-04 18:58 UTC (permalink / raw)
To: Andy Shevchenko, linux-leds, linux-kernel
Cc: oe-kbuild-all, Lee Jones, Pavel Machek
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1 1/1] leds: pca955x: Avoid potential overflow when filling default_label
2025-04-04 18:58 ` kernel test robot
@ 2025-04-06 12:37 ` Andy Shevchenko
0 siblings, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2025-04-06 12:37 UTC (permalink / raw)
To: kernel test robot
Cc: linux-leds, linux-kernel, oe-kbuild-all, Lee Jones, Pavel Machek
On Sat, Apr 05, 2025 at 02:58:17AM +0800, kernel test robot wrote:
> 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);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Nice, different GCC have different level of issue here. I will address this
in v2.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-04-06 12:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-04 16:28 [PATCH v1 1/1] leds: pca955x: Avoid potential overflow when filling default_label Andy Shevchenko
2025-04-04 18:58 ` kernel test robot
2025-04-06 12:37 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox