All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH] power: supply: bq25890: Fix power_supply reference leak
@ 2026-06-22  7:23 Ma Ke
  0 siblings, 0 replies; 3+ messages in thread
From: Ma Ke @ 2026-06-22  7:23 UTC (permalink / raw)
  To: sre, hansg, marex; +Cc: linux-pm, linux-kernel, akpm, Ma Ke, stable

bq25890_fw_probe() acquires a reference to a secondary charger via
power_supply_get_by_name(). This helper internally calls
class_find_device() which gets a reference on the found device.
However, the driver does not release this reference on error paths.
The devm cleanup callback also does not put the reference, so normal
driver unload leaks it as well.

Fix the leak by adding a power_supply_put() call in the new error
label of bq25890_fw_probe() and in the devm cleanup routine.

Found by code review.

Signed-off-by: Ma Ke <make_ruc2021@163.com>
Cc: stable@vger.kernel.org
Fixes: d54bf877fd87 ("power: supply: bq25890: Add support for having a secondary charger IC")
---
 drivers/power/supply/bq25890_charger.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index c1c12a447178..874ca620b465 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -1411,7 +1411,8 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
 	if (ret == 0) {
 		if (val > 100) {
 			dev_err(bq->dev, "Error linux,iinlim-percentage %u > 100\n", val);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto err_put_charger;
 		}
 		bq->iinlim_percentage = val;
 	} else {
@@ -1426,12 +1427,19 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
 
 	ret = bq25890_fw_read_u32_props(bq);
 	if (ret < 0)
-		return ret;
+		goto err_put_charger;
 
 	init->ilim_en = device_property_read_bool(bq->dev, "ti,use-ilim-pin");
 	init->boostf = device_property_read_bool(bq->dev, "ti,boost-low-freq");
 
 	return 0;
+
+err_put_charger:
+	if (bq->secondary_chrg) {
+		power_supply_put(bq->secondary_chrg);
+		bq->secondary_chrg = NULL;
+	}
+	return ret;
 }
 
 static void bq25890_non_devm_cleanup(void *data)
@@ -1440,6 +1448,11 @@ static void bq25890_non_devm_cleanup(void *data)
 
 	cancel_delayed_work_sync(&bq->pump_express_work);
 
