Hi,

On Sun, 2022-01-30 at 22:40 +0100, Martin Blumenstingl wrote:

On Fri, Jan 28, 2022 at 1:51 AM Pkshih [off-list ref] wrote:
[...]

quoted

quoted

quoted

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