On Tue, Apr 13, 2021 at 3:10 PM Gong, Sishuai <sish...@purdue.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.

Reply via email to