Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [PATCH 00/11] power: supply: Fix use-after-free in power_supply_changed()
@ 2025-12-20 22:35 Waqar Hameed
  2025-12-20 22:36 ` [PATCH 07/11] power: supply: pf1550: " Waqar Hameed
  2026-01-12  1:56 ` [PATCH 00/11] power: supply: " Sebastian Reichel
  0 siblings, 2 replies; 4+ messages in thread
From: Waqar Hameed @ 2025-12-20 22:35 UTC (permalink / raw)
  To: Linus Walleij, Sebastian Reichel, Samuel Kayode, Wenyou Yang,
	Ricardo Rivera-Matos, Dan Murphy, Tony Lindgren, Mike A. Chan,
	Jun Nakajima, Xiaohui Xin, Yunhong Jiang, Tom Keel, Frank Li,
	Lee Jones, Nikita Travkin, Anda-Maria Nicolae,
	Krzysztof Kozlowski, Phil Reid, Alan Cox, Sheng Yang
  Cc: linux-pm, linux-kernel, imx

The majority of the drivers in `drivers/power/supply/` do the right
thing when registering an interrupt handler and the `power_supply`
handle; namely making sure that the interrupt handler only runs while
the `power_supply` handle is valid. The drivers in this patch series do
not however. This can lead to a nasty use-after-free as thoroughly
explained in the commit message.

These were identified by grepping for `request.+irq` and
`power_supply_changed\(`, and then manually inspecting and fixing the
affected ones. This issue was found when writing a new driver for the
upcoming TI BQ25630 [1]. Patch adding support for that one will be sent
as soon as TI releases the datasheet publicly, which should be anytime
soon...

[1] https://www.ti.com/product/BQ25630

Waqar Hameed (11):
  power: supply: ab8500: Fix use-after-free in power_supply_changed()
  power: supply: act8945a: Fix use-after-free in power_supply_changed()
  power: supply: bq256xx: Fix use-after-free in power_supply_changed()
  power: supply: bq25980: Fix use-after-free in power_supply_changed()
  power: supply: cpcap-battery: Fix use-after-free in
    power_supply_changed()
  power: supply: goldfish: Fix use-after-free in power_supply_changed()
  power: supply: pf1550: Fix use-after-free in power_supply_changed()
  power: supply: pm8916_bms_vm: Fix use-after-free in
    power_supply_changed()
  power: supply: pm8916_lbc: Fix use-after-free in
    power_supply_changed()
  power: supply: rt9455: Fix use-after-free in power_supply_changed()
  power: supply: sbs-battery: Fix use-after-free in
    power_supply_changed()

 drivers/power/supply/ab8500_charger.c   | 40 ++++++++++++-------------
 drivers/power/supply/act8945a_charger.c | 16 +++++-----
 drivers/power/supply/bq256xx_charger.c  | 12 ++++----
 drivers/power/supply/bq25980_charger.c  | 12 ++++----
 drivers/power/supply/cpcap-battery.c    |  8 ++---
 drivers/power/supply/goldfish_battery.c | 12 ++++----
 drivers/power/supply/pf1550-charger.c   | 32 ++++++++++----------
 drivers/power/supply/pm8916_bms_vm.c    | 18 +++++------
 drivers/power/supply/pm8916_lbc.c       | 18 +++++------
 drivers/power/supply/rt9455_charger.c   | 17 ++++++-----
 drivers/power/supply/sbs-battery.c      | 36 +++++++++++-----------
 11 files changed, 111 insertions(+), 110 deletions(-)


base-commit: fa084c35afa13ab07a860ef0936cd987f9aa0460
-- 
2.39.5


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

* [PATCH 07/11] power: supply: pf1550: Fix use-after-free in power_supply_changed()
  2025-12-20 22:35 [PATCH 00/11] power: supply: Fix use-after-free in power_supply_changed() Waqar Hameed
