netdev - Re: [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAK+SQuS7zGpAz4cUBfXUKOdgaPDK2y97-vUx=0mxOFp0iN9pJg@mail.gmail.com>
Date:   Fri, 11 Feb 2022 16:01:42 +0900
From:   Juhee Kang <claudiajkang@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     davem@...emloft.net, Jakub Kicinski <kuba@...nel.org>,
        Networking <netdev@...r.kernel.org>, ennoerlangen@...il.com,
        george.mccollister@...il.com, Vladimir Oltean <olteanv@...il.com>,
        marco.wenzel@...berle.de,
        syzbot+f0eb4f3876de066b128c@...kaller.appspotmail.com
Subject: Re: [PATCH v2 net-next] net: hsr: fix suspicious usage in hsr_node_get_first()

On Fri, Feb 11, 2022 at 3:17 AM Eric Dumazet <eric.dumazet@...il.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@...il.com>
> >> Reported-by: syzbot+f0eb4f3876de066b128c@...kaller.appspotmail.com
> >> Signed-off-by: Juhee Kang <claudiajkang@...il.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