Re: [PATCH 2/5] wifi: ath11k: avoid IDR mutation during pending mgmt TX cleanup

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

On 9/26/2026 8:48 AM, Jiale Yao wrote:
> ath11k_mac_tx_mgmt_pending_free() is passed as an idr_for_each()
> callback and removes the current entry from txmgmt_idr. This can
> invalidate the radix-tree iterator retained by idr_for_each().
> 
> Both callers destroy the IDR immediately after the walk, so removing
> each entry in the callback is unnecessary. Split skb release from IDR
> removal and let the callback release the supplied skb without updating
> the IDR. The following idr_destroy() tears down the IDR itself.
> 
> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  drivers/net/wireless/ath/ath11k/mac.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
> index ae91b57c8422..696f9dd65ad4 100644
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -6146,18 +6146,11 @@ static void ath11k_mgmt_over_wmi_tx_drop(struct ath11k *ar, struct sk_buff *skb)
>  		wake_up(&ar->txmgmt_empty_waitq);
>  }
>  
> -static void ath11k_mac_tx_mgmt_free(struct ath11k *ar, int buf_id)
> +static void ath11k_mac_tx_mgmt_free_skb(struct ath11k *ar,
> +					struct sk_buff *msdu)
>  {
> -	struct sk_buff *msdu;
>  	struct ieee80211_tx_info *info;
>  
> -	spin_lock_bh(&ar->txmgmt_idr_lock);
> -	msdu = idr_remove(&ar->txmgmt_idr, buf_id);
> -	spin_unlock_bh(&ar->txmgmt_idr_lock);
> -
> -	if (!msdu)
> -		return;
> -
>  	dma_unmap_single(ar->ab->dev, ATH11K_SKB_CB(msdu)->paddr, msdu->len,
>  			 DMA_TO_DEVICE);
>  
> @@ -6167,11 +6160,23 @@ static void ath11k_mac_tx_mgmt_free(struct ath11k *ar, int buf_id)
>  	ath11k_mgmt_over_wmi_tx_drop(ar, msdu);
>  }
>  
> +static void ath11k_mac_tx_mgmt_free(struct ath11k *ar, int buf_id)
> +{
> +	struct sk_buff *msdu;
> +
> +	spin_lock_bh(&ar->txmgmt_idr_lock);
> +	msdu = idr_remove(&ar->txmgmt_idr, buf_id);
> +	spin_unlock_bh(&ar->txmgmt_idr_lock);
> +
> +	if (msdu)
> +		ath11k_mac_tx_mgmt_free_skb(ar, msdu);
> +}
> +
>  int ath11k_mac_tx_mgmt_pending_free(int buf_id, void *skb, void *ctx)
>  {
>  	struct ath11k *ar = ctx;
>  
> -	ath11k_mac_tx_mgmt_free(ar, buf_id);
> +	ath11k_mac_tx_mgmt_free_skb(ar, skb);
>  
>  	return 0;
>  }

Just documenting that this has the same potential UAF issue as the ath12k patch:

The ath11k patch makes the same structural change:
ath11k_mac_tx_mgmt_pending_free now calls
ath11k_mac_tx_mgmt_free_skb(ar, skb) instead of ath11k_mac_tx_mgmt_free(ar,
buf_id), so it frees the skb while leaving the IDR entry pointing to it until
idr_destroy() runs.

And we already confirmed that wmi_process_mgmt_tx_comp() in ath11k/wmi.c:6031
holds txmgmt_idr_lock, calls idr_find(), drops the lock, then dereferences the
returned pointer — the same UAF race as in ath12k.

/jeff