l2tp: Avoid possible recursive deadlock in l2tp_tunnel_register()

[ Upstream commit 9ca5e7ecab064f1f47da07f7c1ddf40e4bc0e5ac ]

When a file descriptor of pppol2tp socket is passed as file descriptor
of UDP socket, a recursive deadlock occurs in l2tp_tunnel_register().
This situation is reproduced by the following program:

int main(void)
{
	int sock;
	struct sockaddr_pppol2tp addr;

	sock = socket(AF_PPPOX, SOCK_DGRAM, PX_PROTO_OL2TP);
	if (sock < 0) {
		perror("socket");
		return 1;
	}

	addr.sa_family = AF_PPPOX;
	addr.sa_protocol = PX_PROTO_OL2TP;
	addr.pppol2tp.pid = 0;
	addr.pppol2tp.fd = sock;
	addr.pppol2tp.addr.sin_family = PF_INET;
	addr.pppol2tp.addr.sin_port = htons(0);
	addr.pppol2tp.addr.sin_addr.s_addr = inet_addr("192.168.0.1");
	addr.pppol2tp.s_tunnel = 1;
	addr.pppol2tp.s_session = 0;
	addr.pppol2tp.d_tunnel = 0;
	addr.pppol2tp.d_session = 0;

	if (connect(sock, (const struct sockaddr *)&addr, sizeof(addr)) < 0) {
		perror("connect");
		return 1;
	}

	return 0;
}

This program causes the following lockdep warning:

 ============================================
 WARNING: possible recursive locking detected
 6.2.0-rc5-00205-gc96618275234 #56 Not tainted
 --------------------------------------------
 repro/8607 is trying to acquire lock:
 ffff8880213c8130 (sk_lock-AF_PPPOX){+.+.}-{0:0}, at: l2tp_tunnel_register+0x2b7/0x11c0

 but task is already holding lock:
 ffff8880213c8130 (sk_lock-AF_PPPOX){+.+.}-{0:0}, at: pppol2tp_connect+0xa82/0x1a30

 other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(sk_lock-AF_PPPOX);
   lock(sk_lock-AF_PPPOX);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

 1 lock held by repro/8607:
  #0: ffff8880213c8130 (sk_lock-AF_PPPOX){+.+.}-{0:0}, at: pppol2tp_connect+0xa82/0x1a30

 stack backtrace:
 CPU: 0 PID: 8607 Comm: repro Not tainted 6.2.0-rc5-00205-gc96618275234 #56
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.1-2.fc37 04/01/2014
 Call Trace:
  <TASK>
  dump_stack_lvl+0x100/0x178
  __lock_acquire.cold+0x119/0x3b9
  ? lockdep_hardirqs_on_prepare+0x410/0x410
  lock_acquire+0x1e0/0x610
  ? l2tp_tunnel_register+0x2b7/0x11c0
  ? lock_downgrade+0x710/0x710
  ? __fget_files+0x283/0x3e0
  lock_sock_nested+0x3a/0xf0
  ? l2tp_tunnel_register+0x2b7/0x11c0
  l2tp_tunnel_register+0x2b7/0x11c0
  ? sprintf+0xc4/0x100
  ? l2tp_tunnel_del_work+0x6b0/0x6b0
  ? debug_object_deactivate+0x320/0x320
  ? lockdep_init_map_type+0x16d/0x7a0
  ? lockdep_init_map_type+0x16d/0x7a0
  ? l2tp_tunnel_create+0x2bf/0x4b0
  ? l2tp_tunnel_create+0x3c6/0x4b0
  pppol2tp_connect+0x14e1/0x1a30
  ? pppol2tp_put_sk+0xd0/0xd0
  ? aa_sk_perm+0x2b7/0xa80
  ? aa_af_perm+0x260/0x260
  ? bpf_lsm_socket_connect+0x9/0x10
  ? pppol2tp_put_sk+0xd0/0xd0
  __sys_connect_file+0x14f/0x190
  __sys_connect+0x133/0x160
  ? __sys_connect_file+0x190/0x190
  ? lockdep_hardirqs_on+0x7d/0x100
  ? ktime_get_coarse_real_ts64+0x1b7/0x200
  ? ktime_get_coarse_real_ts64+0x147/0x200
  ? __audit_syscall_entry+0x396/0x500
  __x64_sys_connect+0x72/0xb0
  do_syscall_64+0x38/0xb0
  entry_SYSCALL_64_after_hwframe+0x63/0xcd

This patch fixes the issue by getting/creating the tunnel before
locking the pppol2tp socket.

Fixes: 0b2c59720e65 ("l2tp: close all race conditions in l2tp_tunnel_register()")
Cc: Cong Wang <cong.wang@bytedance.com>
Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
Reviewed-by: Guillaume Nault <gnault@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index db2e584..f011af6 100644 (file)
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -650,54 +650,22 @@ static int pppol2tp_tunnel_mtu(const struct l2tp_tunnel *tunnel)
        return mtu - PPPOL2TP_HEADER_OVERHEAD;
 }
 
