All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first()
@ 2022-02-10 16:23 Juhee Kang
  2022-02-10 17:47 ` Eric Dumazet
  0 siblings, 1 reply; 4+ messages in thread
From: Juhee Kang @ 2022-02-10 16:23 UTC (permalink / raw)
  To: davem, kuba, netdev, eric.dumazet
  Cc: ennoerlangen, george.mccollister, olteanv, marco.wenzel,
	syzbot+f0eb4f3876de066b128c

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 <eric.dumazet@gmail.com>
Reported-by: syzbot+f0eb4f3876de066b128c@syzkaller.appspotmail.com
Signed-off-by: Juhee Kang <claudiajkang@gmail.com>
---
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


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first()
  2022-02-10 16:23 [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first() Juhee Kang
@ 2022-02-10 17:47 ` Eric Dumazet
  2022-02-10 18:17   ` Eric Dumazet
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2022-02-10 17:47 UTC (permalink / raw)
  To: Juhee Kang, davem, kuba, netdev
  Cc: ennoerlangen, george.mccollister, olteanv, marco.wenzel,
	syzbot+f0eb4f3876de066b128c


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.


>
> Fixes: 4acc45db7115 ("net: hsr: use hlist_head instead of list_head for mac addresses")
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> Reported-by: syzbot+f0eb4f3876de066b128c@syzkaller.appspotmail.com
> Signed-off-by: Juhee Kang <claudiajkang@gmail.com>
> ---
> 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.



^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first()
  2022-02-10 17:47 ` Eric Dumazet
@ 2022-02-10 18:17   ` Eric Dumazet
  2022-02-11  7:01     ` Juhee Kang
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2022-02-10 18:17 UTC (permalink / raw)
  To: Juhee Kang, davem, kuba, netdev
  Cc: ennoerlangen, george.mccollister, olteanv, marco.wenzel,
	syzbot+f0eb4f3876de066b128c


On 2/10/22 09:47, Eric Dumazet wrote:
>
> 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.
>
>
>>
>> Fixes: 4acc45db7115 ("net: hsr: use hlist_head instead of list_head 
>> for mac addresses")
>> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
>> Reported-by: syzbot+f0eb4f3876de066b128c@syzkaller.appspotmail.com
>> Signed-off-by: Juhee Kang <claudiajkang@gmail.com>
>> ---
>> 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()




^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first()
  2022-02-10 18:17   ` Eric Dumazet
@ 2022-02-11  7:01     ` Juhee Kang
  0 siblings, 0 replies; 4+ messages in thread
From: Juhee Kang @ 2022-02-11  7:01 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: davem, Jakub Kicinski, Networking, ennoerlangen,
	george.mccollister, Vladimir Oltean, marco.wenzel,
	syzbot+f0eb4f3876de066b128c

On Fri, Feb 11, 2022 at 3:17 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 2/10/22 09:47, Eric Dumazet wrote:
> >
> > 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.
> >
> >
> >>
> >> Fixes: 4acc45db7115 ("net: hsr: use hlist_head instead of list_head
> >> for mac addresses")
> >> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> >> Reported-by: syzbot+f0eb4f3876de066b128c@syzkaller.appspotmail.com
> >> Signed-off-by: Juhee Kang <claudiajkang@gmail.com>
> >> ---
> >> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-11  7:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-10 16:23 [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first() Juhee Kang
2022-02-10 17:47 ` Eric Dumazet
2022-02-10 18:17   ` Eric Dumazet
2022-02-11  7:01     ` Juhee Kang

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.