[PATCH] wifi: mac80211: fix cfg80211_bss always hold when assoc response fail for MLO connection

Wen Gong quic_wgong at quicinc.com
Mon Aug 21 00:48:04 PDT 2023


On 8/21/2023 3:40 PM, Johannes Berg wrote:
> On Mon, 2023-08-21 at 02:13 -0400, Wen Gong wrote:
>> +++ b/net/mac80211/mlme.c
>> @@ -5429,17 +5429,22 @@ static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata,
>>   	for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
>>   		struct ieee80211_link_data *link;
>>   
>> -		link = sdata_dereference(sdata->link[link_id], sdata);
>> -		if (!link)
>> -			continue;
>> -
>>   		if (!assoc_data->link[link_id].bss)
>>   			continue;
>>   
>>   		resp.links[link_id].bss = assoc_data->link[link_id].bss;
>> -		resp.links[link_id].addr = link->conf->addr;
>>   		resp.links[link_id].status = assoc_data->link[link_id].status;
>>   
>> +		link = sdata_dereference(sdata->link[link_id], sdata);
>> +
>> +		if (!link) {
>> +			/* use the addr of assoc_data link which is set in ieee80211_mgd_assoc() */
>> +			resp.links[link_id].addr = assoc_data->link[link_id].addr;
> As I mentioned before, this cannot be done - it introduces use-after-
> free since assoc_data is freed after the loop, and the
> cfg80211_rx_assoc_resp() is after that.

So I want to change the "const u8 *addr" to "u8 addr[ETH_ALEN] 
__aligned(2);" of struct

cfg80211_rx_assoc_resp and copy the value, then no use-after-free, is it OK?

>> +			continue;
>> +		}
>> +
>> +		resp.links[link_id].addr = link->conf->addr;
>>
> Also, I don't see that we need to use two different assignments for the
> two cases.
Then I will change to both use "assoc_data->link[link_id].addr", is it OK?
> johannes



More information about the ath12k mailing list