+	if (bq->secondary_chrg) {
+		power_supply_put(bq->secondary_chrg);
+		bq->secondary_chrg = NULL;
+	}
+
 	if (bq->id >= 0) {
 		mutex_lock(&bq25890_id_mutex);
 		idr_remove(&bq25890_id, bq->id);
-- 
2.43.0


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

* [RESEND PATCH] power: supply: bq25890: Fix power_supply reference leak
@ 2026-06-30  8:33 Ma Ke
  2026-07-20 21:32 ` Sebastian Reichel
  0 siblings, 1 reply; 3+ messages in thread
From: Ma Ke @ 2026-06-30  8:33 UTC (permalink / raw)
  To: sre, hansg, marex; +Cc: linux-pm, linux-kernel, akpm, Ma Ke, stable

bq25890_fw_probe() acquires a reference to a secondary charger via
power_supply_get_by_name(). This helper internally calls
class_find_device() which gets a reference on the found device.
However, the driver does not release this reference on error paths.
The devm cleanup callback also does not put the reference, so normal
driver unload leaks it as well.

Fix the leak by adding a power_supply_put() call in the new error
label of bq25890_fw_probe() and in the devm cleanup routine.

Found by code review.

Signed-off-by: Ma Ke <make_ruc2021@163.com>
Cc: stable@vger.kernel.org
Fixes: d54bf877fd87 ("power: supply: bq25890: Add support for having a secondary charger IC")
---
 drivers/power/supply/bq25890_charger.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index c1c12a447178..b50be3fca77c 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -1411,7 +1411,8 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
 	if (ret == 0) {
 		if (val > 100) {
 			dev_err(bq->dev, "Error linux,iinlim-percentage %u > 100\n", val);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto err_put_charger;
 		}
 		bq->iinlim_percentage = val;
 	} else {
@@ -1426,12 +1427,19 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
 
 	ret = bq25890_fw_read_u32_props(bq);
 	if (ret < 0)
-		return ret;
+		goto err_put_charger;
 
 	init->ilim_en = device_property_read_bool(bq->dev, "ti,use-ilim-pin");
 	init->boostf = device_property_read_bool(bq->dev, "ti,boost-low-freq");
 
 	return 0;
+
+err_put_charger:
+	if (bq->secondary_chrg) {
+		power_supply_put(bq->secondary_chrg);
+		bq->secondary_chrg = NULL;
+	}
+	return ret;
 }
 
 static void bq25890_non_devm_cleanup(void *data)
@@ -1440,6 +1448,11 @@ static void bq25890_non_devm_cleanup(void *data)
 
 	cancel_delayed_work_sync(&bq->pump_express_work);
 
+	if (bq->secondary_chrg) {
+		power_supply_put(bq->secondary_chrg);
+		bq->secondary_chrg = NULL;
+	}
+
 	if (bq->id >= 0) {
 		mutex_lock(&bq25890_id_mutex);
 		idr_remove(&bq25890_id, bq->id);
-- 
2.43.0


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

* Re: [RESEND PATCH] power: supply: bq25890: Fix power_supply reference leak
  2026-06-30  8:33 [RESEND PATCH] power: supply: bq25890: Fix power_supply reference leak Ma Ke
@ 2026-07-20 21:32 ` Sebastian Reichel
  0 siblings, 0 replies; 3+ messages in thread
From: Sebastian Reichel @ 2026-07-20 21:32 UTC (permalink / raw)
  To: Ma Ke; +Cc: hansg, marex, linux-pm, linux-kernel, akpm, stable

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

Hi,

On Tue, Jun 30, 2026 at 04:33:52PM +0800, Ma Ke wrote:
> bq25890_fw_probe() acquires a reference to a secondary charger via
> power_supply_get_by_name(). This helper internally calls
> class_find_device() which gets a reference on the found device.
> However, the driver does not release this reference on error paths.
> The devm cleanup callback also does not put the reference, so normal
> driver unload leaks it as well.
> 
> Fix the leak by adding a power_supply_put() call in the new error
> label of bq25890_fw_probe() and in the devm cleanup routine.
> 
> Found by code review.
> 
> Signed-off-by: Ma Ke <make_ruc2021@163.com>
> Cc: stable@vger.kernel.org
> Fixes: d54bf877fd87 ("power: supply: bq25890: Add support for having a secondary charger IC")
> ---
>  drivers/power/supply/bq25890_charger.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index c1c12a447178..b50be3fca77c 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -1411,7 +1411,8 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
>  	if (ret == 0) {
>  		if (val > 100) {
>  			dev_err(bq->dev, "Error linux,iinlim-percentage %u > 100\n", val);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto err_put_charger;
>  		}
>  		bq->iinlim_percentage = val;
>  	} else {
> @@ -1426,12 +1427,19 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
>  
>  	ret = bq25890_fw_read_u32_props(bq);
>  	if (ret < 0)
> -		return ret;
> +		goto err_put_charger;
>  
>  	init->ilim_en = device_property_read_bool(bq->dev, "ti,use-ilim-pin");
>  	init->boostf = device_property_read_bool(bq->dev, "ti,boost-low-freq");
>  
>  	return 0;
> +
> +err_put_charger:
> +	if (bq->secondary_chrg) {
> +		power_supply_put(bq->secondary_chrg);
> +		bq->secondary_chrg = NULL;
> +	}
> +	return ret;
>  }

The fix is incomplete. It only handles errors in bq25890_fw_probe,
but does not release if bq25890_hw_init() fails. It's a lot simpler
to register a new cleanup function:

static void bq25890_release_secondary_chrg(void *data)
{
	struct bq25890_device *bq = data;

    power_supply_put(bq->secondary_chrg);
	bq->secondary_chrg = NULL;
}

	if (ret == 0) {
		bq->secondary_chrg = power_supply_get_by_name(str);
		if (!bq->secondary_chrg)
			return -EPROBE_DEFER;
        ret = devm_add_action_or_reset(dev, bq25890_release_secondary_chrg, bq);
        if (ret)
            return ret;
	}

Greetings,

-- Sebastian

>  static void bq25890_non_devm_cleanup(void *data)
> @@ -1440,6 +1448,11 @@ static void bq25890_non_devm_cleanup(void *data)
>  
>  	cancel_delayed_work_sync(&bq->pump_express_work);
>  
> +	if (bq->secondary_chrg) {
> +		power_supply_put(bq->secondary_chrg);
> +		bq->secondary_chrg = NULL;
> +	}
> +
>  	if (bq->id >= 0) {
>  		mutex_lock(&bq25890_id_mutex);
>  		idr_remove(&bq25890_id, bq->id);
> -- 
> 2.43.0
> 

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

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

end of thread, other threads:[~2026-07-20 21:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-30  8:33 [RESEND PATCH] power: supply: bq25890: Fix power_supply reference leak Ma Ke
2026-07-20 21:32 ` Sebastian Reichel
  -- strict thread matches above, loose matches on Subject: below --
2026-06-22  7:23 Ma Ke

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.