netdev - Re: A concurrency bug between l2tp_tunnel_register() and l2tp_xmit

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] [day] [month] [year] [list]

Date:   Wed, 14 Apr 2021 13:07:01 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     "Gong, Sishuai" <sishuai@...due.edu>
Cc:     "jchapman@...alix.com" <jchapman@...alix.com>,
        "tparkin@...alix.com" <tparkin@...alix.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: A concurrency bug between l2tp_tunnel_register() and l2tp_xmit_core()

On Tue, Apr 13, 2021 at 3:10 PM Gong, Sishuai <sishuai@...due.edu> wrote:
>
> Hi,
>
> We found a concurrency bug in linux 5.12-rc3 and we are able to reproduce it under x86. This bug happens when two l2tp functions l2tp_tunnel_register() and l2tp_xmit_core() are running in parallel. In general, l2tp_tunnel_register() registered a tunnel that hasn’t been fully initialized and then l2tp_xmit_core() tries to access an uninitialized attribute. The interleaving is shown below..
>
> ------------------------------------------
> Execution interleaving
>
> Thread 1                                                                                                Thread 2
>
> l2tp_tunnel_register()
>         spin_lock_bh(&pn->l2tp_tunnel_list_lock);
>                 …
>                 list_add_rcu(&tunnel->list, &pn->l2tp_tunnel_list);
>                 // tunnel becomes visible
>         spin_unlock_bh(&pn->l2tp_tunnel_list_lock);
>                                                                                                         pppol2tp_connect()
>                                                                                                                 …
>                                                                                                                 tunnel = l2tp_tunnel_get(sock_net(sk), info.tunnel_id);
>                                                                                                                 // Successfully get the new tunnel
>                                                                                                         …
>                                                                                                         l2tp_xmit_core()
>                                                                                                                 struct sock *sk = tunnel->sock;
>                                                                                                                 // uninitialized, sk=0
>                                                                                                                 …
>                                                                                                                 bh_lock_sock(sk);
>                                                                                                                 // Null-pointer exception happens
>         …
>         tunnel->sock = sk;
>
> ------------------------------------------
> Impact & fix
>
> This bug causes a kernel NULL pointer deference error, as attached below. Currently, we think a potential fix is to initialize tunnel->sock before adding the tunnel into l2tp_tunnel_list.

I think this is the right fix. Please submit a patch formally.

Thanks.