-/* connect() handler. Attach a PPPoX socket to a tunnel UDP socket
- */
-static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
-                           int sockaddr_len, int flags)
+static struct l2tp_tunnel *pppol2tp_tunnel_get(struct net *net,
+                                              const struct l2tp_connect_info *info,
+                                              bool *new_tunnel)
 {
-       struct sock *sk = sock->sk;
-       struct pppox_sock *po = pppox_sk(sk);
-       struct l2tp_session *session = NULL;
-       struct l2tp_connect_info info;
        struct l2tp_tunnel *tunnel;
-       struct pppol2tp_session *ps;
-       struct l2tp_session_cfg cfg = { 0, };
-       bool drop_refcnt = false;
-       bool drop_tunnel = false;
-       bool new_session = false;
-       bool new_tunnel = false;
        int error;
 
-       error = pppol2tp_sockaddr_get_info(uservaddr, sockaddr_len, &info);
-       if (error < 0)
-               return error;
+       *new_tunnel = false;
 
-       lock_sock(sk);
-
-       /* Check for already bound sockets */
-       error = -EBUSY;
-       if (sk->sk_state & PPPOX_CONNECTED)
-               goto end;
-
-       /* We don't supporting rebinding anyway */
-       error = -EALREADY;
-       if (sk->sk_user_data)
-               goto end; /* socket is already attached */
-
-       /* Don't bind if tunnel_id is 0 */
-       error = -EINVAL;
-       if (!info.tunnel_id)
-               goto end;
-
-       tunnel = l2tp_tunnel_get(sock_net(sk), info.tunnel_id);
-       if (tunnel)
-               drop_tunnel = true;
+       tunnel = l2tp_tunnel_get(net, info->tunnel_id);
 
        /* Special case: create tunnel context if session_id and
         * peer_session_id is 0. Otherwise look up tunnel using supplied
         * tunnel id.
         */
-       if (!info.session_id && !info.peer_session_id) {
+       if (!info->session_id && !info->peer_session_id) {
                if (!tunnel) {
                        struct l2tp_tunnel_cfg tcfg = {
                                .encap = L2TP_ENCAPTYPE_UDP,
@@ -706,40 +674,82 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
                        /* Prevent l2tp_tunnel_register() from trying to set up
                         * a kernel socket.
                         */
-                       if (info.fd < 0) {
-                               error = -EBADF;
-                               goto end;
-                       }
+                       if (info->fd < 0)
+                               return ERR_PTR(-EBADF);
 
-                       error = l2tp_tunnel_create(info.fd,
-                                                  info.version,
-                                                  info.tunnel_id,
-                                                  info.peer_tunnel_id, &tcfg,
+                       error = l2tp_tunnel_create(info->fd,
+                                                  info->version,
+                                                  info->tunnel_id,
+                                                  info->peer_tunnel_id, &tcfg,
                                                   &tunnel);
                        if (error < 0)
-                               goto end;
+                               return ERR_PTR(error);
 
                        l2tp_tunnel_inc_refcount(tunnel);
-                       error = l2tp_tunnel_register(tunnel, sock_net(sk),
-                                                    &tcfg);
+                       error = l2tp_tunnel_register(tunnel, net, &tcfg);
                        if (error < 0) {
                                kfree(tunnel);
-                               goto end;
+                               return ERR_PTR(error);
                        }
-                       drop_tunnel = true;
-                       new_tunnel = true;
+
+                       *new_tunnel = true;
                }
        } else {
                /* Error if we can't find the tunnel */
-               error = -ENOENT;
                if (!tunnel)
-                       goto end;
+                       return ERR_PTR(-ENOENT);
 
                /* Error if socket is not prepped */
-               if (!tunnel->sock)
-                       goto end;
+               if (!tunnel->sock) {
+                       l2tp_tunnel_dec_refcount(tunnel);
+                       return ERR_PTR(-ENOENT);
+               }
        }
 
+       return tunnel;
+}
+
+/* connect() handler. Attach a PPPoX socket to a tunnel UDP socket
+ */
+static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
+                           int sockaddr_len, int flags)
+{
+       struct sock *sk = sock->sk;
+       struct pppox_sock *po = pppox_sk(sk);
+       struct l2tp_session *session = NULL;
+       struct l2tp_connect_info info;
+       struct l2tp_tunnel *tunnel;
+       struct pppol2tp_session *ps;
+       struct l2tp_session_cfg cfg = { 0, };
+       bool drop_refcnt = false;
+       bool new_session = false;
+       bool new_tunnel = false;
+       int error;
+
+       error = pppol2tp_sockaddr_get_info(uservaddr, sockaddr_len, &info);
+       if (error < 0)
+               return error;
+
+       /* Don't bind if tunnel_id is 0 */
+       if (!info.tunnel_id)
+               return -EINVAL;
+
+       tunnel = pppol2tp_tunnel_get(sock_net(sk), &info, &new_tunnel);
+       if (IS_ERR(tunnel))
+               return PTR_ERR(tunnel);
+
+       lock_sock(sk);
+
+       /* Check for already bound sockets */
+       error = -EBUSY;
+       if (sk->sk_state & PPPOX_CONNECTED)
+               goto end;
+
+       /* We don't supporting rebinding anyway */
+       error = -EALREADY;
+       if (sk->sk_user_data)
+               goto end; /* socket is already attached */
+
        if (tunnel->peer_tunnel_id == 0)
                tunnel->peer_tunnel_id = info.peer_tunnel_id;
 
@@ -840,8 +850,7 @@ end:
        }
        if (drop_refcnt)
                l2tp_session_dec_refcount(session);
-       if (drop_tunnel)
-               l2tp_tunnel_dec_refcount(tunnel);
+       l2tp_tunnel_dec_refcount(tunnel);
        release_sock(sk);
 
        return error;