[PATCH v2 1/2] wifi: ath11k: Unprepare MHI on power up failure

[Vladimir Zapolskiy]

If MHI power up fails, for instance due to a missing firmware file,
ath11k_core_init() does not power down the device on this error,
the MHI controller stays prepared, and the MHI device context and
the RDDM image table allocated by mhi_prepare_for_power_up() are
not released, here is a report from kmemleak:

  % 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

Track the prepared state of MHI in a new ab_pci flag, and unprepare it
in ath11k_mhi_unregister() if it is still set. The prepared state is
intentionally kept on power up failure, because the RDDM image may still
be read by a firmware crash dump collection in the reset work. After a
failed power up in the reset or resume path the device stays bound, and
ath11k_mhi_stop() releases the resources on the next power down, as
before.

Also unprepare MHI in ath11k_mhi_stop() only if the flag is set. If
mhi_prepare_for_power_up() fails in the reset or resume path, it
releases the device context on its own error path, and the unconditional
mhi_unprepare_after_power_down() in a later ath11k_mhi_stop() frees the
command ring buffers again and dereferences the NULL device context.

Fixes: 3df6d74aedfd ("ath11k: mhi: use mhi_sync_power_up()")
Signed-off-by: Vladimir Zapolskiy <vz at kernel.org>
---
 drivers/net/wireless/ath/ath11k/mhi.c | 9 ++++++++-
 drivers/net/wireless/ath/ath11k/pci.h | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
index a6c9ff112c68..7cc652292d6f 100644
--- a/drivers/net/wireless/ath/ath11k/mhi.c
+++ b/drivers/net/wireless/ath/ath11k/mhi.c
@@ -425,6 +425,10 @@ void ath11k_mhi_unregister(struct ath11k_pci *ab_pci)
 {
 	struct mhi_controller *mhi_ctrl = ab_pci->mhi_ctrl;
 
+	/* MHI stays prepared after a failed power up */
+	if (test_and_clear_bit(ATH11K_PCI_MHI_PREPARED, &ab_pci->flags))
+		mhi_unprepare_after_power_down(mhi_ctrl);
+
 	mhi_unregister_controller(mhi_ctrl);
 	kfree(mhi_ctrl->irq);
 	mhi_free_controller(mhi_ctrl);
@@ -443,6 +447,8 @@ int ath11k_mhi_start(struct ath11k_pci *ab_pci)
 		return ret;
 	}
 
+	set_bit(ATH11K_PCI_MHI_PREPARED, &ab_pci->flags);
+
 	ret = mhi_sync_power_up(ab_pci->mhi_ctrl);
 	if (ret) {
 		ath11k_warn(ab, "failed to power up mhi: %d", ret);
@@ -463,7 +469,8 @@ void ath11k_mhi_stop(struct ath11k_pci *ab_pci, bool is_suspend)
 	else
 		mhi_power_down(ab_pci->mhi_ctrl, true);
 
-	mhi_unprepare_after_power_down(ab_pci->mhi_ctrl);
+	if (test_and_clear_bit(ATH11K_PCI_MHI_PREPARED, &ab_pci->flags))
+		mhi_unprepare_after_power_down(ab_pci->mhi_ctrl);
 }
 
 int ath11k_mhi_suspend(struct ath11k_pci *ab_pci)
diff --git a/drivers/net/wireless/ath/ath11k/pci.h b/drivers/net/wireless/ath/ath11k/pci.h
index 1e3005a4b64c..1e811245c1a5 100644
--- a/drivers/net/wireless/ath/ath11k/pci.h
+++ b/drivers/net/wireless/ath/ath11k/pci.h
@@ -55,6 +55,7 @@
 
 enum ath11k_pci_flags {
 	ATH11K_PCI_ASPM_RESTORE,
+	ATH11K_PCI_MHI_PREPARED,
 };
 
 struct ath11k_pci {
-- 
2.51.0