* [PATCH] mac80211: mesh: fix RCU warning
@ 2019-05-15 22:21 Thomas Pedersen
2019-05-15 22:24 ` Thomas Pedersen
2019-05-24 8:29 ` Johannes Berg
0 siblings, 2 replies; 4+ messages in thread
From: Thomas Pedersen @ 2019-05-15 22:21 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg, peter.oh, Thomas Pedersen
ifmsh->csa was being dereferenced without the RCU read
lock held.
fixes the following warning:
[ 74.930435] =============================
[ 74.932066] WARNING: suspicious RCU usage
[ 74.933671] 4.20.13 #5 Tainted: G W
[ 74.935804] -----------------------------
[ 74.937427] net/mac80211/mesh.c:1218 suspicious rcu_dereference_check() usage!
[ 74.940473] other info that might help us debug this:
[ 74.943654] rcu_scheduler_active = 2, debug_locks = 1
[ 74.946311] 5 locks held by kworker/u8:3/107:
[ 74.948087] #0: 000000007623c1f0 ((wq_completion)"%s"wiphy_name(local->hw.wiphy)){+.+.}, at: process_one_work+0x1a2/0x610
[ 74.952464] #1: 00000000077b4215 ((work_completion)(&sdata->csa_finalize_work)){+.+.}, at: process_one_work+0x1a2/0x610
[ 74.957228] #2: 00000000e02b12da (&wdev->mtx){+.+.}, at: ieee80211_csa_finalize_work+0x2f/0x90
[ 74.959870] #3: 00000000e6855095 (&local->mtx){+.+.}, at: ieee80211_csa_finalize_work+0x47/0x90
[ 74.962937] #4: 00000000bb5e3bca (&local->chanctx_mtx){+.+.}, at: ieee80211_csa_finalize_work+0x51/0x90
Signed-off-by: Thomas Pedersen <thomas@eero.com>
---
net/mac80211/mesh.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 766e5e5..70aeb34 100644
--- a/net/mac80211/mesh.c
+++ 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();
ret = ieee80211_mesh_rebuild_beacon(sdata);
if (ret)
return -EINVAL;
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] mac80211: mesh: fix RCU warning
2019-05-15 22:21 [PATCH] mac80211: mesh: fix RCU warning Thomas Pedersen
@ 2019-05-15 22:24 ` Thomas Pedersen
2019-05-24 8:29 ` Johannes Berg
1 sibling, 0 replies; 4+ messages in thread
From: Thomas Pedersen @ 2019-05-15 22:24 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg, Peter Oh
On Wed, May 15, 2019 at 3:21 PM Thomas Pedersen <thomas@eero.com> wrote:
>
> ifmsh->csa was being dereferenced without the RCU read
> lock held.
>
> fixes the following warning:
>
> [ 74.930435] =============================
> [ 74.932066] WARNING: suspicious RCU usage
> [ 74.933671] 4.20.13 #5 Tainted: G W
> [ 74.935804] -----------------------------
> [ 74.937427] net/mac80211/mesh.c:1218 suspicious rcu_dereference_check() usage!
> [ 74.940473] other info that might help us debug this:
> [ 74.943654] rcu_scheduler_active = 2, debug_locks = 1
> [ 74.946311] 5 locks held by kworker/u8:3/107:
> [ 74.948087] #0: 000000007623c1f0 ((wq_completion)"%s"wiphy_name(local->hw.wiphy)){+.+.}, at: process_one_work+0x1a2/0x610
> [ 74.952464] #1: 00000000077b4215 ((work_completion)(&sdata->csa_finalize_work)){+.+.}, at: process_one_work+0x1a2/0x610
> [ 74.957228] #2: 00000000e02b12da (&wdev->mtx){+.+.}, at: ieee80211_csa_finalize_work+0x2f/0x90
> [ 74.959870] #3: 00000000e6855095 (&local->mtx){+.+.}, at: ieee80211_csa_finalize_work+0x47/0x90
> [ 74.962937] #4: 00000000bb5e3bca (&local->chanctx_mtx){+.+.}, at: ieee80211_csa_finalize_work+0x51/0x90
Sorry the commit message is a little out of date, I actually tested on
5.1.0-rc7-wt as well.
> Signed-off-by: Thomas Pedersen <thomas@eero.com>
> ---
> net/mac80211/mesh.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
> index 766e5e5..70aeb34 100644
> --- a/net/mac80211/mesh.c
> +++ 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();
> ret = ieee80211_mesh_rebuild_beacon(sdata);
> if (ret)
> return -EINVAL;
> --
> 2.7.4
>
--
thomas
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mac80211: mesh: fix RCU warning
2019-05-15 22:21 [PATCH] mac80211: mesh: fix RCU warning Thomas Pedersen
2019-05-15 22:24 ` Thomas Pedersen
@ 2019-05-24 8:29 ` Johannes Berg
2019-05-25 3:46 ` Thomas Pedersen
1 sibling, 1 reply; 4+ messages in thread
From: Johannes Berg @ 2019-05-24 8:29 UTC (permalink / raw)
To: Thomas Pedersen, linux-wireless; +Cc: peter.oh
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:
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mac80211: mesh: fix RCU warning
2019-05-24 8:29 ` Johannes Berg
@ 2019-05-25 3:46 ` Thomas Pedersen
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Pedersen @ 2019-05-25 3:46 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Peter Oh
On Fri, May 24, 2019 at 1:29 AM Johannes Berg <johannes@sipsolutions.net> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-25 3:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-15 22:21 [PATCH] mac80211: mesh: fix RCU warning Thomas Pedersen
2019-05-15 22:24 ` Thomas Pedersen
2019-05-24 8:29 ` Johannes Berg
2019-05-25 3:46 ` Thomas Pedersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.