[PATCH] ath12k: fix NULL pointer dereference in rhash table destroy

[PATCH] ath12k: fix NULL pointer dereference in rhash table destroy

[Jose Ignacio Tornos Martinez]

When unbinding the ath12k driver, kernel NULL pointer dereferences
occur in irq_work_sync() called from rhashtable_destroy().

Two hash tables are affected:
1. ath12k_link_sta hash table in ath12k_base
2. ath12k_dp_link_peer hash table in ath12k_dp

The issue happens because the destroy functions are called unconditionally
in cleanup paths, but the hash tables are only initialized late in their
respective init functions. If the device was never fully started or if the
init functions failed before initializing the hash tables, the pointers
will be NULL. The issues are always reproducible from a VM because the MSI
addressing initialization is failing.

Call trace for ath12k_link_sta_rhash_tbl_destroy:
 RIP: irq_work_sync+0x1e/0x70
 rhashtable_destroy+0x12/0x60
 ath12k_link_sta_rhash_tbl_destroy+0x19/0x40 [ath12k]
 ath12k_core_stop+0xe/0x80 [ath12k]
 ath12k_core_hw_group_cleanup+0x6b/0xb0 [ath12k]
 ath12k_pci_remove+0x60/0x110 [ath12k]

Call trace for ath12k_dp_link_peer_rhash_tbl_destroy:
 RIP: irq_work_sync+0x1e/0x70
 rhashtable_destroy+0x12/0x60
 ath12k_dp_link_peer_rhash_tbl_destroy+0x29/0x50 [ath12k]
 ath12k_dp_cmn_device_deinit+0x21/0x140 [ath12k]
 ath12k_core_hw_group_cleanup+0x6b/0xb0 [ath12k]
 ath12k_pci_remove+0x60/0x110 [ath12k]

Fix this by adding NULL checks before calling rhashtable_destroy() in
both destroy functions.

Fixes: 57ccca410237 ("wifi: ath12k: Add hash table for ath12k_link_sta in ath12k_base")
Fixes: a88cf5f71adf ("wifi: ath12k: Add hash table for ath12k_dp_link_peer")
Cc: stable@vger.kernel.org
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 drivers/net/wireless/ath/ath12k/dp_peer.c | 5 +++++
 drivers/net/wireless/ath/ath12k/peer.c    | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/dp_peer.c b/drivers/net/wireless/ath/ath12k/dp_peer.c
index a1100782d45e..38045564e223 100644
--- a/drivers/net/wireless/ath/ath12k/dp_peer.c
+++ b/drivers/net/wireless/ath/ath12k/dp_peer.c
@@ -275,9 +275,14 @@ int ath12k_dp_link_peer_rhash_tbl_init(struct ath12k_dp *dp)
 void ath12k_dp_link_peer_rhash_tbl_destroy(struct ath12k_dp *dp)
 {
 	mutex_lock(&dp->link_peer_rhash_tbl_lock);
+	if (!dp->rhead_peer_addr)
+		goto unlock;
+
 	rhashtable_destroy(dp->rhead_peer_addr);
 	kfree(dp->rhead_peer_addr);
 	dp->rhead_peer_addr = NULL;
+
+unlock:
 	mutex_unlock(&dp->link_peer_rhash_tbl_lock);
 }
 
