| Message ID | 20250404162849.3650361-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1,1/1] leds: pca955x: Avoid potential overflow when filling default_label | expand |
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
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.
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;
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(-)