On Wed, 2023-11-29 at 16:18 +0800, Edward Adam Davis wrote: > On Wed, 29 Nov 2023 07:57:07 +0100, Johannes Berg wrote: > > > [Analysis] > > > When ieee80211_get_link_sband() fails to find a valid sband and first checks > > > for params in sta_link_apply_parameters(), it will return 0 due to new_link > > > being 0, which will lead to an incorrect process after sta_apply_parameters(). > > > > > > [Fix] > > > First obtain sband and perform a non null check before checking the params. > > > > Not sure I can even disagree with that analysis, it seems right, but ... > > > > > + if (!link || !link_sta) > > > + return -EINVAL; > > > + > > > + sband = ieee80211_get_link_sband(link); > > > + if (!sband) > > > + return -EINVAL; > > > + > > > /* > > > * If there are no changes, then accept a link that doesn't exist, > > > * unless it's a new link. > > > > There's a comment here which is clearly not true after this change, > > since you've already returned for !link_sta? > No, after applying my patch, it will return due to !sband. > Right, OK, but the way I read the comment (now) is that it wanted to accept it in that case? That said, I just threw the patch into our internal testing machinery quickly (probably has more MLO tests than upstream hostap for now), and it worked just fine ... Maybe we should just remove the comment? johannes
