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
