[PATCH 0/2] fix leaked of_node references in drivers/power

[PATCH 0/2] fix leaked of_node references in drivers/power

[Wen Yang]

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 <sre@kernel.org>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

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

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

[Wen Yang]

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 <wen.yang99@zte.com.cn>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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

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

[Wen Yang]

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 <wen.yang99@zte.com.cn>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 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

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

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 <sre@kernel.org>
> Cc: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]