[PATCH v2 1/2] power: bq2415x_charger: Properly handle ENODEV from power_supply_get_by_phandle

[PATCH v2 1/2] power: bq2415x_charger: Properly handle ENODEV from power_supply_get_by_phandle

[Krzysztof Kozlowski]

The power_supply_get_by_phandle() on error returns ENODEV or NULL.
The driver later expects obtained pointer to power supply to be
valid or NULL. If it is not NULL then it dereferences it in
bq2415x_notifier_call() which would lead to dereferencing ENODEV-value
pointer.

Properly handle the power_supply_get_by_phandle() error case by
replacing error value with NULL. This indicates that usb charger
detection won't be used.

Fix also memory leak of 'name' if power_supply_get_by_phandle() fails
with NULL and probe should defer.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Fixes: faffd234cf85 ("bq2415x_charger: Add DT support")
Cc: <stable@vger.kernel.org>

---

Changes since v1:
1. Fix memory leak in probe defer (goto error_2) (suggested by Sebastian
   Reichel).
2. Don't fail the probe if 'ti,usb-charger-detection' is not present.
   Just print message.
---
 drivers/power/bq2415x_charger.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/power/bq2415x_charger.c b/drivers/power/bq2415x_charger.c
index e384844a1ae1..edf945a5260f 100644
--- a/drivers/power/bq2415x_charger.c
+++ b/drivers/power/bq2415x_charger.c
@@ -1579,8 +1579,15 @@ static int bq2415x_probe(struct i2c_client *client,
 	if (np) {
 		bq->notify_psy = power_supply_get_by_phandle(np, "ti,usb-charger-detection");
 
-		if (!bq->notify_psy)
-			return -EPROBE_DEFER;
+		if (IS_ERR(bq->notify_psy)) {
+			bq->notify_psy = NULL;
+			dev_info(&client->dev,
+				"no 'ti,usb-charger-detection' property (err=%ld)\n",
+				PTR_ERR(bq->notify_psy));
+		} else if (!bq->notify_psy) {
+			ret = -EPROBE_DEFER;
+			goto error_2;
+		}
 	}
 	else if (pdata->notify_device)
 		bq->notify_psy = power_supply_get_by_name(pdata->notify_device);
-- 
1.9.1

[PATCH v2 2/2] power: bq2415x_charger: Fix memory leak on DTS parsing error

[Krzysztof Kozlowski]

Memory allocated for 'name' was leaking if required binding properties
were not present.

The memory for 'name' was allocated early at probe with kasprintf(). It
was freed in error paths executed before and after parsing DTS but not
in that error path.

Fix the error path for parsing device tree properties.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Fixes: faffd234cf85 ("bq2415x_charger: Add DT support")
Cc: <stable@vger.kernel.org>

---

Changes since v1:
1. None, new patch.
---
 drivers/power/bq2415x_charger.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/power/bq2415x_charger.c b/drivers/power/bq2415x_charger.c
index edf945a5260f..311be01480c8 100644
--- a/drivers/power/bq2415x_charger.c
+++ b/drivers/power/bq2415x_charger.c
@@ -1609,27 +1609,27 @@ static int bq2415x_probe(struct i2c_client *client,
 		ret = of_property_read_u32(np, "ti,current-limit",
 				&bq->init_data.current_limit);
 		if (ret)
-			return ret;
+			goto error_2;
 		ret = of_property_read_u32(np, "ti,weak-battery-voltage",
 				&bq->init_data.weak_battery_voltage);
 		if (ret)
-			return ret;
+			goto error_2;
 		ret = of_property_read_u32(np, "ti,battery-regulation-voltage",
 				&bq->init_data.battery_regulation_voltage);
 		if (ret)
-			return ret;
+			goto error_2;
 		ret = of_property_read_u32(np, "ti,charge-current",
 				&bq->init_data.charge_current);
 		if (ret)
-			return ret;
+			goto error_2;
 		ret = of_property_read_u32(np, "ti,termination-current",
 				&bq->init_data.termination_current);
 		if (ret)
-			return ret;
+			goto error_2;
 		ret = of_property_read_u32(np, "ti,resistor-sense",
 				&bq->init_data.resistor_sense);
 		if (ret)
-			return ret;
+			goto error_2;
 	} else {
 		memcpy(&bq->init_data, pdata, sizeof(bq->init_data));
 	}
-- 
1.9.1

Re: [PATCH v2 2/2] power: bq2415x_charger: Fix memory leak on DTS parsing error

[Sebastian Reichel]

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

Hi,

On Wed, Oct 15, 2014 at 04:25:10PM +0200, Krzysztof Kozlowski wrote:
> Memory allocated for 'name' was leaking if required binding properties
> were not present.
> 
> The memory for 'name' was allocated early at probe with kasprintf(). It
> was freed in error paths executed before and after parsing DTS but not
> in that error path.
> 
> Fix the error path for parsing device tree properties.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Fixes: faffd234cf85 ("bq2415x_charger: Add DT support")
> Cc: <stable@vger.kernel.org>

Applied to next.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 1/2] power: bq2415x_charger: Properly handle ENODEV from power_supply_get_by_phandle

[Sebastian Reichel]

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

Hi,

On Wed, Oct 15, 2014 at 04:25:09PM +0200, Krzysztof Kozlowski wrote:
> The power_supply_get_by_phandle() on error returns ENODEV or NULL.
> The driver later expects obtained pointer to power supply to be
> valid or NULL. If it is not NULL then it dereferences it in
> bq2415x_notifier_call() which would lead to dereferencing ENODEV-value
> pointer.
> 
> Properly handle the power_supply_get_by_phandle() error case by
> replacing error value with NULL. This indicates that usb charger
> detection won't be used.
> 
> Fix also memory leak of 'name' if power_supply_get_by_phandle() fails
> with NULL and probe should defer.

Applied to next.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]