Hi, > My case is: > When connect with 2 links AP, the cfg80211_hold_bss() is called by > cfg80211_mlme_assoc() > for each BSS of the 2 links, > > When asssocResp from AP is not success(such as status_code==1), the > ieee80211_link_data of 2nd link(sdata->link[link_id]) > is NULL because ieee80211_assoc_success()->ieee80211_vif_update_links() > is not called. > > Then struct cfg80211_rx_assoc_resp resp in cfg80211_rx_assoc_resp() and > struct cfg80211_connect_resp_params cr in __cfg80211_connect_result() > will only have the data of > the 1st link, and finally cfg80211_connect_result_release_bsses() only > call cfg80211_unhold_bss() > for the 1st link, then BSS of the 2nd link will never free because its > hold is awlays > 0 now. Hah, yes, ouch. > I found it is not easy to refine it, so do you have any advise/idea? > > 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; > But is it really so hard? We only need link for link->conf->addr, and we can use assoc_data->link[link_id].addr for that instead, no? We need to store those locally to avoid a use-after-free, but that's at most 15 addresses on the stack, which seems acceptable? Oh and there's the uapsd stuff but that only matters in the success case anyway. In fact I'm not even sure the address matters in the unsuccessful case but we have it pretty easily. johannes