@ 2025-12-20 22:36 ` Waqar Hameed
  2026-01-06  2:59   ` Samuel Kayode
  2026-01-12  1:56 ` [PATCH 00/11] power: supply: " Sebastian Reichel
  1 sibling, 1 reply; 4+ messages in thread
From: Waqar Hameed @ 2025-12-20 22:36 UTC (permalink / raw)
  To: Samuel Kayode, Sebastian Reichel, Frank Li, Lee Jones
  Cc: kernel, imx, linux-pm, linux-kernel

Using the `devm_` variant for requesting IRQ _before_ the `devm_`
variant for allocating/registering the `power_supply` handle, means that
the `power_supply` handle will be deallocated/unregistered _before_ the
interrupt handler (since `devm_` naturally deallocates in reverse
allocation order). This means that during removal, there is a race
condition where an interrupt can fire just _after_ the `power_supply`
handle has been freed, *but* just _before_ the corresponding
unregistration of the IRQ handler has run.

This will lead to the IRQ handler calling `power_supply_changed()` with
a freed `power_supply` handle. Which usually crashes the system or
otherwise silently corrupts the memory...

Note that there is a similar situation which can also happen during
`probe()`; the possibility of an interrupt firing _before_ registering
the `power_supply` handle. This would then lead to the nasty situation
of using the `power_supply` handle *uninitialized* in
`power_supply_changed()`.

Fix this racy use-after-free by making sure the IRQ is requested _after_
the registration of the `power_supply` handle.

Fixes: 4b6b6433a97d ("power: supply: pf1550: add battery charger support")
Signed-off-by: Waqar Hameed <waqar.hameed@axis.com>
---
 drivers/power/supply/pf1550-charger.c | 32 +++++++++++++--------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
index 98f1ee8eca3bc..a457862ef4610 100644
--- a/drivers/power/supply/pf1550-charger.c
+++ b/drivers/power/supply/pf1550-charger.c
@@ -584,22 +584,6 @@ static int pf1550_charger_probe(struct platform_device *pdev)
 		return dev_err_probe(chg->dev, ret,
 				     "failed to add battery sense work\n");
 
-	for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++) {
-		irq = platform_get_irq(pdev, i);
-		if (irq < 0)
-			return irq;
-
-		chg->virqs[i] = irq;
-
-		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
-						pf1550_charger_irq_handler,
-						IRQF_NO_SUSPEND,
-						"pf1550-charger", chg);
-		if (ret)
-			return dev_err_probe(&pdev->dev, ret,
-					     "failed irq request\n");
-	}
-
 	psy_cfg.drv_data = chg;
 
 	chg->charger = devm_power_supply_register(&pdev->dev,
@@ -616,6 +600,22 @@ static int pf1550_charger_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(chg->battery),
 				     "failed: power supply register\n");
 
+	for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++) {
+		irq = platform_get_irq(pdev, i);
+		if (irq < 0)
+			return irq;
+
+		chg->virqs[i] = irq;
+
+		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+						pf1550_charger_irq_handler,
+						IRQF_NO_SUSPEND,
+						"pf1550-charger", chg);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					     "failed irq request\n");
+	}
+
 	pf1550_dt_parse_dev_info(chg);
 
 	return pf1550_reg_init(chg);
-- 
2.39.5


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

* Re: [PATCH 07/11] power: supply: pf1550: Fix use-after-free in power_supply_changed()
  2025-12-20 22:36 ` [PATCH 07/11] power: supply: pf1550: " Waqar Hameed
@ 2026-01-06  2:59   ` Samuel Kayode
  0 siblings, 0 replies; 4+ messages in thread
From: Samuel Kayode @ 2026-01-06  2:59 UTC (permalink / raw)
  To: Waqar Hameed
  Cc: Sebastian Reichel, Frank Li, Lee Jones, kernel, imx, linux-pm,
	linux-kernel

On Sat, Dec 20, 2025 at 11:36:01PM +0100, Waqar Hameed wrote:
> Using the `devm_` variant for requesting IRQ _before_ the `devm_`
> variant for allocating/registering the `power_supply` handle, means that
> the `power_supply` handle will be deallocated/unregistered _before_ the
> interrupt handler (since `devm_` naturally deallocates in reverse
> allocation order). This means that during removal, there is a race
> condition where an interrupt can fire just _after_ the `power_supply`
> handle has been freed, *but* just _before_ the corresponding
> unregistration of the IRQ handler has run.
> 
> This will lead to the IRQ handler calling `power_supply_changed()` with
> a freed `power_supply` handle. Which usually crashes the system or
> otherwise silently corrupts the memory...
> 
> Note that there is a similar situation which can also happen during
> `probe()`; the possibility of an interrupt firing _before_ registering
> the `power_supply` handle. This would then lead to the nasty situation
> of using the `power_supply` handle *uninitialized* in
> `power_supply_changed()`.
> 
> Fix this racy use-after-free by making sure the IRQ is requested _after_
> the registration of the `power_supply` handle.
> 
> Fixes: 4b6b6433a97d ("power: supply: pf1550: add battery charger support")
> Signed-off-by: Waqar Hameed <waqar.hameed@axis.com>

Reviewed-by: Samuel Kayode <samkay014@gmail.com>

Thank you!
Sam

> ---
>  drivers/power/supply/pf1550-charger.c | 32 +++++++++++++--------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/power/supply/pf1550-charger.c b/drivers/power/supply/pf1550-charger.c
> index 98f1ee8eca3bc..a457862ef4610 100644
> --- a/drivers/power/supply/pf1550-charger.c
> +++ b/drivers/power/supply/pf1550-charger.c
> @@ -584,22 +584,6 @@ static int pf1550_charger_probe(struct platform_device *pdev)
>  		return dev_err_probe(chg->dev, ret,
>  				     "failed to add battery sense work\n");
>  
> -	for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++) {
> -		irq = platform_get_irq(pdev, i);
> -		if (irq < 0)
> -			return irq;
> -
> -		chg->virqs[i] = irq;
> -
> -		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> -						pf1550_charger_irq_handler,
> -						IRQF_NO_SUSPEND,
> -						"pf1550-charger", chg);
> -		if (ret)
> -			return dev_err_probe(&pdev->dev, ret,
> -					     "failed irq request\n");
> -	}
> -
>  	psy_cfg.drv_data = chg;
>  
>  	chg->charger = devm_power_supply_register(&pdev->dev,
> @@ -616,6 +600,22 @@ static int pf1550_charger_probe(struct platform_device *pdev)
>  		return dev_err_probe(&pdev->dev, PTR_ERR(chg->battery),
>  				     "failed: power supply register\n");
>  
> +	for (i = 0; i < PF1550_CHARGER_IRQ_NR; i++) {
> +		irq = platform_get_irq(pdev, i);
> +		if (irq < 0)
> +			return irq;
> +
> +		chg->virqs[i] = irq;
> +
> +		ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> +						pf1550_charger_irq_handler,
> +						IRQF_NO_SUSPEND,
> +						"pf1550-charger", chg);
> +		if (ret)
> +			return dev_err_probe(&pdev->dev, ret,
> +					     "failed irq request\n");
> +	}
> +
>  	pf1550_dt_parse_dev_info(chg);
>  
>  	return pf1550_reg_init(chg);

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

* Re: [PATCH 00/11] power: supply: Fix use-after-free in power_supply_changed()
  2025-12-20 22:35 [PATCH 00/11] power: supply: Fix use-after-free in power_supply_changed() Waqar Hameed
  2025-12-20 22:36 ` [PATCH 07/11] power: supply: pf1550: " Waqar Hameed
