On Fri, May 24, 2019 at 1:29 AM Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote: > > On Wed, 2019-05-15 at 15:21 -0700, Thomas Pedersen wrote: > > ifmsh->csa was being dereferenced without the RCU read > > lock held. > > > +++ b/net/mac80211/mesh.c > > @@ -1220,10 +1220,12 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata) > > ifmsh->chsw_ttl = 0; > > > > /* Remove the CSA and MCSP elements from the beacon */ > > + rcu_read_lock(); > > tmp_csa_settings = rcu_dereference(ifmsh->csa); > > RCU_INIT_POINTER(ifmsh->csa, NULL); > > if (tmp_csa_settings) > > kfree_rcu(tmp_csa_settings, rcu_head); > > + rcu_read_unlock(); > > This seems wrong to me. > > Really this code is the *writer* side, so you should do something like > this: Thanks this looks correct. I should've thought about this a tiny bit more ;) > diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c > index 766e5e5bab8a..d578147ad7e8 100644 > --- a/net/mac80211/mesh.c > +++ b/net/mac80211/mesh.c > @@ -1220,7 +1220,8 @@ int ieee80211_mesh_finish_csa(struct > ieee80211_sub_if_data *sdata) > ifmsh->chsw_ttl = 0; > > /* Remove the CSA and MCSP elements from the beacon */ > - tmp_csa_settings = rcu_dereference(ifmsh->csa); > + tmp_csa_settings = rcu_dereference_protected(ifmsh->csa, > + lockdep_is_held(&sdata->wdev.mtx)); > RCU_INIT_POINTER(ifmsh->csa, NULL); > if (tmp_csa_settings) > kfree_rcu(tmp_csa_settings, rcu_head); > @@ -1242,6 +1243,8 @@ int ieee80211_mesh_csa_beacon(struct > ieee80211_sub_if_data *sdata, > struct mesh_csa_settings *tmp_csa_settings; > int ret = 0; > > + lockdep_assert_held(&sdata->wdev.mtx); > + > tmp_csa_settings = kmalloc(sizeof(*tmp_csa_settings), > GFP_ATOMIC); > if (!tmp_csa_settings) > > > Can you test that and send a proper patch? > > johannes > -- thomas
