Hi, On Sun, 2022-01-30 at 22:40 +0100, Martin Blumenstingl wrote: > > On Fri, Jan 28, 2022 at 1:51 AM Pkshih <pkshih@xxxxxxxxxxx> wrote: > [...] > > > > To avoid this, we can add a flag to struct rtw_vif, and set this flag > > > > when ::remove_interface. Then, only collect vif without this flag into list > > > > when we use iterate_actiom(). > > > > > > > > As well as ieee80211_sta can do similar fix. > > > > > > > > I would prefer my method that adds a 'bool disabled' flag to struct rtw_vif/rtw_sta > > and set it when ::remove_interface/::sta_remove. Then rtw_iterate_stas() can > > check this flag to decide whether does thing or not. > That would indeed be a very straight forward approach and easy to read. > In net/mac80211/iface.c there's some cases where after > drv_remove_interface() (which internally calls our .remove_interface > op) will kfree the vif (sdata). Doesn't that then result in a > use-after-free if we rely on a boolean within rtw_vif? The rtw_vif is drv_priv of ieee80211_vif, and they will be freed at the same time. We must set 'bool disabled' after holding rtwdev->mutex lock, and check this flag in iterator of ieee80211_iterate_active_interfaces_atomic() to contruct a list of vif. That means we never access this flag out of rtwdev->mutx or iterator. Does it make sense? -- Ping-Ke
