* [PATCH] wifi: ath11k: Unprepare MHI on power up failure
@ 2026-10-28 10:43 Vladimir Zapolskiy
2026-10-29 5:10 ` Rameshkumar Sundaram
2026-10-29 5:38 ` Baochen Qiang
0 siblings, 2 replies; 4+ messages in thread
From: Vladimir Zapolskiy @ 2026-10-28 10:43 UTC (permalink / raw)
To: Jeff Johnson, Kalle Valo; +Cc: linux-wireless, ath11k
If MHI power up fails, for instance it happens due to a missing firmware,
ath11k_mhi_start() returns without undoing mhi_prepare_for_power_up().
Then the failed probe or power up unwinding frees the MHI controller,
but the MHI device context and the RDDM image table allocated by
mhi_prepare_for_power_up() are leaked along with their DMA buffers:
% cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff00079156a140 (size 64):
comm "(udev-worker)", pid 595, jiffies 4294893781
hex dump (first 32 bytes):
00 d0 f1 80 00 80 ff ff 00 50 f1 80 00 80 ff ff .........P......
00 d0 f7 80 00 80 ff ff 00 30 fc ff 00 00 00 00 .........0......
backtrace (crc 62662f0a):
kmemleak_alloc+0x34/0x40
__kmalloc_cache_noprof+0x224/0x3b8
mhi_prepare_for_power_up+0x58/0x548 [mhi]
ath11k_mhi_start+0x30/0xb8 [ath11k_pci]
ath11k_pci_power_up+0xcc/0x188 [ath11k_pci]
ath11k_core_init+0x8c/0x190 [ath11k]
ath11k_pci_probe+0x4e0/0x870 [ath11k_pci]
local_pci_probe+0x40/0xa8
pci_call_probe+0x64/0x190
pci_device_probe+0xac/0x138
really_probe+0xbc/0x298
__driver_probe_device+0x16c/0x19c
driver_probe_device+0x3c/0x114
__driver_attach+0x98/0x19c
bus_for_each_dev+0x74/0xd0
driver_attach+0x24/0x30
unreferenced object 0xffff0007916fb800 (size 512):
comm "(udev-worker)", pid 595, jiffies 4294893781
hex dump (first 32 bytes):
00 60 04 81 00 80 ff ff 0a 00 00 00 00 00 00 00 .`..............
00 10 e0 82 00 80 ff ff 00 00 00 00 00 00 00 00 ................
backtrace (crc 52e9e075):
kmemleak_alloc+0x34/0x40
__kmalloc_noprof+0x280/0x440
mhi_alloc_bhie_table+0x54/0x168 [mhi]
mhi_prepare_for_power_up+0x318/0x548 [mhi]
ath11k_mhi_start+0x30/0xb8 [ath11k_pci]
ath11k_pci_power_up+0xcc/0x188 [ath11k_pci]
ath11k_core_init+0x8c/0x190 [ath11k]
ath11k_pci_probe+0x4e0/0x870 [ath11k_pci]
local_pci_probe+0x40/0xa8
pci_call_probe+0x64/0x190
pci_device_probe+0xac/0x138
really_probe+0xbc/0x298
__driver_probe_device+0x16c/0x19c
driver_probe_device+0x3c/0x114
__driver_attach+0x98/0x19c
bus_for_each_dev+0x74/0xd0
The MHI device is already powered down by mhi_sync_power_up() on
failure, but mhi_unprepare_after_power_down() still has to be
called on error path to release the allocated resources.
Fixes: 3df6d74aedfd ("ath11k: mhi: use mhi_sync_power_up()")
Signed-off-by: Vladimir Zapolskiy <vz@kernel.org>
---
drivers/net/wireless/ath/ath11k/mhi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
index a6c9ff112c68..76c194573ec7 100644
--- a/drivers/net/wireless/ath/ath11k/mhi.c
+++ b/drivers/net/wireless/ath/ath11k/mhi.c
@@ -446,6 +446,7 @@ int ath11k_mhi_start(struct ath11k_pci *ab_pci)
ret = mhi_sync_power_up(ab_pci->mhi_ctrl);
if (ret) {
ath11k_warn(ab, "failed to power up mhi: %d", ret);
+ mhi_unprepare_after_power_down(ab_pci->mhi_ctrl);
return ret;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] wifi: ath11k: Unprepare MHI on power up failure
2026-10-28 10:43 [PATCH] wifi: ath11k: Unprepare MHI on power up failure Vladimir Zapolskiy
@ 2026-10-29 5:10 ` Rameshkumar Sundaram
2026-10-29 5:38 ` Baochen Qiang
1 sibling, 0 replies; 4+ messages in thread
From: Rameshkumar Sundaram @ 2026-10-29 5:10 UTC (permalink / raw)
To: Vladimir Zapolskiy, Jeff Johnson, Kalle Valo; +Cc: linux-wireless, ath11k
On 10/28/2026 4:13 PM, Vladimir Zapolskiy wrote:
> If MHI power up fails, for instance it happens due to a missing firmware,
> ath11k_mhi_start() returns without undoing mhi_prepare_for_power_up().
> Then the failed probe or power up unwinding frees the MHI controller,
> but the MHI device context and the RDDM image table allocated by
> mhi_prepare_for_power_up() are leaked along with their DMA buffers:
>
> % cat /sys/kernel/debug/kmemleak
> unreferenced object 0xffff00079156a140 (size 64):
> comm "(udev-worker)", pid 595, jiffies 4294893781
> hex dump (first 32 bytes):
> 00 d0 f1 80 00 80 ff ff 00 50 f1 80 00 80 ff ff .........P......
> 00 d0 f7 80 00 80 ff ff 00 30 fc ff 00 00 00 00 .........0......
> backtrace (crc 62662f0a):
> kmemleak_alloc+0x34/0x40
> __kmalloc_cache_noprof+0x224/0x3b8
> mhi_prepare_for_power_up+0x58/0x548 [mhi]
> ath11k_mhi_start+0x30/0xb8 [ath11k_pci]
> ath11k_pci_power_up+0xcc/0x188 [ath11k_pci]
> ath11k_core_init+0x8c/0x190 [ath11k]
> ath11k_pci_probe+0x4e0/0x870 [ath11k_pci]
> local_pci_probe+0x40/0xa8
> pci_call_probe+0x64/0x190
> pci_device_probe+0xac/0x138
> really_probe+0xbc/0x298
> __driver_probe_device+0x16c/0x19c
> driver_probe_device+0x3c/0x114
> __driver_attach+0x98/0x19c
> bus_for_each_dev+0x74/0xd0
> driver_attach+0x24/0x30
> unreferenced object 0xffff0007916fb800 (size 512):
> comm "(udev-worker)", pid 595, jiffies 4294893781
> hex dump (first 32 bytes):
> 00 60 04 81 00 80 ff ff 0a 00 00 00 00 00 00 00 .`..............
> 00 10 e0 82 00 80 ff ff 00 00 00 00 00 00 00 00 ................
> backtrace (crc 52e9e075):
> kmemleak_alloc+0x34/0x40
> __kmalloc_noprof+0x280/0x440
> mhi_alloc_bhie_table+0x54/0x168 [mhi]
> mhi_prepare_for_power_up+0x318/0x548 [mhi]
> ath11k_mhi_start+0x30/0xb8 [ath11k_pci]
> ath11k_pci_power_up+0xcc/0x188 [ath11k_pci]
> ath11k_core_init+0x8c/0x190 [ath11k]
> ath11k_pci_probe+0x4e0/0x870 [ath11k_pci]
> local_pci_probe+0x40/0xa8
> pci_call_probe+0x64/0x190
> pci_device_probe+0xac/0x138
> really_probe+0xbc/0x298
> __driver_probe_device+0x16c/0x19c
> driver_probe_device+0x3c/0x114
> __driver_attach+0x98/0x19c
> bus_for_each_dev+0x74/0xd0
>
> The MHI device is already powered down by mhi_sync_power_up() on
> failure, but mhi_unprepare_after_power_down() still has to be
> called on error path to release the allocated resources.
>
> Fixes: 3df6d74aedfd ("ath11k: mhi: use mhi_sync_power_up()")
> Signed-off-by: Vladimir Zapolskiy <vz@kernel.org>
> ---
> drivers/net/wireless/ath/ath11k/mhi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
> index a6c9ff112c68..76c194573ec7 100644
> --- a/drivers/net/wireless/ath/ath11k/mhi.c
> +++ b/drivers/net/wireless/ath/ath11k/mhi.c
> @@ -446,6 +446,7 @@ int ath11k_mhi_start(struct ath11k_pci *ab_pci)
> ret = mhi_sync_power_up(ab_pci->mhi_ctrl);
> if (ret) {
> ath11k_warn(ab, "failed to power up mhi: %d", ret);
> + mhi_unprepare_after_power_down(ab_pci->mhi_ctrl);
> return ret;
> }
>
Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] wifi: ath11k: Unprepare MHI on power up failure
2026-10-28 10:43 [PATCH] wifi: ath11k: Unprepare MHI on power up failure Vladimir Zapolskiy
2026-10-29 5:10 ` Rameshkumar Sundaram
@ 2026-10-29 5:38 ` Baochen Qiang
2026-10-29 8:43 ` Vladimir Zapolskiy
1 sibling, 1 reply; 4+ messages in thread
From: Baochen Qiang @ 2026-10-29 5:38 UTC (permalink / raw)
To: Vladimir Zapolskiy, Jeff Johnson, Kalle Valo; +Cc: linux-wireless, ath11k
On 10/28/2026 6:43 PM, Vladimir Zapolskiy wrote:
> If MHI power up fails, for instance it happens due to a missing firmware,
> ath11k_mhi_start() returns without undoing mhi_prepare_for_power_up().
> Then the failed probe or power up unwinding frees the MHI controller,
> but the MHI device context and the RDDM image table allocated by
> mhi_prepare_for_power_up() are leaked along with their DMA buffers:
>
> % cat /sys/kernel/debug/kmemleak
> unreferenced object 0xffff00079156a140 (size 64):
> comm "(udev-worker)", pid 595, jiffies 4294893781
> hex dump (first 32 bytes):
> 00 d0 f1 80 00 80 ff ff 00 50 f1 80 00 80 ff ff .........P......
> 00 d0 f7 80 00 80 ff ff 00 30 fc ff 00 00 00 00 .........0......
> backtrace (crc 62662f0a):
> kmemleak_alloc+0x34/0x40
> __kmalloc_cache_noprof+0x224/0x3b8
> mhi_prepare_for_power_up+0x58/0x548 [mhi]
> ath11k_mhi_start+0x30/0xb8 [ath11k_pci]
> ath11k_pci_power_up+0xcc/0x188 [ath11k_pci]
> ath11k_core_init+0x8c/0x190 [ath11k]
> ath11k_pci_probe+0x4e0/0x870 [ath11k_pci]
> local_pci_probe+0x40/0xa8
> pci_call_probe+0x64/0x190
> pci_device_probe+0xac/0x138
> really_probe+0xbc/0x298
> __driver_probe_device+0x16c/0x19c
> driver_probe_device+0x3c/0x114
> __driver_attach+0x98/0x19c
> bus_for_each_dev+0x74/0xd0
> driver_attach+0x24/0x30
> unreferenced object 0xffff0007916fb800 (size 512):
> comm "(udev-worker)", pid 595, jiffies 4294893781
> hex dump (first 32 bytes):
> 00 60 04 81 00 80 ff ff 0a 00 00 00 00 00 00 00 .`..............
> 00 10 e0 82 00 80 ff ff 00 00 00 00 00 00 00 00 ................
> backtrace (crc 52e9e075):
> kmemleak_alloc+0x34/0x40
> __kmalloc_noprof+0x280/0x440
> mhi_alloc_bhie_table+0x54/0x168 [mhi]
> mhi_prepare_for_power_up+0x318/0x548 [mhi]
> ath11k_mhi_start+0x30/0xb8 [ath11k_pci]
> ath11k_pci_power_up+0xcc/0x188 [ath11k_pci]
> ath11k_core_init+0x8c/0x190 [ath11k]
> ath11k_pci_probe+0x4e0/0x870 [ath11k_pci]
> local_pci_probe+0x40/0xa8
> pci_call_probe+0x64/0x190
> pci_device_probe+0xac/0x138
> really_probe+0xbc/0x298
> __driver_probe_device+0x16c/0x19c
> driver_probe_device+0x3c/0x114
> __driver_attach+0x98/0x19c
> bus_for_each_dev+0x74/0xd0
>
> The MHI device is already powered down by mhi_sync_power_up() on
> failure, but mhi_unprepare_after_power_down() still has to be
> called on error path to release the allocated resources.
>
> Fixes: 3df6d74aedfd ("ath11k: mhi: use mhi_sync_power_up()")
> Signed-off-by: Vladimir Zapolskiy <vz@kernel.org>
> ---
> drivers/net/wireless/ath/ath11k/mhi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
> index a6c9ff112c68..76c194573ec7 100644
> --- a/drivers/net/wireless/ath/ath11k/mhi.c
> +++ b/drivers/net/wireless/ath/ath11k/mhi.c
> @@ -446,6 +446,7 @@ int ath11k_mhi_start(struct ath11k_pci *ab_pci)
> ret = mhi_sync_power_up(ab_pci->mhi_ctrl);
> if (ret) {
> ath11k_warn(ab, "failed to power up mhi: %d", ret);
> + mhi_unprepare_after_power_down(ab_pci->mhi_ctrl);
For the probe path itself this is fine: when probe fails the driver is not bound, so no
remove or shutdown follows and this is the only unprepare.
The problem is that in runtime, ath11k_hif_power_up() is also called from reset and resume:
reset:
ath11k_core_reset() {
...
ath11k_hif_power_up()
}
resume:
ath11k_core_resume_early() {
...
ath11k_hif_power_up()
}
The later teardown in rmmod path or shutdown path then calls power_down unconditionally,
regardless of whether the last power_up succeeded:
rmmod:
mhi_unprepare_after_power_down() <- 2nd call
ath11k_mhi_stop()
ath11k_pci_power_down()
ath11k_hif_power_down()
ath11k_core_deinit()
ath11k_pci_remove()
shutdown:
mhi_unprepare_after_power_down() <- 2nd call
ath11k_mhi_stop()
ath11k_pci_power_down()
ath11k_pci_shutdown()
so the unprepare is called twice, while in the second call we can hit NULL pointer
dereference or double free ...
This comes from the fact the existing code does not handle power up failure correctly in
reset and resume. I don't see a proper solution to this, maybe we need a new flag telling
us whether the unprepare has been done already ?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] wifi: ath11k: Unprepare MHI on power up failure
2026-10-29 5:38 ` Baochen Qiang
@ 2026-10-29 8:43 ` Vladimir Zapolskiy
0 siblings, 0 replies; 4+ messages in thread
From: Vladimir Zapolskiy @ 2026-10-29 8:43 UTC (permalink / raw)
To: Baochen Qiang, Jeff Johnson, Kalle Valo; +Cc: linux-wireless, ath11k
Hi Baochen.
On 10/29/26 07:38, Baochen Qiang wrote:
>
>
> On 10/28/2026 6:43 PM, Vladimir Zapolskiy wrote:
>> If MHI power up fails, for instance it happens due to a missing firmware,
>> ath11k_mhi_start() returns without undoing mhi_prepare_for_power_up().
>> Then the failed probe or power up unwinding frees the MHI controller,
>> but the MHI device context and the RDDM image table allocated by
>> mhi_prepare_for_power_up() are leaked along with their DMA buffers:
>>
>> % cat /sys/kernel/debug/kmemleak
>> unreferenced object 0xffff00079156a140 (size 64):
>> comm "(udev-worker)", pid 595, jiffies 4294893781
>> hex dump (first 32 bytes):
>> 00 d0 f1 80 00 80 ff ff 00 50 f1 80 00 80 ff ff .........P......
>> 00 d0 f7 80 00 80 ff ff 00 30 fc ff 00 00 00 00 .........0......
>> backtrace (crc 62662f0a):
>> kmemleak_alloc+0x34/0x40
>> __kmalloc_cache_noprof+0x224/0x3b8
>> mhi_prepare_for_power_up+0x58/0x548 [mhi]
>> ath11k_mhi_start+0x30/0xb8 [ath11k_pci]
>> ath11k_pci_power_up+0xcc/0x188 [ath11k_pci]
>> ath11k_core_init+0x8c/0x190 [ath11k]
>> ath11k_pci_probe+0x4e0/0x870 [ath11k_pci]
>> local_pci_probe+0x40/0xa8
>> pci_call_probe+0x64/0x190
>> pci_device_probe+0xac/0x138
>> really_probe+0xbc/0x298
>> __driver_probe_device+0x16c/0x19c
>> driver_probe_device+0x3c/0x114
>> __driver_attach+0x98/0x19c
>> bus_for_each_dev+0x74/0xd0
>> driver_attach+0x24/0x30
>> unreferenced object 0xffff0007916fb800 (size 512):
>> comm "(udev-worker)", pid 595, jiffies 4294893781
>> hex dump (first 32 bytes):
>> 00 60 04 81 00 80 ff ff 0a 00 00 00 00 00 00 00 .`..............
>> 00 10 e0 82 00 80 ff ff 00 00 00 00 00 00 00 00 ................
>> backtrace (crc 52e9e075):
>> kmemleak_alloc+0x34/0x40
>> __kmalloc_noprof+0x280/0x440
>> mhi_alloc_bhie_table+0x54/0x168 [mhi]
>> mhi_prepare_for_power_up+0x318/0x548 [mhi]
>> ath11k_mhi_start+0x30/0xb8 [ath11k_pci]
>> ath11k_pci_power_up+0xcc/0x188 [ath11k_pci]
>> ath11k_core_init+0x8c/0x190 [ath11k]
>> ath11k_pci_probe+0x4e0/0x870 [ath11k_pci]
>> local_pci_probe+0x40/0xa8
>> pci_call_probe+0x64/0x190
>> pci_device_probe+0xac/0x138
>> really_probe+0xbc/0x298
>> __driver_probe_device+0x16c/0x19c
>> driver_probe_device+0x3c/0x114
>> __driver_attach+0x98/0x19c
>> bus_for_each_dev+0x74/0xd0
>>
>> The MHI device is already powered down by mhi_sync_power_up() on
>> failure, but mhi_unprepare_after_power_down() still has to be
>> called on error path to release the allocated resources.
>>
>> Fixes: 3df6d74aedfd ("ath11k: mhi: use mhi_sync_power_up()")
>> Signed-off-by: Vladimir Zapolskiy <vz@kernel.org>
>> ---
>> drivers/net/wireless/ath/ath11k/mhi.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
>> index a6c9ff112c68..76c194573ec7 100644
>> --- a/drivers/net/wireless/ath/ath11k/mhi.c
>> +++ b/drivers/net/wireless/ath/ath11k/mhi.c
>> @@ -446,6 +446,7 @@ int ath11k_mhi_start(struct ath11k_pci *ab_pci)
>> ret = mhi_sync_power_up(ab_pci->mhi_ctrl);
>> if (ret) {
>> ath11k_warn(ab, "failed to power up mhi: %d", ret);
>> + mhi_unprepare_after_power_down(ab_pci->mhi_ctrl);
>
> For the probe path itself this is fine: when probe fails the driver is not bound, so no
> remove or shutdown follows and this is the only unprepare.
>
> The problem is that in runtime, ath11k_hif_power_up() is also called from reset and resume:
>
> reset:
> ath11k_core_reset() {
> ...
> ath11k_hif_power_up()
> }
>
> resume:
> ath11k_core_resume_early() {
> ...
> ath11k_hif_power_up()
> }
>
> The later teardown in rmmod path or shutdown path then calls power_down unconditionally,
> regardless of whether the last power_up succeeded:
>
> rmmod:
> mhi_unprepare_after_power_down() <- 2nd call
> ath11k_mhi_stop()
> ath11k_pci_power_down()
> ath11k_hif_power_down()
> ath11k_core_deinit()
> ath11k_pci_remove()
>
> shutdown:
> mhi_unprepare_after_power_down() <- 2nd call
> ath11k_mhi_stop()
> ath11k_pci_power_down()
> ath11k_pci_shutdown()
>
> so the unprepare is called twice, while in the second call we can hit NULL pointer
> dereference or double free ...
>
> This comes from the fact the existing code does not handle power up failure correctly in
> reset and resume. I don't see a proper solution to this, maybe we need a new flag telling
> us whether the unprepare has been done already ?
Thank you for your review!
Due to my check the analysis is correct, and apparently the proposed solution
will work also, but I'd prefer to introduce a "prepared" flag set right after
mhi_sync_power_up() successful return rather than to toggle "unprepared" flag.
By the way, there is a memory leak on error path in ath12 driver also, but
the rootcause and its solution are different, there is a missing state set to
ATH12K_MHI_DEINIT, if ATH12K_MHI_POWER_ON fails in ath12k_mhi_start().
After the testing I'll prepare a changeset including both fixes for review.
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-29 8:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 10:43 [PATCH] wifi: ath11k: Unprepare MHI on power up failure Vladimir Zapolskiy
2026-10-29 5:10 ` Rameshkumar Sundaram
2026-10-29 5:38 ` Baochen Qiang
2026-10-29 8:43 ` Vladimir Zapolskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox