Re: [RESEND PATCH ath-current 3/3] wifi: ath11k: unregister PM notifier on QMI init failure path

[Jeff Johnson <jeff.johnson@oss.qualcomm.com>]

On 7/29/2026 8:09 PM, Miaoqing Pan wrote:
> ath11k_core_init() registers a PM notifier before the QMI server
> becomes available. If the QMI server never arrives, the device remove()
> path can take the early-exit path introduced for QMI initialization
> failures, skipping ath11k_core_deinit().

This patch felt wrong to me, so I spent some time with my buddy Claude.

I asked Claude: in the last patch the commit text says "If the QMI server
never arrives, the device remove() path can take the early-exit path
introduced for QMI initialization failures". where is this device remove()
path code? I'm trying to understand why there needs to be a call to
ath11k_core_pm_notifier_unregister() in ath11k_ahb_remove() as well as in
ath11k_core_deinit(). since ath11k_core_pm_notifier_register() is only called
from one place it is logical that ath11k_core_pm_notifier_unregister() should
also be called in one place that is symmetrical. the proposed patch breaks the
init/deinit symmetry.

Claude said:
The symmetry argument is well-founded. Here's the actual structure:

ath11k_core_init() registers the notifier. ath11k_core_deinit() is its
symmetric counterpart and unregisters it. That is the intended 1:1 pairing.

The problem is that both ath11k_ahb_remove() and ath11k_pci_remove() have an
early-exit path for QMI_FAIL that bypasses ath11k_core_deinit() — but still
need to undo what ath11k_core_init() did. So they replicate the unregister
call directly. This is an existing pattern: pci.c:1188 already had
ath11k_core_pm_notifier_unregister() in its QMI_FAIL early-exit before this
patch series.

Patch 3 simply extends that same pattern to ahb.c's early-exit path, which was
missing it.

The symmetry break isn't introduced by this patch — it was already present for
PCI. The fix for the symmetry issue would be to restructure so the early-exit
paths all go through ath11k_core_deinit() (or a new partial-deinit function),
but that's a larger refactor. This patch's approach matches the existing
convention in the tree.

The commit message claim about "the early-exit path introduced for QMI
initialization failures" refers to the QMI_FAIL branch at ahb.c:1301 — the
goto qmi_fail block that skips ath11k_core_deinit().

Thank you, Claude.

So I guess this patch is OK since it is replicating the PCI pattern, but is
there any way to have ath11k_core_deinit() called in those early exit paths so
that we can keep the init/deinit symmetry? It just seems wrong that if
ath11k_core_init() did not return an error that we would have deinit logic
that would skip calling ath11k_core_deinit()

> 
> As a result, the PM notifier remains registered after the ath11k base
> object has been freed. A subsequent suspend or resume event may invoke
> the stale notifier and trigger a use-after-free.
> 
> Fix this by explicitly unregistering the PM notifier in the QMI failure
> cleanup path before releasing ath11k resources.
> 
> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.2.0.c2-00204-QCAMSLSWPLZ-1
> 
> Fixes: 32d93b51bc7e ("wifi: ath11k: choose default PM policy for hibernation")
> Signed-off-by: Miaoqing Pan <miaoqing.pan@oss.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath11k/ahb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c
> index 7f5f5c8d7c56..bdb8c99f10fa 100644
> --- a/drivers/net/wireless/ath/ath11k/ahb.c
> +++ b/drivers/net/wireless/ath/ath11k/ahb.c
> @@ -1272,6 +1272,7 @@ static void ath11k_ahb_remove(struct platform_device *pdev)
>  		ath11k_ahb_power_down(ab, false);
>  		ath11k_debugfs_soc_destroy(ab);
>  		ath11k_qmi_deinit_service(ab);
> +		ath11k_core_pm_notifier_unregister(ab);
>  		goto qmi_fail;
>  	}
>