2019-10-02 16:16:06

by Yizhuo Zhai

[permalink] [raw]
Subject: [PATCH] power: supply: max17042_battery: fix some usage of uninitialized variables

Several functions in this file are trying to use regmap_read() to
initialize the specific variable, however, if regmap_read() fails,
the variable could be uninitialized but used directly, which is
potentially unsafe. The return value of regmap_read() should be
checked and handled. The same case also happens in function
max17042_thread_handler() but it needs more effort to patch.

Signed-off-by: Yizhuo <[email protected]>
---
drivers/power/supply/max17042_battery.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
index e6a2dacaa730..8e5727010ed2 100644
--- a/drivers/power/supply/max17042_battery.c
+++ b/drivers/power/supply/max17042_battery.c
@@ -675,8 +675,12 @@ static void max17042_reset_vfsoc0_reg(struct max17042_chip *chip)
{
unsigned int vfSoc;
struct regmap *map = chip->regmap;
+ int ret;
+
+ ret = regmap_read(map, MAX17042_VFSOC, &vfSoc);
+ if (ret)
+ return;

- regmap_read(map, MAX17042_VFSOC, &vfSoc);
regmap_write(map, MAX17042_VFSOC0Enable, VFSOC0_UNLOCK);
max17042_write_verify_reg(map, MAX17042_VFSOC0, vfSoc);
regmap_write(map, MAX17042_VFSOC0Enable, VFSOC0_LOCK);
@@ -686,12 +690,18 @@ static void max17042_load_new_capacity_params(struct max17042_chip *chip)
{
u32 full_cap0, rep_cap, dq_acc, vfSoc;
u32 rem_cap;
+ int ret;

struct max17042_config_data *config = chip->pdata->config_data;
struct regmap *map = chip->regmap;

- regmap_read(map, MAX17042_FullCAP0, &full_cap0);
- regmap_read(map, MAX17042_VFSOC, &vfSoc);
+ ret = regmap_read(map, MAX17042_FullCAP0, &full_cap0);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(map, MAX17042_VFSOC, &vfSoc);
+ if (ret)
+ return ret;

/* fg_vfSoc needs to shifted by 8 bits to get the
* perc in 1% accuracy, to get the right rem_cap multiply
@@ -1108,7 +1118,12 @@ static int max17042_probe(struct i2c_client *client,
if (!client->irq)
regmap_write(chip->regmap, MAX17042_SALRT_Th, 0xff00);

- regmap_read(chip->regmap, MAX17042_STATUS, &val);
+ ret = regmap_read(chip->regmap, MAX17042_STATUS, &val);
+ if (ret) {
+ dev_err(&client->dev, "Failed to get MAX17042_STATUS.\n");
+ return ret;
+ }
+
if (val & STATUS_POR_BIT) {
INIT_WORK(&chip->work, max17042_init_worker);
ret = devm_add_action(&client->dev, max17042_stop_work, chip);
--
2.17.1


2019-10-02 21:54:58

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] power: supply: max17042_battery: fix some usage of uninitialized variables

Hi Yizhuo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on power-supply/for-next]
[cannot apply to v5.4-rc1 next-20191002]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Yizhuo/power-supply-max17042_battery-fix-some-usage-of-uninitialized-variables/20191003-040948
base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sh

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/power/supply/max17042_battery.c: In function 'max17042_load_new_capacity_params':
>> drivers/power/supply/max17042_battery.c:697:10: warning: 'return' with a value, in function returning void
return ret;
^~~
drivers/power/supply/max17042_battery.c:686:13: note: declared here
static void max17042_load_new_capacity_params(struct max17042_chip *chip)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/power/supply/max17042_battery.c:701:10: warning: 'return' with a value, in function returning void
return ret;
^~~
drivers/power/supply/max17042_battery.c:686:13: note: declared here
static void max17042_load_new_capacity_params(struct max17042_chip *chip)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/return +697 drivers/power/supply/max17042_battery.c

685
686 static void max17042_load_new_capacity_params(struct max17042_chip *chip)
687 {
688 u32 full_cap0, rep_cap, dq_acc, vfSoc;
689 u32 rem_cap;
690 int ret;
691
692 struct max17042_config_data *config = chip->pdata->config_data;
693 struct regmap *map = chip->regmap;
694
695 ret = regmap_read(map, MAX17042_FullCAP0, &full_cap0);
696 if (ret)
> 697 return ret;
698
699 ret = regmap_read(map, MAX17042_VFSOC, &vfSoc);
700 if (ret)
701 return ret;
702
703 /* fg_vfSoc needs to shifted by 8 bits to get the
704 * perc in 1% accuracy, to get the right rem_cap multiply
705 * full_cap0, fg_vfSoc and devide by 100
706 */
707 rem_cap = ((vfSoc >> 8) * full_cap0) / 100;
708 max17042_write_verify_reg(map, MAX17042_RemCap, rem_cap);
709
710 rep_cap = rem_cap;
711 max17042_write_verify_reg(map, MAX17042_RepCap, rep_cap);
712
713 /* Write dQ_acc to 200% of Capacity and dP_acc to 200% */
714 dq_acc = config->fullcap / dQ_ACC_DIV;
715 max17042_write_verify_reg(map, MAX17042_dQacc, dq_acc);
716 max17042_write_verify_reg(map, MAX17042_dPacc, dP_ACC_200);
717
718 max17042_write_verify_reg(map, MAX17042_FullCAP,
719 config->fullcap);
720 regmap_write(map, MAX17042_DesignCap,
721 config->design_cap);
722 max17042_write_verify_reg(map, MAX17042_FullCAPNom,
723 config->fullcapnom);
724 /* Update SOC register with new SOC */
725 regmap_write(map, MAX17042_RepSOC, vfSoc);
726 }
727

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.67 kB)
.config.gz (50.62 kB)
Download all attachments