netdev - Re: WARNING: locking bug in inet

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <YydimPlesKO+4QKG@boqun-archlinux>
Date:   Sun, 18 Sep 2022 11:25:28 -0700
From:   Boqun Feng <boqun.feng@...il.com>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
        Waiman Long <longman@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        syzbot <syzbot+94cc2a66fc228b23f360@...kaller.appspotmail.com>,
        syzkaller-bugs@...glegroups.com
Subject: Re: WARNING: locking bug in inet_autobind

On Mon, Sep 19, 2022 at 12:52:45AM +0900, Tetsuo Handa wrote:
> syzbot is reporting locking bug in inet_autobind(), for
> commit 37159ef2c1ae1e69 ("l2tp: fix a lockdep splat") started
> calling 
> 
>   lockdep_set_class_and_name(&sk->sk_lock.slock, &l2tp_socket_class, "l2tp_sock")
> 
> in l2tp_tunnel_create() (which is currently in l2tp_tunnel_register()).
> How can we fix this problem?
> 

Just a theory, it seems that we have a memory corruption happened for
lockdep_set_class_and_name(), in l2tp_tunnel_register(), the "sk" gets
published before lockdep_set_class_and_name():

	tunnel->sock = sk;
	...
	lockdep_set_class_and_name(&sk->sk_lock.slock,...);

And what could happen is that sock_lock_init() races with the
l2tp_tunnel_register(), which results into two
lockdep_set_class_and_name()s race with each other.

Anyway, "sk" should not be published until its lock gets properly
initialized, could you try the following (untested)? Looks to me all
other code around the lockdep_set_class_and_name() should be moved
upwards, but I don't want to pretend I'm an expert ;-)

Regards,
Boqun

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 7499c51b1850..1a01d23abc53 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1480,7 +1480,9 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net,

        sk = sock->sk;
        sock_hold(sk);
-       tunnel->sock = sk;
+       lockdep_set_class_and_name(&sk->sk_lock.slock, &l2tp_socket_class,
+                                  "l2tp_sock");
+       smp_store_release(&tunnel->sock, sk);

        spin_lock_bh(&pn->l2tp_tunnel_list_lock);
        list_for_each_entry(tunnel_walk, &pn->l2tp_tunnel_list, list) {
@@ -1509,8 +1511,6 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net,

        tunnel->old_sk_destruct = sk->sk_destruct;
        sk->sk_destruct = &l2tp_tunnel_destruct;
-       lockdep_set_class_and_name(&sk->sk_lock.slock, &l2tp_socket_class,
-                                  "l2tp_sock");
        sk->sk_allocation = GFP_ATOMIC;

        trace_register_tunnel(tunnel);  

>   ------------[ cut here ]------------
>   class->name=slock-AF_INET6 lock->name=l2tp_sock lock->key=l2tp_socket_class
>   WARNING: CPU: 2 PID: 9237 at kernel/locking/lockdep.c:940 look_up_lock_class+0xcc/0x140
>   Modules linked in:
>   CPU: 2 PID: 9237 Comm: a.out Not tainted 6.0.0-rc5-00094-ga335366bad13-dirty #860
>   Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
>   RIP: 0010:look_up_lock_class+0xcc/0x140
> 
> On 2019/05/16 14:46, syzbot wrote:
> > HEAD commit:    35c99ffa Merge tag 'for_linus' of git://git.kernel.org/pub..
> > git tree:       net-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=10e970f4a00000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=82f0809e8f0a8c87
> > dashboard link: https://syzkaller.appspot.com/bug?extid=94cc2a66fc228b23f360
> > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> 
> C reproducer is available at
> https://syzkaller.appspot.com/text?tag=ReproC&x=15062310080000 .
>