@ 2026-01-12  1:56 ` Sebastian Reichel
  1 sibling, 0 replies; 4+ messages in thread
From: Sebastian Reichel @ 2026-01-12  1:56 UTC (permalink / raw)
  To: Linus Walleij, Sebastian Reichel, Samuel Kayode, Wenyou Yang,
	Ricardo Rivera-Matos, Dan Murphy, Tony Lindgren, Mike A. Chan,
	Jun Nakajima, Xiaohui Xin, Yunhong Jiang, Tom Keel, Frank Li,
	Lee Jones, Nikita Travkin, Anda-Maria Nicolae,
	Krzysztof Kozlowski, Phil Reid, Alan Cox, Sheng Yang,
	Waqar Hameed
  Cc: linux-pm, linux-kernel, imx


On Sat, 20 Dec 2025 23:35:58 +0100, Waqar Hameed wrote:
> The majority of the drivers in `drivers/power/supply/` do the right
> thing when registering an interrupt handler and the `power_supply`
> handle; namely making sure that the interrupt handler only runs while
> the `power_supply` handle is valid. The drivers in this patch series do
> not however. This can lead to a nasty use-after-free as thoroughly
> explained in the commit message.
> 
> [...]

Applied, thanks!

[01/11] power: supply: ab8500: Fix use-after-free in power_supply_changed()
        commit: c4af8a98bb52825a5331ae1d0604c0ea6956ba4b
[02/11] power: supply: act8945a: Fix use-after-free in power_supply_changed()
        commit: 3291c51d4684d048dd2eb91b5b65fcfdaf72141f
[03/11] power: supply: bq256xx: Fix use-after-free in power_supply_changed()
        commit: 8005843369723d9c8975b7c4202d1b85d6125302
[04/11] power: supply: bq25980: Fix use-after-free in power_supply_changed()
        commit: 5f0b1cb41906e86b64bf69f5ededb83b0d757c27
[05/11] power: supply: cpcap-battery: Fix use-after-free in power_supply_changed()
        commit: 642f33e34b969eedec334738fd5df95d2dc42742
[06/11] power: supply: goldfish: Fix use-after-free in power_supply_changed()
        commit: b2ce982e2e0c888dc55c888ad0e20ea04daf2e6b
[07/11] power: supply: pf1550: Fix use-after-free in power_supply_changed()
        commit: 838767f5074700552d3f006d867caed65edc7328
[08/11] power: supply: pm8916_bms_vm: Fix use-after-free in power_supply_changed()
        commit: 62914959b35e9a1e29cc0f64cb8cfc5075a5366f
[09/11] power: supply: pm8916_lbc: Fix use-after-free in power_supply_changed()
        commit: b7508129978ae1e2ed9b0410396abc05def9c4eb
[10/11] power: supply: rt9455: Fix use-after-free in power_supply_changed()
        commit: e2febe375e5ea5afed92f4cd9711bde8f24ee6d2
[11/11] power: supply: sbs-battery: Fix use-after-free in power_supply_changed()
        commit: 8d59cf3887fbabacef53bfba473e33e8a8d9d07b

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>


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

end of thread, other threads:[~2026-01-12  1:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-20 22:35 [PATCH 00/11] power: supply: Fix use-after-free in power_supply_changed() Waqar Hameed
2025-12-20 22:36 ` [PATCH 07/11] power: supply: pf1550: " Waqar Hameed
2026-01-06  2:59   ` Samuel Kayode
2026-01-12  1:56 ` [PATCH 00/11] power: supply: " Sebastian Reichel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox