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.