Re: [PATCH ath-next] wifi: ath12k: Guard dp_peer dereference in ath12k_mac_peer_cleanup_all()

On 26-Jul-26 1:08 AM, Jeff Johnson wrote:
> On 7/22/2026 2:58 AM, Harsh Kumar Bijlani wrote:
>> From: Harsh Kumar Bijlani <[email protected]>
>>
>> In ath12k_mac_peer_cleanup_all(), the code iterates over all
>> ath12k_dp_link_peer entries in the dp->peers list and performs
>> datapath peer cleanup. For each peer, it accesses peer->dp_peer
>> to clear the link_peers[] and dp_peers[] RCU pointers.
>>
>> However, peer->dp_peer can legitimately be NULL. The
>> ath12k_dp_link_peer structure is allocated and added to dp->peers
>> during peer creation (ath12k_peer_create()), but the dp_peer
>> assignment happens later via ath12k_dp_link_peer_assign(), which
>> is called only when a valid dp_peer is found or created. If the
>> peer creation sequence is interrupted, for example, due to a
>> firmware crash, a race during interface teardown, or a failure
>> in the HTT peer-map event processing, then a link peer entry can
>> exist in dp->peers with dp_peer still NULL.
>>
>> Accessing peer->dp_peer without a NULL check in this path leads
>> to a NULL pointer dereference kernel crash during the mac cleanup
>> path, which is invoked on radio stop or firmware recovery.
>>
>> This is consistent with how the rest of the driver handles this
>> field: ath12k_dp_link_peer_unassign(), ath12k_dp_peer_cleanup(),
>> and several other paths all guard access to peer->dp_peer with
>> an explicit NULL check before dereferencing it.
>>
>> Fix this by wrapping the dp_peer dereference block inside a
>> NULL check, mirroring the pattern already used in the same
>> function for the Rx TID cleanup:
>>
>>      if (peer->sta && peer->dp_peer)
>>          ath12k_dp_rx_peer_tid_cleanup(ar, peer);
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.6-01243-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.1.c5-00302-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.115823.3
>>
>> Signed-off-by: Harsh Kumar Bijlani <[email protected]>
>> ---
>>   drivers/net/wireless/ath/ath12k/mac.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 956414b953ec..18c1c809ae4d 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -1237,10 +1237,12 @@ void ath12k_mac_peer_cleanup_all(struct ath12k *ar)
>>   
>>   		/* cleanup dp peer */
>>   		spin_lock_bh(&dp_hw->peer_lock);
>> -		dp_peer = peer->dp_peer;
>> -		peerid_index = ath12k_dp_peer_get_peerid_index(dp, peer->peer_id);
>> -		rcu_assign_pointer(dp_peer->link_peers[peer->link_id], NULL);
>> -		rcu_assign_pointer(dp_hw->dp_peers[peerid_index], NULL);
>> +		if (peer->dp_peer) {
>> +			dp_peer = peer->dp_peer;
>> +			peerid_index = ath12k_dp_peer_get_peerid_index(dp, peer->peer_id);
>> +			rcu_assign_pointer(dp_peer->link_peers[peer->link_id], NULL);
>> +			rcu_assign_pointer(dp_hw->dp_peers[peerid_index], NULL);
>> +		}
>>   		spin_unlock_bh(&dp_hw->peer_lock);
>>   
>>   		ath12k_dp_link_peer_rhash_delete(dp, peer);
>>
>> base-commit: ca27e3c2284a7a43ba390ed5ce0d68b3c869c0ce
> This patch does not apply:
> * Applying
> 20260722_harsh_bijlani_wifi_ath12k_guard_dp_peer_dereference_in_ath12k_mac_peer_cleanup_all.mbx
>
> Applying: wifi: ath12k: Guard dp_peer dereference in ath12k_mac_peer_cleanup_all()
> Using index info to reconstruct a base tree...
> M       drivers/net/wireless/ath/ath12k/mac.c
> Falling back to patching base and 3-way merge...
> Auto-merging drivers/net/wireless/ath/ath12k/mac.c
> CONFLICT (content): Merge conflict in drivers/net/wireless/ath/ath12k/mac.c
>
> It conflicts with
> a2fe9dc70f3b ("wifi: ath12k: Fix low MLO RX throughput on WCN7850")
>
> And since that adds the NULL check I think your patch is actually not needed?

Yes, since change a2fe9dc70f3b ("wifi: ath12k: Fix low MLO RX throughput 
on WCN7850") is already adding exactly the same check,

hence this patch is no longer required and can be dropped.

>
> /jeff
>