linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] power: supply_core: Pass pointer to battery info
@ 2022-01-06 13:19 Dan Carpenter
  2022-01-10 15:50 ` Linus Walleij
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2022-01-06 13:19 UTC (permalink / raw)
  To: linus.walleij; +Cc: linux-pm

Hello Linus Walleij,

The patch 25fd330370ac: "power: supply_core: Pass pointer to battery
info" from Dec 15, 2021, leads to the following Smatch static checker
warning:

	drivers/power/supply/bq256xx_charger.c:1529 bq256xx_hw_init()
	error: potentially dereferencing uninitialized 'bat_info'.

drivers/power/supply/bq256xx_charger.c
    1505 static int bq256xx_hw_init(struct bq256xx_device *bq)
    1506 {
    1507         struct power_supply_battery_info *bat_info;
    1508         int wd_reg_val = BQ256XX_WATCHDOG_DIS;
    1509         int ret = 0;
    1510         int i;
    1511 
    1512         for (i = 0; i < BQ256XX_NUM_WD_VAL; i++) {
    1513                 if (bq->watchdog_timer == bq256xx_watchdog_time[i]) {
    1514                         wd_reg_val = i;
    1515                         break;
    1516                 }
    1517                 if (bq->watchdog_timer > bq256xx_watchdog_time[i] &&
    1518                     bq->watchdog_timer < bq256xx_watchdog_time[i + 1])
    1519                         wd_reg_val = i;
    1520         }
    1521         ret = regmap_update_bits(bq->regmap, BQ256XX_CHARGER_CONTROL_1,
    1522                                  BQ256XX_WATCHDOG_MASK, wd_reg_val <<
    1523                                                 BQ256XX_WDT_BIT_SHIFT);
    1524 
    1525         ret = power_supply_get_battery_info(bq->charger, &bat_info);

If the first allocation in power_supply_get_battery_info() fails then
bat_info is uninitialized.  (It's not really possible unless you do
failure injection).

    1526         if (ret) {
    1527                 dev_warn(bq->dev, "battery info missing, default values will be applied\n");
    1528 
--> 1529                 bat_info->constant_charge_current_max_ua =
    1530                                 bq->chip_info->bq256xx_def_ichg;
    1531 
    1532                 bat_info->constant_charge_voltage_max_uv =
    1533                                 bq->chip_info->bq256xx_def_vbatreg;
    1534 
    1535                 bat_info->precharge_current_ua =
    1536                                 bq->chip_info->bq256xx_def_iprechg;
    1537 
    1538                 bat_info->charge_term_current_ua =
    1539                                 bq->chip_info->bq256xx_def_iterm;
    1540 
    1541                 bq->init_data.ichg_max =
    1542                                 bq->chip_info->bq256xx_max_ichg;
    1543 
    1544                 bq->init_data.vbatreg_max =
    1545                                 bq->chip_info->bq256xx_max_vbatreg;
    1546         } else {
    1547                 bq->init_data.ichg_max =
    1548                         bat_info->constant_charge_current_max_ua;
    1549 
    1550                 bq->init_data.vbatreg_max =
    1551                         bat_info->constant_charge_voltage_max_uv;
    1552         }
    1553 
    1554         ret = bq->chip_info->bq256xx_set_vindpm(bq, bq->init_data.vindpm);
    1555         if (ret)
    1556                 return ret;
    1557 
    1558         ret = bq->chip_info->bq256xx_set_iindpm(bq, bq->init_data.iindpm);
    1559         if (ret)
    1560                 return ret;
    1561 
    1562         ret = bq->chip_info->bq256xx_set_ichg(bq,
    1563                                 bat_info->constant_charge_current_max_ua);
    1564         if (ret)
    1565                 return ret;
    1566 
    1567         ret = bq->chip_info->bq256xx_set_iprechg(bq,
    1568                                 bat_info->precharge_current_ua);
    1569         if (ret)
    1570                 return ret;
    1571 
    1572         ret = bq->chip_info->bq256xx_set_vbatreg(bq,
    1573                                 bat_info->constant_charge_voltage_max_uv);
    1574         if (ret)
    1575                 return ret;
    1576 
    1577         ret = bq->chip_info->bq256xx_set_iterm(bq,
    1578                                 bat_info->charge_term_current_ua);
    1579         if (ret)
    1580                 return ret;
    1581 
    1582         power_supply_put_battery_info(bq->charger, bat_info);
    1583 
    1584         return 0;
    1585 }

regards,
dan carpenter

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

* Re: [bug report] power: supply_core: Pass pointer to battery info
  2022-01-06 13:19 [bug report] power: supply_core: Pass pointer to battery info Dan Carpenter
@ 2022-01-10 15:50 ` Linus Walleij
  0 siblings, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2022-01-10 15:50 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-pm

