2019-04-17 02:43:13

by Wen Yang

[permalink] [raw]
Subject: [PATCH 0/2] fix leaked of_node references in drivers/power

The call to of_get_cpu_node/of_find_compatible_node/of_parse_phandle...
returns a node pointer with refcount incremented thus it must be
explicitly decremented after the last usage.

We developed a coccinelle SmPL to detect drivers/power code and
found some issues.
This patch series fixes those issues.

Cc: Sebastian Reichel <[email protected]>
Cc: [email protected]
Cc: [email protected]

Wen Yang (2):
power: supply: fix leaked of_node refs in ab8500_bm_of_probe
power: supply: fix leaked of_node refs in
power_supply_get_battery_info

drivers/power/supply/ab8500_bmdata.c | 1 +
drivers/power/supply/power_supply_core.c | 24 ++++++++++++++++--------
2 files changed, 17 insertions(+), 8 deletions(-)

--
2.9.5


2019-04-17 02:44:37

by Wen Yang

[permalink] [raw]
Subject: [PATCH 2/2] power: supply: fix leaked of_node refs in power_supply_get_battery_info

The call to of_parse_phandle returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.

Detected by coccinelle with the following warnings:
./drivers/power/supply/power_supply_core.c:601:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 595, but without a corresponding object release within this function.
./drivers/power/supply/power_supply_core.c:604:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 595, but without a corresponding object release within this function.
./drivers/power/supply/power_supply_core.c:632:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 595, but without a corresponding object release within this function.
./drivers/power/supply/power_supply_core.c:635:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 595, but without a corresponding object release within this function.
./drivers/power/supply/power_supply_core.c:653:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 595, but without a corresponding object release within this function.
./drivers/power/supply/power_supply_core.c:664:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 595, but without a corresponding object release within this function.
./drivers/power/supply/power_supply_core.c:673:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 595, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/power/supply/power_supply_core.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 65c619c..874495c 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -598,10 +598,12 @@ int power_supply_get_battery_info(struct power_supply *psy,

err = of_property_read_string(battery_np, "compatible", &value);
if (err)
- return err;
+ goto out_put_node;

- if (strcmp("simple-battery", value))
- return -ENODEV;
+ if (strcmp("simple-battery", value)) {
+ err = -ENODEV;
+ goto out_put_node;
+ }

/* The property and field names below must correspond to elements
* in enum power_supply_property. For reasoning, see
@@ -629,10 +631,12 @@ int power_supply_get_battery_info(struct power_supply *psy,

len = of_property_count_u32_elems(battery_np, "ocv-capacity-celsius");
if (len < 0 && len != -EINVAL) {
- return len;
+ err = len;
+ goto out_put_node;
} else if (len > POWER_SUPPLY_OCV_TEMP_MAX) {
dev_err(&psy->dev, "Too many temperature values\n");
- return -EINVAL;
+ err = -EINVAL;
+ goto out_put_node;
} else if (len > 0) {
of_property_read_u32_array(battery_np, "ocv-capacity-celsius",
info->ocv_temp, len);
@@ -650,7 +654,8 @@ int power_supply_get_battery_info(struct power_supply *psy,
dev_err(&psy->dev, "failed to get %s\n", propname);
kfree(propname);
power_supply_put_battery_info(psy, info);
- return -EINVAL;
+ err = -EINVAL;
+ goto out_put_node;
}

kfree(propname);
@@ -661,7 +666,8 @@ int power_supply_get_battery_info(struct power_supply *psy,
devm_kcalloc(&psy->dev, tab_len, sizeof(*table), GFP_KERNEL);
if (!info->ocv_table[index]) {
power_supply_put_battery_info(psy, info);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto out_put_node;
}

for (i = 0; i < tab_len; i++) {
@@ -670,7 +676,9 @@ int power_supply_get_battery_info(struct power_supply *psy,
}
}

- return 0;
+out_put_node:
+ of_node_put(battery_np);
+ return err;
}
EXPORT_SYMBOL_GPL(power_supply_get_battery_info);

--
2.9.5

2019-04-17 02:44:53

by Wen Yang

[permalink] [raw]
Subject: [PATCH 1/2] power: supply: fix leaked of_node refs in ab8500_bm_of_probe

The call to of_parse_phandle returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.

492 int ab8500_bm_of_probe(struct device *dev,
493 struct device_node *np,
494 struct abx500_bm_data *bm)
495 {
496 const struct batres_vs_temp *tmp_batres_tbl;
497 struct device_node *battery_node;
...
501 /* get phandle to 'battery-info' node */
502 battery_node = of_parse_phandle(np, "battery", 0);
...
509 if (!btech) {
510 dev_warn(dev, "missing property battery-name/type\n");
511 return -EINVAL; ---> leaked here
512 }
...
540 of_node_put(battery_node); ---> released here
541
542 return 0;
543 }

Detected by coccinelle with the following warnings:
./drivers/power/supply/ab8500_bmdata.c:511:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 502, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/power/supply/ab8500_bmdata.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/power/supply/ab8500_bmdata.c b/drivers/power/supply/ab8500_bmdata.c
index 7b2b699..f6a6697 100644
--- a/drivers/power/supply/ab8500_bmdata.c
+++ b/drivers/power/supply/ab8500_bmdata.c
@@ -508,6 +508,7 @@ int ab8500_bm_of_probe(struct device *dev,
btech = of_get_property(battery_node, "stericsson,battery-type", NULL);
if (!btech) {
dev_warn(dev, "missing property battery-name/type\n");
+ of_node_put(battery_node);
return -EINVAL;
}

--
2.9.5

2019-04-18 02:37:43

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH 0/2] fix leaked of_node references in drivers/power

Hi,

On Wed, Apr 17, 2019 at 10:43:01AM +0800, Wen Yang wrote:
> The call to of_get_cpu_node/of_find_compatible_node/of_parse_phandle...
> returns a node pointer with refcount incremented thus it must be
> explicitly decremented after the last usage.
>
> We developed a coccinelle SmPL to detect drivers/power code and
> found some issues.
> This patch series fixes those issues.
>
> Cc: Sebastian Reichel <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
>
> Wen Yang (2):
> power: supply: fix leaked of_node refs in ab8500_bm_of_probe
> power: supply: fix leaked of_node refs in
> power_supply_get_battery_info
>
> drivers/power/supply/ab8500_bmdata.c | 1 +
> drivers/power/supply/power_supply_core.c | 24 ++++++++++++++++--------
> 2 files changed, 17 insertions(+), 8 deletions(-)

Thanks, queued.

-- Sebastian


Attachments:
(No filename) (896.00 B)
signature.asc (849.00 B)
Download all attachments