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

[Johannes Berg]

On Mon, 2023-08-21 at 15:48 +0800, Wen Gong wrote:
> 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?

Yeah I guess that works too.

> 
> > > +			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?

Sure

johannes