diff --git a/drivers/net/wireless/ath/ath12k/peer.c b/drivers/net/wireless/ath/ath12k/peer.c
index 2e875176baaa..80fee2ce68f1 100644
--- a/drivers/net/wireless/ath/ath12k/peer.c
+++ b/drivers/net/wireless/ath/ath12k/peer.c
@@ -444,6 +444,9 @@ int ath12k_link_sta_rhash_tbl_init(struct ath12k_base *ab)
 
 void ath12k_link_sta_rhash_tbl_destroy(struct ath12k_base *ab)
 {
+	if (!ab->rhead_sta_addr)
+		return;
+
 	rhashtable_destroy(ab->rhead_sta_addr);
 	kfree(ab->rhead_sta_addr);
 	ab->rhead_sta_addr = NULL;
-- 
2.54.0

Re: [PATCH] ath12k: fix NULL pointer dereference in rhash table destroy

[Jeff Johnson]

On 6/4/2026 12:10 AM, Jose Ignacio Tornos Martinez wrote:
> When unbinding the ath12k driver, kernel NULL pointer dereferences
> occur in irq_work_sync() called from rhashtable_destroy().
> 
> Two hash tables are affected:
> 1. ath12k_link_sta hash table in ath12k_base
> 2. ath12k_dp_link_peer hash table in ath12k_dp
> 
> The issue happens because the destroy functions are called unconditionally
> in cleanup paths, but the hash tables are only initialized late in their
> respective init functions. If the device was never fully started or if the
> init functions failed before initializing the hash tables, the pointers
> will be NULL. The issues are always reproducible from a VM because the MSI
> addressing initialization is failing.

My preference would be to fix the logic so that the deinit is symmetric with
init, and if any stage of init fails, it should unwind whatever init occurred
up to that point. So this patch seems to be a bandage instead of an engineered
fix.

But I'll let the Qualcomm engineering team give their opinion.

> 
> Call trace for ath12k_link_sta_rhash_tbl_destroy:
>  RIP: irq_work_sync+0x1e/0x70
>  rhashtable_destroy+0x12/0x60
>  ath12k_link_sta_rhash_tbl_destroy+0x19/0x40 [ath12k]
>  ath12k_core_stop+0xe/0x80 [ath12k]
>  ath12k_core_hw_group_cleanup+0x6b/0xb0 [ath12k]
>  ath12k_pci_remove+0x60/0x110 [ath12k]
> 
> Call trace for ath12k_dp_link_peer_rhash_tbl_destroy:
>  RIP: irq_work_sync+0x1e/0x70
>  rhashtable_destroy+0x12/0x60
>  ath12k_dp_link_peer_rhash_tbl_destroy+0x29/0x50 [ath12k]
>  ath12k_dp_cmn_device_deinit+0x21/0x140 [ath12k]
>  ath12k_core_hw_group_cleanup+0x6b/0xb0 [ath12k]
>  ath12k_pci_remove+0x60/0x110 [ath12k]
> 
> Fix this by adding NULL checks before calling rhashtable_destroy() in
> both destroy functions.
> 
> Fixes: 57ccca410237 ("wifi: ath12k: Add hash table for ath12k_link_sta in ath12k_base")
> Fixes: a88cf5f71adf ("wifi: ath12k: Add hash table for ath12k_dp_link_peer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
>  drivers/net/wireless/ath/ath12k/dp_peer.c | 5 +++++
>  drivers/net/wireless/ath/ath12k/peer.c    | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/dp_peer.c b/drivers/net/wireless/ath/ath12k/dp_peer.c
> index a1100782d45e..38045564e223 100644
> --- a/drivers/net/wireless/ath/ath12k/dp_peer.c
> +++ b/drivers/net/wireless/ath/ath12k/dp_peer.c
> @@ -275,9 +275,14 @@ int ath12k_dp_link_peer_rhash_tbl_init(struct ath12k_dp *dp)
>  void ath12k_dp_link_peer_rhash_tbl_destroy(struct ath12k_dp *dp)
>  {
>  	mutex_lock(&dp->link_peer_rhash_tbl_lock);
> +	if (!dp->rhead_peer_addr)
> +		goto unlock;

not a fan of this pattern.
if we go with a bandaid solution then i'd want to use guard(mutex) so that
there isn't a need for a cleanup goto -- we could just return like the patch below

> +
>  	rhashtable_destroy(dp->rhead_peer_addr);
>  	kfree(dp->rhead_peer_addr);
>  	dp->rhead_peer_addr = NULL;
> +
> +unlock:
>  	mutex_unlock(&dp->link_peer_rhash_tbl_lock);
>  }
>  
> diff --git a/drivers/net/wireless/ath/ath12k/peer.c b/drivers/net/wireless/ath/ath12k/peer.c
> index 2e875176baaa..80fee2ce68f1 100644
> --- a/drivers/net/wireless/ath/ath12k/peer.c
> +++ b/drivers/net/wireless/ath/ath12k/peer.c
> @@ -444,6 +444,9 @@ int ath12k_link_sta_rhash_tbl_init(struct ath12k_base *ab)
>  
>  void ath12k_link_sta_rhash_tbl_destroy(struct ath12k_base *ab)
>  {
> +	if (!ab->rhead_sta_addr)
> +		return;
> +
>  	rhashtable_destroy(ab->rhead_sta_addr);
>  	kfree(ab->rhead_sta_addr);
>  	ab->rhead_sta_addr = NULL;

Re: [PATCH] ath12k: fix NULL pointer dereference in rhash table destroy

[Jose Ignacio Tornos Martinez]

Hello Jeff,

Thank you for the feedback.

> My preference would be to fix the logic so that the deinit is symmetric with
> init, and if any stage of init fails, it should unwind whatever init occurred
> up to that point. So this patch seems to be a bandage instead of an engineered
> fix.
>
> But I'll let the Qualcomm engineering team give their opinion.
I understand the concern about symmetry.
The NULL check approach is intentional here. The ath12k driver has complex
multi-stage initialization, with many potential failure points.
The init/deinit paths are symmetric - destroy is called from the same
cleanup paths as init.
The issue is that init can fail partway through, leaving some components
uninitialized.
The rhashtable pointer itself serves as the initialization state indicator -
it's NULL before init and non-NULL after. Checking it directly is simpler
than adding separate state flags that must be kept in sync.
That said, I'm open to alternative approaches if you see a cleaner way to
handle partial initialization states across the driver's initialization
sequence. Happy to revise if there's a better solution I'm missing.

> not a fan of this pattern.
> if we go with a bandaid solution then i'd want to use guard(mutex) so that
> there isn't a need for a cleanup goto -- we could just return like the patch below
Agreed, guard(mutex) is cleaner. I'll incorporate this in v2 once we align
on the overall approach.

Thanks

Best regards
José Ignacio