[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