[PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first()

Subsystems: hsr network protocol, networking [general], the rest

STALE1676d

4 messages, 2 authors, 2022-02-11 · open the first message on its own page

[PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first()

From: Juhee Kang <hidden>
Date: 2022-02-10 16:23:58

Currently, to dereference hlist_node which is result of hlist_first_rcu(),
rcu_dereference() is used. But, suspicious RCU warnings occur because
the caller doesn't acquire RCU. So it was solved by adding rcu_read_lock().

The kernel test robot reports:
    [   53.750001][ T3597] =============================
    [   53.754849][ T3597] WARNING: suspicious RCU usage
    [   53.759833][ T3597] 5.17.0-rc2-syzkaller-00903-g45230829827b #0 Not tainted
    [   53.766947][ T3597] -----------------------------
    [   53.771840][ T3597] net/hsr/hsr_framereg.c:34 suspicious rcu_dereference_check() usage!
    [   53.780129][ T3597] other info that might help us debug this:
    [   53.790594][ T3597] rcu_scheduler_active = 2, debug_locks = 1
    [   53.798896][ T3597] 2 locks held by syz-executor.0/3597:

Fixes: 4acc45db7115 ("net: hsr: use hlist_head instead of list_head for mac addresses")
Reported-by: Eric Dumazet <redacted>
Reported-by: syzbot+f0eb4f3876de066b128c@syzkaller.appspotmail.com
Signed-off-by: Juhee Kang <redacted>
---
v2:
 - rebase current net-next tree

 net/hsr/hsr_framereg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
index b3c6ffa1894d..92abdf855327 100644
--- a/net/hsr/hsr_framereg.c
+++ b/net/hsr/hsr_framereg.c

@@ -31,7 +31,10 @@ struct hsr_node *hsr_node_get_first(struct hlist_head *head)
 {
 	struct hlist_node *first;
 
+	rcu_read_lock();
 	first = rcu_dereference(hlist_first_rcu(head));
+	rcu_read_unlock();
+
 	if (first)
 		return hlist_entry(first, struct hsr_node, mac_list);

-- 
2.25.1

Re: [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first()

From: Eric Dumazet <hidden>
Date: 2022-02-10 17:47:50

On 2/10/22 08:23, Juhee Kang wrote:

Currently, to dereference hlist_node which is result of hlist_first_rcu(),
rcu_dereference() is used. But, suspicious RCU warnings occur because
the caller doesn't acquire RCU. So it was solved by adding rcu_read_lock().

The kernel test robot reports:
     [   53.750001][ T3597] =============================
     [   53.754849][ T3597] WARNING: suspicious RCU usage
     [   53.759833][ T3597] 5.17.0-rc2-syzkaller-00903-g45230829827b #0 Not tainted
     [   53.766947][ T3597] -----------------------------
     [   53.771840][ T3597] net/hsr/hsr_framereg.c:34 suspicious rcu_dereference_check() usage!
     [   53.780129][ T3597] other info that might help us debug this:
     [   53.790594][ T3597] rcu_scheduler_active = 2, debug_locks = 1
     [   53.798896][ T3597] 2 locks held by syz-executor.0/3597:


Please include whole stack.

quoted hunk

Fixes: 4acc45db7115 ("net: hsr: use hlist_head instead of list_head for mac addresses")
Reported-by: Eric Dumazet <redacted>
Reported-by: syzbot+f0eb4f3876de066b128c@syzkaller.appspotmail.com
Signed-off-by: Juhee Kang <redacted>
---
v2:
  - rebase current net-next tree

  net/hsr/hsr_framereg.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
index b3c6ffa1894d..92abdf855327 100644
--- a/net/hsr/hsr_framereg.c
+++ b/net/hsr/hsr_framereg.c

@@ -31,7 +31,10 @@ struct hsr_node *hsr_node_get_first(struct hlist_head *head)
  {
  	struct hlist_node *first;
  
+	rcu_read_lock();
  	first = rcu_dereference(hlist_first_rcu(head));
+	rcu_read_unlock();
+
  	if (first)
  		return hlist_entry(first, struct hsr_node, mac_list);


This is not fixing anything, just silence the warning.

Re: [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first()

From: Eric Dumazet <hidden>
Date: 2022-02-10 18:17:46

On 2/10/22 09:47, Eric Dumazet wrote:

On 2/10/22 08:23, Juhee Kang wrote:

quoted

Currently, to dereference hlist_node which is result of 
hlist_first_rcu(),
rcu_dereference() is used. But, suspicious RCU warnings occur because
the caller doesn't acquire RCU. So it was solved by adding 
rcu_read_lock().

The kernel test robot reports:
     [   53.750001][ T3597] =============================
     [   53.754849][ T3597] WARNING: suspicious RCU usage
     [   53.759833][ T3597] 5.17.0-rc2-syzkaller-00903-g45230829827b 
#0 Not tainted
     [   53.766947][ T3597] -----------------------------
     [   53.771840][ T3597] net/hsr/hsr_framereg.c:34 suspicious 
rcu_dereference_check() usage!
     [   53.780129][ T3597] other info that might help us debug this:
     [   53.790594][ T3597] rcu_scheduler_active = 2, debug_locks = 1
     [   53.798896][ T3597] 2 locks held by syz-executor.0/3597:


Please include whole stack.

quoted

Fixes: 4acc45db7115 ("net: hsr: use hlist_head instead of list_head 
for mac addresses")
Reported-by: Eric Dumazet <redacted>
Reported-by: syzbot+f0eb4f3876de066b128c@syzkaller.appspotmail.com
Signed-off-by: Juhee Kang <redacted>
---
v2:
  - rebase current net-next tree

  net/hsr/hsr_framereg.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
index b3c6ffa1894d..92abdf855327 100644
--- a/net/hsr/hsr_framereg.c
+++ b/net/hsr/hsr_framereg.c

@@ -31,7 +31,10 @@ struct hsr_node *hsr_node_get_first(struct

hlist_head *head)
  {
      struct hlist_node *first;
  +    rcu_read_lock();
      first = rcu_dereference(hlist_first_rcu(head));
+    rcu_read_unlock();
+
      if (first)
          return hlist_entry(first, struct hsr_node, mac_list);


This is not fixing anything, just silence the warning.



I suggest replacing rcu_dereference() by rcu_dereference_rtnl()

Re: [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first()

From: Juhee Kang <hidden>
Date: 2022-02-11 07:02:21

On Fri, Feb 11, 2022 at 3:17 AM Eric Dumazet [off-list ref] wrote:


On 2/10/22 09:47, Eric Dumazet wrote:

quoted

On 2/10/22 08:23, Juhee Kang wrote:

quoted

Currently, to dereference hlist_node which is result of
hlist_first_rcu(),
rcu_dereference() is used. But, suspicious RCU warnings occur because
the caller doesn't acquire RCU. So it was solved by adding
rcu_read_lock().

The kernel test robot reports:
     [   53.750001][ T3597] =============================
     [   53.754849][ T3597] WARNING: suspicious RCU usage
     [   53.759833][ T3597] 5.17.0-rc2-syzkaller-00903-g45230829827b
#0 Not tainted
     [   53.766947][ T3597] -----------------------------
     [   53.771840][ T3597] net/hsr/hsr_framereg.c:34 suspicious
rcu_dereference_check() usage!
     [   53.780129][ T3597] other info that might help us debug this:
     [   53.790594][ T3597] rcu_scheduler_active = 2, debug_locks = 1
     [   53.798896][ T3597] 2 locks held by syz-executor.0/3597:


Please include whole stack.

quoted

Fixes: 4acc45db7115 ("net: hsr: use hlist_head instead of list_head
for mac addresses")
Reported-by: Eric Dumazet <redacted>
Reported-by: syzbot+f0eb4f3876de066b128c@syzkaller.appspotmail.com
Signed-off-by: Juhee Kang <redacted>
---
v2:
  - rebase current net-next tree

  net/hsr/hsr_framereg.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
index b3c6ffa1894d..92abdf855327 100644
--- a/net/hsr/hsr_framereg.c
+++ b/net/hsr/hsr_framereg.c

@@ -31,7 +31,10 @@ struct hsr_node *hsr_node_get_first(struct

hlist_head *head)
  {
      struct hlist_node *first;
  +    rcu_read_lock();
      first = rcu_dereference(hlist_first_rcu(head));
+    rcu_read_unlock();
+
      if (first)
          return hlist_entry(first, struct hsr_node, mac_list);


This is not fixing anything, just silence the warning.



I suggest replacing rcu_dereference() by rcu_dereference_rtnl()

Hi Eric,
Thank you for your review!

I will send a v3 patch that applies to your opinion after some tests.

Thank you so much for catching it!

-- 

Best regards,
Juhee Kang

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help