On Thu, Jan 6, 2022 at 2:19 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:

> The patch 25fd330370ac: "power: supply_core: Pass pointer to battery
> info" from Dec 15, 2021, leads to the following Smatch static checker
> warning:
>
>         drivers/power/supply/bq256xx_charger.c:1529 bq256xx_hw_init()
>         error: potentially dereferencing uninitialized 'bat_info'.
(...)
>     1525         ret = power_supply_get_battery_info(bq->charger, &bat_info);
>
> If the first allocation in power_supply_get_battery_info() fails then
> bat_info is uninitialized.  (It's not really possible unless you do
> failure injection).
>
>     1526         if (ret) {
>     1527                 dev_warn(bq->dev, "battery info missing, default values will be applied\n");
>     1528
> --> 1529                 bat_info->constant_charge_current_max_ua =

Ouch, that one was easy to miss! I sent a fixup patch.

Yours,
Linus Walleij

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

* [bug report] power: supply_core: Pass pointer to battery info
@ 2024-02-12  6:31 Dan Carpenter
  0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2024-02-12  6:31 UTC (permalink / raw)
  To: linus.walleij; +Cc: linux-pm

Hello Linus Walleij,

The patch 25fd330370ac: "power: supply_core: Pass pointer to battery
info" from Dec 15, 2021 (linux-next), leads to the following Smatch
static checker warning:

	drivers/power/supply/bq256xx_charger.c:1595 bq256xx_hw_init()
	error: uninitialized symbol 'bat_info'.

drivers/power/supply/bq256xx_charger.c
    1565 static int bq256xx_hw_init(struct bq256xx_device *bq)
    1566 {
    1567         struct power_supply_battery_info *bat_info;
    1568         int wd_reg_val = BQ256XX_WATCHDOG_DIS;
    1569         int ret = 0;
    1570         int i;
    1571 
    1572         for (i = 0; i < BQ256XX_NUM_WD_VAL; i++) {
    1573                 if (bq->watchdog_timer == bq256xx_watchdog_time[i]) {
    1574                         wd_reg_val = i;
    1575                         break;
    1576                 }
    1577                 if (i + 1 < BQ256XX_NUM_WD_VAL &&
    1578                     bq->watchdog_timer > bq256xx_watchdog_time[i] &&
    1579                     bq->watchdog_timer < bq256xx_watchdog_time[i + 1])
    1580                         wd_reg_val = i;
    1581         }
    1582         ret = regmap_update_bits(bq->regmap, BQ256XX_CHARGER_CONTROL_1,
    1583                                  BQ256XX_WATCHDOG_MASK, wd_reg_val <<
    1584                                                 BQ256XX_WDT_BIT_SHIFT);
    1585         if (ret)
    1586                 return ret;
    1587 
    1588         ret = power_supply_get_battery_info(bq->charger, &bat_info);
    1589         if (ret == -ENOMEM)

Only -ENOMEM is handled but it can return a bunch of different errors
where bat_info is uninitialized and we just introduced another recently.

    1590                 return ret;
    1591 
    1592         if (ret) {
    1593                 dev_warn(bq->dev, "battery info missing, default values will be applied\n");
    1594 
--> 1595                 bat_info->constant_charge_current_max_ua =
                         ^^^^^^^^^^
This used to be bat_info.constant_charge_current_max_ua = ...


    1596                                 bq->chip_info->bq256xx_def_ichg;
    1597 
    1598                 bat_info->constant_charge_voltage_max_uv =

regards,
dan carpenter

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

end of thread, other threads:[~2024-02-12  6:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-06 13:19 [bug report] power: supply_core: Pass pointer to battery info Dan Carpenter
2022-01-10 15:50 ` Linus Walleij
  -- strict thread matches above, loose matches on Subject: below --
2024-02-12  6:31 Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).