From patchwork Fri Feb 9 15:00:17 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871438 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJFb4Cb2z9s1h for ; Sat, 10 Feb 2018 02:02:27 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751151AbeBIPCV (ORCPT ); Fri, 9 Feb 2018 10:02:21 -0500 Received: from mail.katalix.com ([82.103.140.233]:38412 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751285AbeBIPAg (ORCPT ); Fri, 9 Feb 2018 10:00:36 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id 7E0277E2CF for ; Fri, 9 Feb 2018 15:00:34 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 01/16] l2tp: update sk_user_data while holding sk_callback_lock Date: Fri, 9 Feb 2018 15:00:17 +0000 Message-Id: <1518188432-9245-2-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Since L2TP hooks on sockets opened by userspace using sk_user_data, we may race with other socket families that attempt to use the same socket. This problem was discovered by syzbot using AF_KCM. KCM has since been modified to use only TCP sockets to avoid hitting this issue but we should prevent such races in L2TP anyway. Fixes: c8fffcea0a079 ("l2tp: Refactor l2tp core driver to make use of the common UDP tunnel function") Reported-by: syzbot+8865eaff7f9acd593945@syzkaller.appspotmail.com Kernel BUG at net/l2tp/l2tp_ppp.c:176! invalid opcode: 0000 [#1] SMP KASAN Dumping ftrace buffer: (ftrace buffer empty) Modules linked in: CPU: 1 PID: 3503 Comm: syzkaller938388 Not tainted 4.15.0-rc7+ #181 Hardware name: Google Google Compute Engine/Google Compute Engine RIP: 0010:pppol2tp_sock_to_session net/l2tp/l2tp_ppp.c:176 [inline] RIP: 0010:pppol2tp_sendmsg+0x512/0x670 net/l2tp/l2tp_ppp.c:304 RSP: 0018:ffff8801d4887438 EFLAGS: 00010293 RAX: ffff8801bfef2180 RBX: ffff8801bff88440 RCX: ffffffff84ffbca2 RDX: 0000000000000000 RSI: ffff8801d4887598 RDI: ffff8801bff88820 RBP: ffff8801d48874a8 R08: 0000000000000000 R09: 1ffff1003a910e17 R10: 0000000000000003 R11: 0000000000000001 R12: ffff8801bfff9bc0 R13: 0000000000000000 R14: 0000000000008000 R15: 0000000000000000 FS: 0000000001194880(0000) GS:ffff8801db300000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000020ea0000 CR3: 00000001bfecf001 CR4: 00000000001606e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: sock_sendmsg_nosec net/socket.c:628 [inline] sock_sendmsg+0xca/0x110 net/socket.c:638 kernel_sendmsg+0x47/0x60 net/socket.c:646 sock_no_sendpage+0x1cc/0x280 net/core/sock.c:2581 kernel_sendpage+0xbf/0xe0 net/socket.c:3349 kcm_write_msgs+0x404/0x1b80 net/kcm/kcmsock.c:646 kcm_sendmsg+0x148d/0x22d0 net/kcm/kcmsock.c:1035 sock_sendmsg_nosec net/socket.c:628 [inline] sock_sendmsg+0xca/0x110 net/socket.c:638 ___sys_sendmsg+0x767/0x8b0 net/socket.c:2018 __sys_sendmsg+0xe5/0x210 net/socket.c:2052 SYSC_sendmsg net/socket.c:2063 [inline] SyS_sendmsg+0x2d/0x50 net/socket.c:2059 entry_SYSCALL_64_fastpath+0x23/0x9a RIP: 0033:0x440159 RSP: 002b:00007ffe74df8288 EFLAGS: 00000217 ORIG_RAX: 000000000000002e RAX: ffffffffffffffda RBX: ffffffffffffffff RCX: 0000000000440159 RDX: 0000000000000000 RSI: 00000000201fcfc8 RDI: 0000000000000005 RBP: 00000000006ca018 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000217 R12: 0000000000401ac0 R13: 0000000000401b50 R14: 0000000000000000 R15: 0000000000000000 Code: c5 61 70 fc 48 8b 7d d0 e8 7c c2 5b fd 84 c0 74 0d e8 b3 61 70 fc 48 89 df e8 3b 49 2f ff 41 bd f7 ff ff ff eb 86 e8 9e 61 70 fc <0f> 0b 41 bd 95 ff ff ff e9 74 ff ff ff e8 ec 32 a8 fc e9 77 fb RIP: pppol2tp_sock_to_session net/l2tp/l2tp_ppp.c:176 [inline] RSP: ffff8801d4887438 RIP: pppol2tp_sendmsg+0x512/0x670 net/l2tp/l2tp_ppp.c:304 RSP: ffff8801d4887438 Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 21 ++++++++++++++++++--- net/l2tp/l2tp_ppp.c | 8 ++++++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 194a7483bb93..de7dce64173f 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1216,6 +1216,7 @@ static void l2tp_tunnel_destruct(struct sock *sk) /* Disable udp encapsulation */ + write_lock_bh(&sk->sk_callback_lock); switch (tunnel->encap) { case L2TP_ENCAPTYPE_UDP: /* No longer an encapsulation socket. See net/ipv4/udp.c */ @@ -1229,7 +1230,8 @@ static void l2tp_tunnel_destruct(struct sock *sk) /* Remove hooks into tunnel socket */ sk->sk_destruct = tunnel->old_sk_destruct; - sk->sk_user_data = NULL; + rcu_assign_sk_user_data(sk, NULL); + write_unlock_bh(&sk->sk_callback_lock); /* Remove the tunnel struct from the tunnel list */ pn = l2tp_pernet(tunnel->l2tp_net); @@ -1583,6 +1585,20 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 } #endif + /* Assign socket sk_user_data. Must be done with + * sk_callback_lock. Bail if sk_user_data is already assigned. + */ + write_lock_bh(&sk->sk_callback_lock); + if (sk->sk_user_data) { + err = -EALREADY; + write_unlock_bh(&sk->sk_callback_lock); + kfree(tunnel); + tunnel = NULL; + goto err; + } + rcu_assign_sk_user_data(sk, tunnel); + write_unlock_bh(&sk->sk_callback_lock); + /* Mark socket as an encapsulation socket. See net/ipv4/udp.c */ tunnel->encap = encap; if (encap == L2TP_ENCAPTYPE_UDP) { @@ -1594,8 +1610,6 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 udp_cfg.encap_destroy = l2tp_udp_encap_destroy; setup_udp_tunnel_sock(net, sock, &udp_cfg); - } else { - sk->sk_user_data = tunnel; } /* Hook on the tunnel socket destructor so that we can cleanup @@ -1603,6 +1617,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 */ tunnel->old_sk_destruct = sk->sk_destruct; sk->sk_destruct = &l2tp_tunnel_destruct; + tunnel->sock = sk; tunnel->fd = fd; lockdep_set_class_and_name(&sk->sk_lock.slock, &l2tp_socket_class, "l2tp_sock"); diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 59f246d7b290..fe5a0043dd32 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -443,7 +443,9 @@ static void pppol2tp_session_destruct(struct sock *sk) skb_queue_purge(&sk->sk_write_queue); if (session) { - sk->sk_user_data = NULL; + write_lock_bh(&sk->sk_callback_lock); + rcu_assign_sk_user_data(sk, NULL); + write_unlock_bh(&sk->sk_callback_lock); BUG_ON(session->magic != L2TP_SESSION_MAGIC); l2tp_session_dec_refcount(session); } @@ -796,7 +798,9 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, out_no_ppp: /* This is how we get the session context from the socket. */ - sk->sk_user_data = session; + write_lock_bh(&sk->sk_callback_lock); + rcu_assign_sk_user_data(sk, session); + write_unlock_bh(&sk->sk_callback_lock); rcu_assign_pointer(ps->sk, sk); mutex_unlock(&ps->sk_lock); From patchwork Fri Feb 9 15:00:18 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871437 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJFW29F4z9s7f for ; Sat, 10 Feb 2018 02:02:23 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752524AbeBIPCW (ORCPT ); Fri, 9 Feb 2018 10:02:22 -0500 Received: from mail.katalix.com ([82.103.140.233]:38413 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751299AbeBIPAg (ORCPT ); Fri, 9 Feb 2018 10:00:36 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id D0E227E2DF for ; Fri, 9 Feb 2018 15:00:34 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 02/16] l2tp: add RCU read lock to protect tunnel ptr in ip socket destroy Date: Fri, 9 Feb 2018 15:00:18 +0000 Message-Id: <1518188432-9245-3-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org If an L2TPIP socket is closed, add RCU protection when we deref sk_user_data to prevent races with another thread closing the same tunnel. Fixes: 0d76751fad ("l2tp: Add L2TPv3 IP encapsulation (no UDP) support") refcount_t: increment on 0; use-after-free. WARNING: CPU: 2 PID: 2892 at lib/refcount.c:153 refcount_inc+0x2b/0x30 Modules linked in: CPU: 2 PID: 2892 Comm: pppol2tp_chaos Not tainted 4.15.0-rc9+ #1 Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 RIP: 0010:refcount_inc+0x2b/0x30 RSP: 0018:ffff880014147b50 EFLAGS: 00010282 RAX: 000000000000002b RBX: ffff8800194785c0 RCX: 0000000000000000 RDX: 0000000000000001 RSI: ffff88001ad1f758 RDI: ffff88001ad1f758 RBP: ffff880014147b50 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000001 R11: 0000000000000001 R12: ffff8800194785c8 R13: ffff8800194785c0 R14: ffff88001a2c6c20 R15: ffff880013a9d580 FS: 0000000000000000(0000) GS:ffff88001ad00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fbc17ea7000 CR3: 0000000003022000 CR4: 00000000000006e0 Call Trace: l2tp_tunnel_delete+0x34/0x60 l2tp_ip_destroy_sock+0x6d/0x80 sk_common_release+0x19/0xd0 l2tp_ip_close+0x89/0xa0 inet_release+0x37/0x60 sock_release+0x1a/0x70 sock_close+0xd/0x20 __fput+0xed/0x1f0 ____fput+0x9/0x10 task_work_run+0x77/0xb0 do_exit+0x311/0xcf0 do_group_exit+0x47/0xc0 get_signal+0x343/0x8e0 do_signal+0x23/0x780 ? find_held_lock+0x39/0xb0 exit_to_usermode_loop+0x4a/0x93 syscall_return_slowpath+0x102/0x150 entry_SYSCALL_64_fastpath+0x98/0x9a RIP: 0033:0x7fbc172d7fcf RSP: 002b:00007ffd524cd4d8 EFLAGS: 00000246 ORIG_RAX: 000000000000000e RAX: 0000000000000000 RBX: 0000000000000006 RCX: 00007fbc172d7fcf RDX: 0000000000000000 RSI: 00007ffd524cd460 RDI: 0000000000000002 RBP: 0000000000404930 R08: 0000000000000000 R09: 00007ffd524cd460 R10: 0000000000000008 R11: 0000000000000246 R12: 000000000000001e R13: 0000000000404a03 R14: 0000000000000000 R15: 0000000000000000 Code: 55 48 89 e5 e8 87 ff ff ff 84 c0 74 02 5d c3 80 3d 97 87 bb 01 00 75 f5 48 c7 c7 58 3e cc 82 c6 05 87 87 bb 01 Signed-off-by: James Chapman --- net/l2tp/l2tp_ip.c | 5 ++++- net/l2tp/l2tp_ip6.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c index ff61124fdf59..42f3c2f72bf4 100644 --- a/net/l2tp/l2tp_ip.c +++ b/net/l2tp/l2tp_ip.c @@ -234,15 +234,18 @@ static void l2tp_ip_close(struct sock *sk, long timeout) static void l2tp_ip_destroy_sock(struct sock *sk) { struct sk_buff *skb; - struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk); + struct l2tp_tunnel *tunnel; while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL) kfree_skb(skb); + rcu_read_lock(); + tunnel = rcu_dereference_sk_user_data(sk); if (tunnel) { l2tp_tunnel_closeall(tunnel); sock_put(sk); } + rcu_read_unlock(); sk_refcnt_debug_dec(sk); } diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c index 192344688c06..be4a3eee85a9 100644 --- a/net/l2tp/l2tp_ip6.c +++ b/net/l2tp/l2tp_ip6.c @@ -248,16 +248,19 @@ static void l2tp_ip6_close(struct sock *sk, long timeout) static void l2tp_ip6_destroy_sock(struct sock *sk) { - struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk); + struct l2tp_tunnel *tunnel; lock_sock(sk); ip6_flush_pending_frames(sk); release_sock(sk); + rcu_read_lock(); + tunnel = rcu_dereference_sk_user_data(sk); if (tunnel) { l2tp_tunnel_closeall(tunnel); sock_put(sk); } + rcu_read_unlock(); inet6_destroy_sock(sk); } From patchwork Fri Feb 9 15:00:19 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871424 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJCn4N40z9s1h for ; Sat, 10 Feb 2018 02:00:53 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752197AbeBIPAn (ORCPT ); Fri, 9 Feb 2018 10:00:43 -0500 Received: from mail.katalix.com ([82.103.140.233]:38414 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751313AbeBIPAh (ORCPT ); Fri, 9 Feb 2018 10:00:37 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id 29B917E2EC for ; Fri, 9 Feb 2018 15:00:35 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 03/16] l2tp: don't use inet_shutdown on tunnel destroy Date: Fri, 9 Feb 2018 15:00:19 +0000 Message-Id: <1518188432-9245-4-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Previously, if a tunnel was closed, we called inet_shutdown to mark the socket as unconnected such that userspace would get errors and then close the socket. This could race with userspace closing the socket. Instead, leave userspace to close the socket in its own time (our tunnel will be detached anyway). Fixes: 309795f4be ("l2tp: Add netlink control API for L2TP") BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0 IP: __lock_acquire+0x263/0x1630 PGD 0 P4D 0 Oops: 0000 [#1] SMP KASAN Modules linked in: CPU: 2 PID: 42 Comm: kworker/u8:2 Not tainted 4.15.0-rc7+ #129 Workqueue: l2tp l2tp_tunnel_del_work RIP: 0010:__lock_acquire+0x263/0x1630 RSP: 0018:ffff88001a37fc70 EFLAGS: 00010002 RAX: 0000000000000001 RBX: 0000000000000088 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: ffff88001a37fd18 R08: 0000000000000001 R09: 0000000000000000 R10: 0000000000000000 R11: 00000000000076fd R12: 00000000000000a0 R13: ffff88001a3722c0 R14: 0000000000000001 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff88001ad00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00000000000000a0 CR3: 000000001730b000 CR4: 00000000000006e0 Call Trace: ? __lock_acquire+0xc77/0x1630 ? console_trylock+0x11/0xa0 lock_acquire+0x117/0x230 ? lock_sock_nested+0x3a/0xa0 _raw_spin_lock_bh+0x3a/0x50 ? lock_sock_nested+0x3a/0xa0 lock_sock_nested+0x3a/0xa0 inet_shutdown+0x33/0xf0 l2tp_tunnel_del_work+0x60/0xef process_one_work+0x1ea/0x5f0 ? process_one_work+0x162/0x5f0 worker_thread+0x48/0x3e0 ? trace_hardirqs_on+0xd/0x10 kthread+0x108/0x140 ? process_one_work+0x5f0/0x5f0 ? kthread_stop+0x2a0/0x2a0 ret_from_fork+0x24/0x30 Code: 00 41 81 ff ff 1f 00 00 0f 87 7a 13 00 00 45 85 f6 49 8b 85 68 08 00 00 0f 84 ae 03 00 00 c7 44 24 18 00 00 00 00 e9 f0 00 00 00 <49> 81 3c 24 80 93 3f 83 b8 00 00 00 00 44 0f 44 c0 83 fe 01 0f RIP: __lock_acquire+0x263/0x1630 RSP: ffff88001a37fc70 CR2: 00000000000000a0 Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index de7dce64173f..b68ae77e021e 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1329,17 +1329,10 @@ static void l2tp_tunnel_del_work(struct work_struct *work) sock = sk->sk_socket; - /* If the tunnel socket was created by userspace, then go through the - * inet layer to shut the socket down, and let userspace close it. - * Otherwise, if we created the socket directly within the kernel, use + /* If the tunnel socket was created within the kernel, use * the sk API to release it here. - * In either case the tunnel resources are freed in the socket - * destructor when the tunnel socket goes away. */ - if (tunnel->fd >= 0) { - if (sock) - inet_shutdown(sock, 2); - } else { + if (tunnel->fd < 0) { if (sock) { kernel_sock_shutdown(sock, SHUT_RDWR); sock_release(sock); From patchwork Fri Feb 9 15:00:20 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871425 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJCt6qtWz9s1h for ; Sat, 10 Feb 2018 02:00:58 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752190AbeBIPAm (ORCPT ); Fri, 9 Feb 2018 10:00:42 -0500 Received: from mail.katalix.com ([82.103.140.233]:38415 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751317AbeBIPAh (ORCPT ); Fri, 9 Feb 2018 10:00:37 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id 6E8E87E2F6 for ; Fri, 9 Feb 2018 15:00:35 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 04/16] l2tp: refactor tunnel lifetime handling wrt its socket Date: Fri, 9 Feb 2018 15:00:20 +0000 Message-Id: <1518188432-9245-5-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Ensure that the tunnel's socket is always extant while the tunnel object exists. Hold a ref on the socket until the tunnel is destroyed and ensure that all tunnel destroy paths go through a common function (l2tp_tunnel_delete). Since the tunnel's socket is now guaranteed to exist if the tunnel exists, we no longer need to use sockfd_lookup via l2tp_sock_to_tunnel to derive the tunnel from the socket since this is always sk_user_data. The tunnel object gains a new closing flag which is protected by a spinlock. The existing dead flag which is accessed using test_and_set_bit APIs is no longer used so is removed. Fixes: 80d84ef3ff1dd ("l2tp: prevent l2tp_tunnel_delete racing with userspace close") Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 128 ++++++++++++++++++--------------------------------- net/l2tp/l2tp_core.h | 26 ++--------- net/l2tp/l2tp_ip.c | 5 +- net/l2tp/l2tp_ip6.c | 3 +- 4 files changed, 52 insertions(+), 110 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index b68ae77e021e..49d6e06099ec 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -136,51 +136,6 @@ static inline struct l2tp_net *l2tp_pernet(const struct net *net) } -/* Lookup the tunnel socket, possibly involving the fs code if the socket is - * owned by userspace. A struct sock returned from this function must be - * released using l2tp_tunnel_sock_put once you're done with it. - */ -static struct sock *l2tp_tunnel_sock_lookup(struct l2tp_tunnel *tunnel) -{ - int err = 0; - struct socket *sock = NULL; - struct sock *sk = NULL; - - if (!tunnel) - goto out; - - if (tunnel->fd >= 0) { - /* Socket is owned by userspace, who might be in the process - * of closing it. Look the socket up using the fd to ensure - * consistency. - */ - sock = sockfd_lookup(tunnel->fd, &err); - if (sock) - sk = sock->sk; - } else { - /* Socket is owned by kernelspace */ - sk = tunnel->sock; - sock_hold(sk); - } - -out: - return sk; -} - -/* Drop a reference to a tunnel socket obtained via. l2tp_tunnel_sock_put */ -static void l2tp_tunnel_sock_put(struct sock *sk) -{ - struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk); - if (tunnel) { - if (tunnel->fd >= 0) { - /* Socket is owned by userspace */ - sockfd_put(sk->sk_socket); - } - sock_put(sk); - } - sock_put(sk); -} - /* Session hash list. * The session_id SHOULD be random according to RFC2661, but several * L2TP implementations (Cisco and Microsoft) use incrementing @@ -193,6 +148,12 @@ static void l2tp_tunnel_sock_put(struct sock *sk) return &tunnel->session_hlist[hash_32(session_id, L2TP_HASH_BITS)]; } +void l2tp_tunnel_free(struct l2tp_tunnel *tunnel) +{ + sock_put(tunnel->sock); + /* the tunnel is freed in the socket destructor */ +} + /* Lookup a tunnel. A new reference is held on the returned tunnel. */ struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id) { @@ -969,7 +930,7 @@ int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb) { struct l2tp_tunnel *tunnel; - tunnel = l2tp_sock_to_tunnel(sk); + tunnel = l2tp_tunnel(sk); if (tunnel == NULL) goto pass_up; @@ -977,13 +938,10 @@ int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb) tunnel->name, skb->len); if (l2tp_udp_recv_core(tunnel, skb, tunnel->recv_payload_hook)) - goto pass_up_put; + goto pass_up; - sock_put(sk); return 0; -pass_up_put: - sock_put(sk); pass_up: return 1; } @@ -1214,7 +1172,6 @@ static void l2tp_tunnel_destruct(struct sock *sk) l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: closing...\n", tunnel->name); - /* Disable udp encapsulation */ write_lock_bh(&sk->sk_callback_lock); switch (tunnel->encap) { @@ -1239,12 +1196,11 @@ static void l2tp_tunnel_destruct(struct sock *sk) list_del_rcu(&tunnel->list); spin_unlock_bh(&pn->l2tp_tunnel_list_lock); - tunnel->sock = NULL; - l2tp_tunnel_dec_refcount(tunnel); - /* Call the original destructor */ if (sk->sk_destruct) (*sk->sk_destruct)(sk); + + kfree_rcu(tunnel, rcu); end: return; } @@ -1305,30 +1261,26 @@ void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel) /* Tunnel socket destroy hook for UDP encapsulation */ static void l2tp_udp_encap_destroy(struct sock *sk) { - struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk); + struct l2tp_tunnel *tunnel; + + rcu_read_lock(); + tunnel = rcu_dereference_sk_user_data(sk); if (tunnel) { - l2tp_tunnel_closeall(tunnel); - sock_put(sk); + l2tp_tunnel_delete(tunnel); } + rcu_read_unlock(); } /* Workqueue tunnel deletion function */ static void l2tp_tunnel_del_work(struct work_struct *work) { - struct l2tp_tunnel *tunnel = NULL; - struct socket *sock = NULL; - struct sock *sk = NULL; - - tunnel = container_of(work, struct l2tp_tunnel, del_work); + struct l2tp_tunnel *tunnel = container_of(work, struct l2tp_tunnel, + del_work); + struct sock *sk = tunnel->sock; + struct socket *sock = sk->sk_socket; l2tp_tunnel_closeall(tunnel); - sk = l2tp_tunnel_sock_lookup(tunnel); - if (!sk) - goto out; - - sock = sk->sk_socket; - /* If the tunnel socket was created within the kernel, use * the sk API to release it here. */ @@ -1339,8 +1291,10 @@ static void l2tp_tunnel_del_work(struct work_struct *work) } } - l2tp_tunnel_sock_put(sk); -out: + /* drop initial ref */ + l2tp_tunnel_dec_refcount(tunnel); + + /* drop workqueue ref */ l2tp_tunnel_dec_refcount(tunnel); } @@ -1550,6 +1504,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 tunnel->magic = L2TP_TUNNEL_MAGIC; sprintf(&tunnel->name[0], "tunl %u", tunnel_id); + spin_lock_init(&tunnel->lock); rwlock_init(&tunnel->hlist_lock); tunnel->acpt_newsess = true; @@ -1605,14 +1560,23 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 setup_udp_tunnel_sock(net, sock, &udp_cfg); } + /* Bump the reference count. The tunnel context is deleted + * only when this drops to zero. A reference is also held on + * the tunnel socket to ensure that it is not released while + * the tunnel is extant. Must be done before sk_destruct is + * set. + */ + refcount_set(&tunnel->ref_count, 1); + sock_hold(sk); + tunnel->sock = sk; + tunnel->fd = fd; + /* Hook on the tunnel socket destructor so that we can cleanup * if the tunnel socket goes away. */ tunnel->old_sk_destruct = sk->sk_destruct; sk->sk_destruct = &l2tp_tunnel_destruct; - tunnel->sock = sk; - tunnel->fd = fd; lockdep_set_class_and_name(&sk->sk_lock.slock, &l2tp_socket_class, "l2tp_sock"); sk->sk_allocation = GFP_ATOMIC; @@ -1622,11 +1586,6 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 /* Add tunnel to our list */ INIT_LIST_HEAD(&tunnel->list); - - /* Bump the reference count. The tunnel context is deleted - * only when this drops to zero. Must be done before list insertion - */ - refcount_set(&tunnel->ref_count, 1); spin_lock_bh(&pn->l2tp_tunnel_list_lock); list_add_rcu(&tunnel->list, &pn->l2tp_tunnel_list); spin_unlock_bh(&pn->l2tp_tunnel_list_lock); @@ -1650,10 +1609,17 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 */ void l2tp_tunnel_delete(struct l2tp_tunnel *tunnel) { - if (!test_and_set_bit(0, &tunnel->dead)) { - l2tp_tunnel_inc_refcount(tunnel); - queue_work(l2tp_wq, &tunnel->del_work); + spin_lock_bh(&tunnel->lock); + if (tunnel->closing) { + spin_unlock_bh(&tunnel->lock); + return; } + tunnel->closing = true; + spin_unlock_bh(&tunnel->lock); + + /* Hold tunnel ref while queued work item is pending */ + l2tp_tunnel_inc_refcount(tunnel); + queue_work(l2tp_wq, &tunnel->del_work); } EXPORT_SYMBOL_GPL(l2tp_tunnel_delete); @@ -1667,8 +1633,6 @@ void l2tp_session_free(struct l2tp_session *session) if (tunnel) { BUG_ON(tunnel->magic != L2TP_TUNNEL_MAGIC); - sock_put(tunnel->sock); - session->tunnel = NULL; l2tp_tunnel_dec_refcount(tunnel); } diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 9bbee90e9963..e88ff7895ccb 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -155,7 +155,8 @@ struct l2tp_tunnel_cfg { struct l2tp_tunnel { int magic; /* Should be L2TP_TUNNEL_MAGIC */ - unsigned long dead; + bool closing; + spinlock_t lock; /* protect closing */ struct rcu_head rcu; rwlock_t hlist_lock; /* protect session_hlist */ @@ -214,27 +215,8 @@ static inline void *l2tp_session_priv(struct l2tp_session *session) return &session->priv[0]; } -static inline struct l2tp_tunnel *l2tp_sock_to_tunnel(struct sock *sk) -{ - struct l2tp_tunnel *tunnel; - - if (sk == NULL) - return NULL; - - sock_hold(sk); - tunnel = (struct l2tp_tunnel *)(sk->sk_user_data); - if (tunnel == NULL) { - sock_put(sk); - goto out; - } - - BUG_ON(tunnel->magic != L2TP_TUNNEL_MAGIC); - -out: - return tunnel; -} - struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id); +void l2tp_tunnel_free(struct l2tp_tunnel *tunnel); struct l2tp_session *l2tp_session_get(const struct net *net, struct l2tp_tunnel *tunnel, @@ -283,7 +265,7 @@ static inline void l2tp_tunnel_inc_refcount(struct l2tp_tunnel *tunnel) static inline void l2tp_tunnel_dec_refcount(struct l2tp_tunnel *tunnel) { if (refcount_dec_and_test(&tunnel->ref_count)) - kfree_rcu(tunnel, rcu); + l2tp_tunnel_free(tunnel); } /* Session reference counts. Incremented when code obtains a reference diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c index 42f3c2f72bf4..a5591bd2fa24 100644 --- a/net/l2tp/l2tp_ip.c +++ b/net/l2tp/l2tp_ip.c @@ -242,12 +242,9 @@ static void l2tp_ip_destroy_sock(struct sock *sk) rcu_read_lock(); tunnel = rcu_dereference_sk_user_data(sk); if (tunnel) { - l2tp_tunnel_closeall(tunnel); - sock_put(sk); + l2tp_tunnel_delete(tunnel); } rcu_read_unlock(); - - sk_refcnt_debug_dec(sk); } static int l2tp_ip_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len) diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c index be4a3eee85a9..de8e7eb7a638 100644 --- a/net/l2tp/l2tp_ip6.c +++ b/net/l2tp/l2tp_ip6.c @@ -257,8 +257,7 @@ static void l2tp_ip6_destroy_sock(struct sock *sk) rcu_read_lock(); tunnel = rcu_dereference_sk_user_data(sk); if (tunnel) { - l2tp_tunnel_closeall(tunnel); - sock_put(sk); + l2tp_tunnel_delete(tunnel); } rcu_read_unlock(); From patchwork Fri Feb 9 15:00:21 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871421 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJCc4Sr1z9s7f for ; Sat, 10 Feb 2018 02:00:44 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752178AbeBIPAm (ORCPT ); Fri, 9 Feb 2018 10:00:42 -0500 Received: from mail.katalix.com ([82.103.140.233]:38416 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751786AbeBIPAi (ORCPT ); Fri, 9 Feb 2018 10:00:38 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id B3A8B7E33A for ; Fri, 9 Feb 2018 15:00:35 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 05/16] l2tp: use tunnel closing flag Date: Fri, 9 Feb 2018 15:00:21 +0000 Message-Id: <1518188432-9245-6-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The tunnel's closing flag is set when the tunnel is being destroyed. Use it to reject new sessions and remove acpt_newsess which was doing the same thing. Also prevent the tunnel being seen in l2tp_tunnel_get lookups. Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 27 +++++++++++++++++++++------ net/l2tp/l2tp_core.h | 4 ---- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 49d6e06099ec..691fe9368d91 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -163,6 +163,13 @@ struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id) rcu_read_lock_bh(); list_for_each_entry_rcu(tunnel, &pn->l2tp_tunnel_list, list) { if (tunnel->tunnel_id == tunnel_id) { + spin_lock_bh(&tunnel->lock); + if (tunnel->closing) { + spin_unlock_bh(&tunnel->lock); + rcu_read_unlock_bh(); + return NULL; + } + spin_unlock_bh(&tunnel->lock); l2tp_tunnel_inc_refcount(tunnel); rcu_read_unlock_bh(); @@ -278,13 +285,16 @@ int l2tp_session_register(struct l2tp_session *session, struct l2tp_net *pn; int err; + spin_lock_bh(&tunnel->lock); + if (tunnel->closing) { + spin_unlock_bh(&tunnel->lock); + return -ENODEV; + } + spin_unlock_bh(&tunnel->lock); + head = l2tp_session_id_hash(tunnel, session->session_id); write_lock_bh(&tunnel->hlist_lock); - if (!tunnel->acpt_newsess) { - err = -ENODEV; - goto err_tlock; - } hlist_for_each_entry(session_walk, head, hlist) if (session_walk->session_id == session->session_id) { @@ -1220,7 +1230,6 @@ void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel) tunnel->name); write_lock_bh(&tunnel->hlist_lock); - tunnel->acpt_newsess = false; for (hash = 0; hash < L2TP_HASH_SIZE; hash++) { again: hlist_for_each_safe(walk, tmp, &tunnel->session_hlist[hash]) { @@ -1506,7 +1515,6 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 sprintf(&tunnel->name[0], "tunl %u", tunnel_id); spin_lock_init(&tunnel->lock); rwlock_init(&tunnel->hlist_lock); - tunnel->acpt_newsess = true; /* The net we belong to */ tunnel->l2tp_net = net; @@ -1710,6 +1718,13 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn { struct l2tp_session *session; + spin_lock_bh(&tunnel->lock); + if (tunnel->closing) { + spin_unlock_bh(&tunnel->lock); + return ERR_PTR(-ENODEV); + } + spin_unlock_bh(&tunnel->lock); + session = kzalloc(sizeof(struct l2tp_session) + priv_size, GFP_KERNEL); if (session != NULL) { session->magic = L2TP_SESSION_MAGIC; diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index e88ff7895ccb..4e098c822cd1 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -160,10 +160,6 @@ struct l2tp_tunnel { struct rcu_head rcu; rwlock_t hlist_lock; /* protect session_hlist */ - bool acpt_newsess; /* Indicates whether this - * tunnel accepts new sessions. - * Protected by hlist_lock. - */ struct hlist_head session_hlist[L2TP_HASH_SIZE]; /* hashed list of sessions, * hashed by id */ From patchwork Fri Feb 9 15:00:22 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871422 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJCd6wNxz9s7f for ; Sat, 10 Feb 2018 02:00:45 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752210AbeBIPAn (ORCPT ); Fri, 9 Feb 2018 10:00:43 -0500 Received: from mail.katalix.com ([82.103.140.233]:38417 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751756AbeBIPAi (ORCPT ); Fri, 9 Feb 2018 10:00:38 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id 0DE857E33C for ; Fri, 9 Feb 2018 15:00:36 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 06/16] l2tp: refactor session lifetime handling Date: Fri, 9 Feb 2018 15:00:22 +0000 Message-Id: <1518188432-9245-7-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Simplify relationship with tunnel such that the session holds a ref on the tunnel, not its socket. This guarantees that the tunnel is always extant if one or more sessions exists on the tunnel. If the session has a socket (ppp), have it hold a ref on the socket until the session is destroyed. Since pppol2tp_sock_to_session returns a session and the session now holds a sock ref, have it return with a ref on the session. Fixes: fd558d186df2c ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts") Fixes: f3c66d4e144a0 ("l2tp: prevent creation of sessions on terminated tunnels") Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 7 ++----- net/l2tp/l2tp_ppp.c | 36 ++++++++++++++++++------------------ 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 691fe9368d91..477b96cf8ab3 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -290,6 +290,7 @@ int l2tp_session_register(struct l2tp_session *session, spin_unlock_bh(&tunnel->lock); return -ENODEV; } + l2tp_tunnel_inc_refcount(tunnel); spin_unlock_bh(&tunnel->lock); head = l2tp_session_id_hash(tunnel, session->session_id); @@ -315,14 +316,9 @@ int l2tp_session_register(struct l2tp_session *session, goto err_tlock_pnlock; } - l2tp_tunnel_inc_refcount(tunnel); - sock_hold(tunnel->sock); hlist_add_head_rcu(&session->global_hlist, g_head); spin_unlock_bh(&pn->l2tp_session_hlist_lock); - } else { - l2tp_tunnel_inc_refcount(tunnel); - sock_hold(tunnel->sock); } hlist_add_head(&session->hlist, head); @@ -334,6 +330,7 @@ int l2tp_session_register(struct l2tp_session *session, spin_unlock_bh(&pn->l2tp_session_hlist_lock); err_tlock: write_unlock_bh(&tunnel->hlist_lock); + l2tp_tunnel_dec_refcount(tunnel); return err; } diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index fe5a0043dd32..ff95a4d4eac5 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -166,16 +166,17 @@ static inline struct l2tp_session *pppol2tp_sock_to_session(struct sock *sk) if (sk == NULL) return NULL; - sock_hold(sk); - session = (struct l2tp_session *)(sk->sk_user_data); + rcu_read_lock_bh(); + session = rcu_dereference_bh(__sk_user_data((sk))); if (session == NULL) { - sock_put(sk); - goto out; + rcu_read_unlock_bh(); + return NULL; } + l2tp_session_inc_refcount(session); + rcu_read_unlock(); BUG_ON(session->magic != L2TP_SESSION_MAGIC); -out: return session; } @@ -243,8 +244,8 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int /* If the socket is bound, send it in to PPP's input queue. Otherwise * queue it on the session socket. */ - rcu_read_lock(); - sk = rcu_dereference(ps->sk); + rcu_read_lock_bh(); + sk = rcu_dereference_bh(ps->sk); if (sk == NULL) goto no_sock; @@ -267,12 +268,12 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int kfree_skb(skb); } } - rcu_read_unlock(); + rcu_read_unlock_bh(); return; no_sock: - rcu_read_unlock(); + rcu_read_unlock_bh(); l2tp_info(session, L2TP_MSG_DATA, "%s: no socket\n", session->name); kfree_skb(skb); } @@ -341,12 +342,12 @@ static int pppol2tp_sendmsg(struct socket *sock, struct msghdr *m, l2tp_xmit_skb(session, skb, session->hdr_len); local_bh_enable(); - sock_put(sk); + l2tp_session_dec_refcount(session); return total_len; error_put_sess: - sock_put(sk); + l2tp_session_dec_refcount(session); error: return error; } @@ -400,12 +401,12 @@ static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb) l2tp_xmit_skb(session, skb, session->hdr_len); local_bh_enable(); - sock_put(sk); + l2tp_session_dec_refcount(session); return 1; abort_put_sess: - sock_put(sk); + l2tp_session_dec_refcount(session); abort: /* Free the original skb */ kfree_skb(skb); @@ -483,7 +484,6 @@ static int pppol2tp_release(struct socket *sock) sock->sk = NULL; session = pppol2tp_sock_to_session(sk); - if (session != NULL) { struct pppol2tp_session *ps; @@ -976,7 +976,7 @@ static int pppol2tp_getname(struct socket *sock, struct sockaddr *uaddr, *usockaddr_len = len; error = 0; - sock_put(sk); + l2tp_session_dec_refcount(session); end: return error; } @@ -1247,7 +1247,7 @@ static int pppol2tp_ioctl(struct socket *sock, unsigned int cmd, err = pppol2tp_session_ioctl(session, cmd, arg); end_put_sess: - sock_put(sk); + l2tp_session_dec_refcount(session); end: return err; } @@ -1398,7 +1398,7 @@ static int pppol2tp_setsockopt(struct socket *sock, int level, int optname, err = pppol2tp_session_setsockopt(sk, session, optname, val); } - sock_put(sk); + l2tp_session_dec_refcount(session); end: return err; } @@ -1530,7 +1530,7 @@ static int pppol2tp_getsockopt(struct socket *sock, int level, int optname, err = 0; end_put_sess: - sock_put(sk); + l2tp_session_dec_refcount(session); end: return err; } From patchwork Fri Feb 9 15:00:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871431 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJDs1N6mz9s1h for ; Sat, 10 Feb 2018 02:01:49 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752522AbeBIPBq (ORCPT ); Fri, 9 Feb 2018 10:01:46 -0500 Received: from mail.katalix.com ([82.103.140.233]:38418 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751859AbeBIPAj (ORCPT ); Fri, 9 Feb 2018 10:00:39 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id 54F957E33E for ; Fri, 9 Feb 2018 15:00:36 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 07/16] l2tp: hide sessions if they are closing Date: Fri, 9 Feb 2018 15:00:23 +0000 Message-Id: <1518188432-9245-8-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Replace the dead flag in the session context with a closing flag and spinlock. Check it in session lookup functions such that we don't try to access session data while it is being destroyed. Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 34 +++++++++++++++++++++++++++++++++- net/l2tp/l2tp_core.h | 3 ++- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 477b96cf8ab3..869dec89ff0f 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -198,7 +198,14 @@ struct l2tp_session *l2tp_session_get(const struct net *net, rcu_read_lock_bh(); hlist_for_each_entry_rcu(session, session_list, global_hlist) { if (session->session_id == session_id) { + spin_lock_bh(&session->lock); + if (session->closing) { + spin_unlock_bh(&session->lock); + rcu_read_unlock_bh(); + return NULL; + } l2tp_session_inc_refcount(session); + spin_unlock_bh(&session->lock); rcu_read_unlock_bh(); return session; @@ -213,7 +220,14 @@ struct l2tp_session *l2tp_session_get(const struct net *net, read_lock_bh(&tunnel->hlist_lock); hlist_for_each_entry(session, session_list, hlist) { if (session->session_id == session_id) { + spin_lock_bh(&session->lock); + if (session->closing) { + spin_unlock_bh(&session->lock); + read_unlock_bh(&tunnel->hlist_lock); + return NULL; + } l2tp_session_inc_refcount(session); + spin_unlock_bh(&session->lock); read_unlock_bh(&tunnel->hlist_lock); return session; @@ -234,6 +248,12 @@ struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth) read_lock_bh(&tunnel->hlist_lock); for (hash = 0; hash < L2TP_HASH_SIZE; hash++) { hlist_for_each_entry(session, &tunnel->session_hlist[hash], hlist) { + spin_lock_bh(&session->lock); + if (session->closing) { + spin_unlock_bh(&session->lock); + continue; + } + spin_unlock_bh(&session->lock); if (++count > nth) { l2tp_session_inc_refcount(session); read_unlock_bh(&tunnel->hlist_lock); @@ -261,6 +281,12 @@ struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net, rcu_read_lock_bh(); for (hash = 0; hash < L2TP_HASH_SIZE_2; hash++) { hlist_for_each_entry_rcu(session, &pn->l2tp_session_hlist[hash], global_hlist) { + spin_lock_bh(&session->lock); + if (session->closing) { + spin_unlock_bh(&session->lock); + continue; + } + spin_unlock_bh(&session->lock); if (!strcmp(session->ifname, ifname)) { l2tp_session_inc_refcount(session); rcu_read_unlock_bh(); @@ -1678,8 +1704,13 @@ void __l2tp_session_unhash(struct l2tp_session *session) */ int l2tp_session_delete(struct l2tp_session *session) { - if (test_and_set_bit(0, &session->dead)) + spin_lock_bh(&session->lock); + if (session->closing) { + spin_unlock_bh(&session->lock); return 0; + } + session->closing = true; + spin_unlock_bh(&session->lock); __l2tp_session_unhash(session); l2tp_session_queue_purge(session); @@ -1747,6 +1778,7 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn INIT_HLIST_NODE(&session->hlist); INIT_HLIST_NODE(&session->global_hlist); + spin_lock_init(&session->lock); /* Inherit debug options from tunnel */ session->debug = tunnel->debug; diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 4e098c822cd1..98709086fe84 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -74,7 +74,8 @@ struct l2tp_session_cfg { struct l2tp_session { int magic; /* should be * L2TP_SESSION_MAGIC */ - long dead; + bool closing; + spinlock_t lock; /* protect closing */ struct l2tp_tunnel *tunnel; /* back pointer to tunnel * context */ From patchwork Fri Feb 9 15:00:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871423 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJCg2Sl0z9s7f for ; Sat, 10 Feb 2018 02:00:47 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752230AbeBIPAo (ORCPT ); Fri, 9 Feb 2018 10:00:44 -0500 Received: from mail.katalix.com ([82.103.140.233]:38419 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751280AbeBIPAj (ORCPT ); Fri, 9 Feb 2018 10:00:39 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id BD8EF7E349 for ; Fri, 9 Feb 2018 15:00:36 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 08/16] l2tp: hide session from pppol2tp_sock_to_session if it is closing Date: Fri, 9 Feb 2018 15:00:24 +0000 Message-Id: <1518188432-9245-9-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Signed-off-by: James Chapman --- net/l2tp/l2tp_ppp.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index ff95a4d4eac5..947066b3d6d8 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -172,8 +172,16 @@ static inline struct l2tp_session *pppol2tp_sock_to_session(struct sock *sk) rcu_read_unlock_bh(); return NULL; } + + spin_lock_bh(&session->lock); + if (session->closing) { + spin_unlock_bh(&session->lock); + rcu_read_unlock_bh(); + return NULL; + } l2tp_session_inc_refcount(session); - rcu_read_unlock(); + spin_unlock_bh(&session->lock); + rcu_read_unlock_bh(); BUG_ON(session->magic != L2TP_SESSION_MAGIC); From patchwork Fri Feb 9 15:00:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871433 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJFL0GKfz9s1h for ; Sat, 10 Feb 2018 02:02:14 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752509AbeBIPBp (ORCPT ); Fri, 9 Feb 2018 10:01:45 -0500 Received: from mail.katalix.com ([82.103.140.233]:38420 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751941AbeBIPAj (ORCPT ); Fri, 9 Feb 2018 10:00:39 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id 193A67E0CD for ; Fri, 9 Feb 2018 15:00:37 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 09/16] l2tp: refactor pppol2tp_connect Date: Fri, 9 Feb 2018 15:00:25 +0000 Message-Id: <1518188432-9245-10-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org It's hard to understand pppol2tp_connect so split it up into separate functions and document it better. Fixes: fd558d186d ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts") Signed-off-by: James Chapman --- net/l2tp/l2tp_ppp.c | 318 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 196 insertions(+), 122 deletions(-) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 947066b3d6d8..90bdeb16a8c7 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -425,6 +425,28 @@ static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb) * Session (and tunnel control) socket create/destroy. *****************************************************************************/ +/* called with ps->sk_lock */ +static void pppol2tp_attach(struct l2tp_session *session, struct sock *sk) +{ + struct pppol2tp_session *ps = l2tp_session_priv(session); + + write_lock_bh(&sk->sk_callback_lock); + rcu_assign_sk_user_data(sk, session); + write_unlock_bh(&sk->sk_callback_lock); + rcu_assign_pointer(ps->sk, sk); +} + +/* called with ps->sk_lock */ +static void pppol2tp_detach(struct l2tp_session *session, struct sock *sk) +{ + struct pppol2tp_session *ps = l2tp_session_priv(session); + + rcu_assign_pointer(ps->sk, NULL); + write_lock_bh(&sk->sk_callback_lock); + rcu_assign_sk_user_data(sk, NULL); + write_unlock_bh(&sk->sk_callback_lock); +} + /* Called by l2tp_core when a session socket is being closed. */ static void pppol2tp_session_close(struct l2tp_session *session) @@ -462,10 +484,18 @@ static void pppol2tp_session_destruct(struct sock *sk) static void pppol2tp_put_sk(struct rcu_head *head) { - struct pppol2tp_session *ps; + struct pppol2tp_session *ps = container_of(head, typeof(*ps), rcu); + struct l2tp_session *session = container_of((void *)ps, + typeof(*session), priv); + struct sock *sk = ps->__sk; - ps = container_of(head, typeof(*ps), rcu); - sock_put(ps->__sk); + /* If session is invalid, something serious is wrong. Warn and + * leak the socket. + */ + WARN_ON(session->magic != L2TP_SESSION_MAGIC); + if (session->magic != L2TP_SESSION_MAGIC) + return; + sock_put(sk); } /* Called when the PPPoX socket (session) is closed. @@ -615,25 +645,147 @@ static void pppol2tp_session_init(struct l2tp_session *session) } } -/* connect() handler. Attach a PPPoX socket to a tunnel UDP socket +/* Prepare a tunnel. If a tunnel instance doesn't already exist, + * optionally create it. Return with a ref on the tunnel instance. + */ +static int pppol2tp_tunnel_prep(struct net *net, int fd, int ver, + u32 tunnel_id, u32 peer_tunnel_id, + bool can_create, struct l2tp_tunnel **tunnelp) +{ + struct l2tp_tunnel *tunnel; + int error; + + tunnel = l2tp_tunnel_get(net, tunnel_id); + if (!tunnel && can_create) { + struct l2tp_tunnel_cfg tcfg = { + .encap = L2TP_ENCAPTYPE_UDP, + .debug = 0, + }; + error = l2tp_tunnel_create(net, fd, ver, tunnel_id, + peer_tunnel_id, &tcfg, &tunnel); + if (error < 0) + return error; + + l2tp_tunnel_inc_refcount(tunnel); + } + + /* Error if we can't find the tunnel */ + if (!tunnel) + return -ENOENT; + + if (!tunnel->recv_payload_hook) + tunnel->recv_payload_hook = pppol2tp_recv_payload_hook; + + if (tunnel->peer_tunnel_id == 0) + tunnel->peer_tunnel_id = peer_tunnel_id; + + *tunnelp = tunnel; + return 0; + + l2tp_tunnel_dec_refcount(tunnel); + return error; +} + +/* Prepare a session in a tunnel. If the session doesn't already + * exist, create it and add it to the tunnel's session list. Return + * with a ref on the session instance and its sk_lock held. + */ +static int pppol2tp_session_prep(struct sock *sk, struct l2tp_tunnel *tunnel, + u32 session_id, u32 peer_session_id, + struct l2tp_session **sessionp) +{ + struct l2tp_session *session; + struct pppol2tp_session *ps; + int error; + struct l2tp_session_cfg cfg = {}; + + session = l2tp_session_get(sock_net(sk), tunnel, session_id); + if (session) { + ps = l2tp_session_priv(session); + + /* Using a pre-existing session is fine as long as it hasn't + * been connected yet. + */ + mutex_lock(&ps->sk_lock); + if (rcu_dereference_protected(ps->sk, + lockdep_is_held(&ps->sk_lock))) { + mutex_unlock(&ps->sk_lock); + l2tp_session_dec_refcount(session); + return -EEXIST; + } + } else { + /* Default MTU must allow space for UDP/L2TP/PPP headers */ + cfg.mtu = 1500 - PPPOL2TP_HEADER_OVERHEAD; + cfg.mru = cfg.mtu; + + session = l2tp_session_create(sizeof(struct pppol2tp_session), + tunnel, session_id, + peer_session_id, &cfg); + if (IS_ERR(session)) { + error = PTR_ERR(session); + return error; + } + + pppol2tp_session_init(session); + ps = l2tp_session_priv(session); + + mutex_lock(&ps->sk_lock); + error = l2tp_session_register(session, tunnel); + if (error < 0) { + mutex_unlock(&ps->sk_lock); + kfree(session); + return error; + } + l2tp_session_inc_refcount(session); + } + + *sessionp = session; + return 0; +} + +static int pppol2tp_setup_ppp(struct l2tp_session *session, struct sock *sk) +{ + struct pppox_sock *po = pppox_sk(sk); + + /* The only header we need to worry about is the L2TP + * header. This size is different depending on whether + * sequence numbers are enabled for the data channel. + */ + po->chan.hdrlen = PPPOL2TP_L2TP_HDR_SIZE_NOSEQ; + + po->chan.private = sk; + po->chan.ops = &pppol2tp_chan_ops; + po->chan.mtu = session->mtu; + + return ppp_register_net_channel(sock_net(sk), &po->chan); +} + +/* connect() handler. Attach a PPPoX socket to a tunnel socket. + * The PPPoX socket is associated with an l2tp_session and the tunnel + * socket is associated with an l2tp_tunnel. The l2tp_tunnel and + * l2tp_session are usually created by netlink before the PPPoX socket + * is connected. However, for L2TPv2 we support a legacy mode where + * netlink is not used and we create the l2tp_tunnel and l2tp_session + * when the PPPoX sockets are connected. In legacy mode, a per-tunnel + * PPPoX socket is used as a control socket for the tunnel and is + * identified by session_id 0. An l2tp_session is created to manage + * the control socket and an l2tp_tunnel is created for the tunnel if + * it doesn't already exist. */ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, int sockaddr_len, int flags) { struct sock *sk = sock->sk; struct sockaddr_pppol2tp *sp = (struct sockaddr_pppol2tp *) uservaddr; - struct pppox_sock *po = pppox_sk(sk); struct l2tp_session *session = NULL; - struct l2tp_tunnel *tunnel; + struct l2tp_tunnel *tunnel = NULL; struct pppol2tp_session *ps; - struct l2tp_session_cfg cfg = { 0, }; int error = 0; u32 tunnel_id, peer_tunnel_id; u32 session_id, peer_session_id; - bool drop_refcnt = false; - bool drop_tunnel = false; int ver = 2; int fd; + bool is_ctrl_skt; lock_sock(sk); @@ -695,139 +847,56 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr, goto end; /* bad socket address */ } - /* Don't bind if tunnel_id is 0 */ error = -EINVAL; - if (tunnel_id == 0) + if (tunnel_id == 0 || peer_tunnel_id == 0) goto end; - tunnel = l2tp_tunnel_get(sock_net(sk), tunnel_id); - if (tunnel) - drop_tunnel = true; - - /* Special case: create tunnel context if session_id and - * peer_session_id is 0. Otherwise look up tunnel using supplied - * tunnel id. + /* The socket is a control socket if session_id is 0. There is + * one control socket per tunnel. Control sockets do not have ppp. */ - if ((session_id == 0) && (peer_session_id == 0)) { - if (tunnel == NULL) { - struct l2tp_tunnel_cfg tcfg = { - .encap = L2TP_ENCAPTYPE_UDP, - .debug = 0, - }; - error = l2tp_tunnel_create(sock_net(sk), fd, ver, tunnel_id, peer_tunnel_id, &tcfg, &tunnel); - if (error < 0) - goto end; - } - } else { - /* Error if we can't find the tunnel */ - error = -ENOENT; - if (tunnel == NULL) - goto end; - - /* Error if socket is not prepped */ - if (tunnel->sock == NULL) - goto end; - } - - if (tunnel->recv_payload_hook == NULL) - tunnel->recv_payload_hook = pppol2tp_recv_payload_hook; - - if (tunnel->peer_tunnel_id == 0) - tunnel->peer_tunnel_id = peer_tunnel_id; - - session = l2tp_session_get(sock_net(sk), tunnel, session_id); - if (session) { - drop_refcnt = true; - ps = l2tp_session_priv(session); + is_ctrl_skt = (session_id == 0 && peer_session_id == 0); - /* Using a pre-existing session is fine as long as it hasn't - * been connected yet. - */ - mutex_lock(&ps->sk_lock); - if (rcu_dereference_protected(ps->sk, - lockdep_is_held(&ps->sk_lock))) { - mutex_unlock(&ps->sk_lock); - error = -EEXIST; - goto end; - } - } else { - /* Default MTU must allow space for UDP/L2TP/PPP headers */ - cfg.mtu = 1500 - PPPOL2TP_HEADER_OVERHEAD; - cfg.mru = cfg.mtu; - - session = l2tp_session_create(sizeof(struct pppol2tp_session), - tunnel, session_id, - peer_session_id, &cfg); - if (IS_ERR(session)) { - error = PTR_ERR(session); - goto end; - } + /* prep and possibly create the l2tp tunnel instance */ + error = pppol2tp_tunnel_prep(sock_net(sk), fd, ver, tunnel_id, + peer_tunnel_id, is_ctrl_skt, &tunnel); + if (error) + goto end; - pppol2tp_session_init(session); - ps = l2tp_session_priv(session); - l2tp_session_inc_refcount(session); + /* prep and possibly create the l2tp session instance */ + error = pppol2tp_session_prep(sk, tunnel, session_id, + peer_session_id, &session); + if (error) + goto end; - mutex_lock(&ps->sk_lock); - error = l2tp_session_register(session, tunnel); - if (error < 0) { + /* setup ppp unless it's a control socket */ + ps = l2tp_session_priv(session); + if (!is_ctrl_skt) { + error = pppol2tp_setup_ppp(session, sk); + if (error) { mutex_unlock(&ps->sk_lock); - kfree(session); goto end; } - drop_refcnt = true; } - /* Special case: if source & dest session_id == 0x0000, this - * socket is being created to manage the tunnel. Just set up - * the internal context for use by ioctl() and sockopt() - * handlers. + /* The session has now been added to the tunnel. Hold the + * socket to prevent it going away until the session is + * destroyed and attach it to the session such that we can get + * the session instance from the socket and vice versa. */ - if ((session->session_id == 0) && - (session->peer_session_id == 0)) { - error = 0; - goto out_no_ppp; - } - - /* The only header we need to worry about is the L2TP - * header. This size is different depending on whether - * sequence numbers are enabled for the data channel. - */ - po->chan.hdrlen = PPPOL2TP_L2TP_HDR_SIZE_NOSEQ; - - po->chan.private = sk; - po->chan.ops = &pppol2tp_chan_ops; - po->chan.mtu = session->mtu; - - error = ppp_register_net_channel(sock_net(sk), &po->chan); - if (error) { - mutex_unlock(&ps->sk_lock); - goto end; - } - -out_no_ppp: - /* This is how we get the session context from the socket. */ - write_lock_bh(&sk->sk_callback_lock); - rcu_assign_sk_user_data(sk, session); - write_unlock_bh(&sk->sk_callback_lock); - rcu_assign_pointer(ps->sk, sk); + sock_hold(sk); + pppol2tp_attach(session, sk); mutex_unlock(&ps->sk_lock); - /* Keep the reference we've grabbed on the session: sk doesn't expect - * the session to disappear. pppol2tp_session_destruct() is responsible - * for dropping it. - */ - drop_refcnt = false; - sk->sk_state = PPPOX_CONNECTED; l2tp_info(session, L2TP_MSG_CONTROL, "%s: created\n", session->name); end: - if (drop_refcnt) + release_sock(sk); + if (session) l2tp_session_dec_refcount(session); - if (drop_tunnel) + if (tunnel) l2tp_tunnel_dec_refcount(tunnel); - release_sock(sk); return error; } @@ -841,6 +910,7 @@ static int pppol2tp_session_create(struct net *net, struct l2tp_tunnel *tunnel, { int error; struct l2tp_session *session; + struct pppol2tp_session *ps; /* Error if tunnel socket is not prepped */ if (!tunnel->sock) { @@ -864,10 +934,14 @@ static int pppol2tp_session_create(struct net *net, struct l2tp_tunnel *tunnel, } pppol2tp_session_init(session); - + ps = l2tp_session_priv(session); + mutex_lock(&ps->sk_lock); error = l2tp_session_register(session, tunnel); - if (error < 0) + if (error < 0) { + mutex_unlock(&ps->sk_lock); goto err_sess; + } + mutex_unlock(&ps->sk_lock); return 0; From patchwork Fri Feb 9 15:00:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871432 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJDx6rmtz9s1h for ; Sat, 10 Feb 2018 02:01:53 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752519AbeBIPBq (ORCPT ); Fri, 9 Feb 2018 10:01:46 -0500 Received: from mail.katalix.com ([82.103.140.233]:38421 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751998AbeBIPAj (ORCPT ); Fri, 9 Feb 2018 10:00:39 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id 624F97E363 for ; Fri, 9 Feb 2018 15:00:37 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 10/16] l2tp: add session_free callback Date: Fri, 9 Feb 2018 15:00:26 +0000 Message-Id: <1518188432-9245-11-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org When a session refcount hits 0, the session is freed via l2tp_session_free. Some pseudowires (ppp, eth) may have additional resources to free when this happens. Add a session_free callback that can be used by pseudowires to override the default kfree. The callback is responsible for freeing the session. Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 7 +++++-- net/l2tp/l2tp_core.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 869dec89ff0f..d6306ba2d78e 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1662,12 +1662,15 @@ void l2tp_session_free(struct l2tp_session *session) BUG_ON(refcount_read(&session->ref_count) != 0); + if (session->session_free) + session->session_free(session); + else + kfree(session); + if (tunnel) { BUG_ON(tunnel->magic != L2TP_TUNNEL_MAGIC); l2tp_tunnel_dec_refcount(tunnel); } - - kfree(session); } EXPORT_SYMBOL_GPL(l2tp_session_free); diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 98709086fe84..be3fa986dfd1 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -125,6 +125,7 @@ struct l2tp_session { int (*build_header)(struct l2tp_session *session, void *buf); void (*recv_skb)(struct l2tp_session *session, struct sk_buff *skb, int data_len); void (*session_close)(struct l2tp_session *session); + void (*session_free)(struct l2tp_session *session); #if IS_ENABLED(CONFIG_L2TP_DEBUGFS) void (*show)(struct seq_file *m, void *priv); #endif From patchwork Fri Feb 9 15:00:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871428 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJDS1946z9s1h for ; Sat, 10 Feb 2018 02:01:28 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752317AbeBIPA7 (ORCPT ); Fri, 9 Feb 2018 10:00:59 -0500 Received: from mail.katalix.com ([82.103.140.233]:38422 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752054AbeBIPAm (ORCPT ); Fri, 9 Feb 2018 10:00:42 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id A988F7E2CF for ; Fri, 9 Feb 2018 15:00:37 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 11/16] l2tp: do session destroy using a workqueue Date: Fri, 9 Feb 2018 15:00:27 +0000 Message-Id: <1518188432-9245-12-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Handle session destroy in the same way as we handle tunnel destroy - through a workqueue. Sessions can be destroyed either because its socket is closed (if it has a socket) or by netlink request. A workqueue synchronises these. Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 30 +++++++++++++++++++++++------- net/l2tp/l2tp_core.h | 2 ++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index d6306ba2d78e..55b1f312fedc 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1702,6 +1702,24 @@ void __l2tp_session_unhash(struct l2tp_session *session) } EXPORT_SYMBOL_GPL(__l2tp_session_unhash); +/* Workqueue session deletion function */ +static void l2tp_session_del_work(struct work_struct *work) +{ + struct l2tp_session *session = container_of(work, struct l2tp_session, + del_work); + + __l2tp_session_unhash(session); + l2tp_session_queue_purge(session); + if (session->session_close) + (*session->session_close)(session); + + /* drop initial ref */ + l2tp_session_dec_refcount(session); + + /* drop workqueue ref */ + l2tp_session_dec_refcount(session); +} + /* This function is used by the netlink SESSION_DELETE command and by pseudowire modules. */ @@ -1715,13 +1733,9 @@ int l2tp_session_delete(struct l2tp_session *session) session->closing = true; spin_unlock_bh(&session->lock); - __l2tp_session_unhash(session); - l2tp_session_queue_purge(session); - if (session->session_close != NULL) - (*session->session_close)(session); - - l2tp_session_dec_refcount(session); - + /* Hold session ref while queued work item is pending */ + l2tp_session_inc_refcount(session); + queue_work(l2tp_wq, &session->del_work); return 0; } EXPORT_SYMBOL_GPL(l2tp_session_delete); @@ -1783,6 +1797,8 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn INIT_HLIST_NODE(&session->global_hlist); spin_lock_init(&session->lock); + INIT_WORK(&session->del_work, l2tp_session_del_work); + /* Inherit debug options from tunnel */ session->debug = tunnel->debug; diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index be3fa986dfd1..73c4ce79c708 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -122,6 +122,8 @@ struct l2tp_session { struct l2tp_stats stats; struct hlist_node global_hlist; /* Global hash list node */ + struct work_struct del_work; + int (*build_header)(struct l2tp_session *session, void *buf); void (*recv_skb)(struct l2tp_session *session, struct sk_buff *skb, int data_len); void (*session_close)(struct l2tp_session *session); From patchwork Fri Feb 9 15:00:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871427 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJCz3tqRz9s7f for ; Sat, 10 Feb 2018 02:01:03 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752359AbeBIPBB (ORCPT ); Fri, 9 Feb 2018 10:01:01 -0500 Received: from mail.katalix.com ([82.103.140.233]:38423 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752046AbeBIPAm (ORCPT ); Fri, 9 Feb 2018 10:00:42 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id F09147E36A for ; Fri, 9 Feb 2018 15:00:37 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 12/16] l2tp: simplify l2tp_tunnel_closeall Date: Fri, 9 Feb 2018 15:00:28 +0000 Message-Id: <1518188432-9245-13-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Since session destroy now uses a workqueue, let l2tp_session_delete handle all the work of destroying a session. Don't remove the session from the tunnel's list immediately. The tunnel will remain extant until all of its sessions are gone anyway. Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 55b1f312fedc..c909fe9273c9 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1254,36 +1254,9 @@ void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel) write_lock_bh(&tunnel->hlist_lock); for (hash = 0; hash < L2TP_HASH_SIZE; hash++) { -again: hlist_for_each_safe(walk, tmp, &tunnel->session_hlist[hash]) { session = hlist_entry(walk, struct l2tp_session, hlist); - - l2tp_info(session, L2TP_MSG_CONTROL, - "%s: closing session\n", session->name); - - hlist_del_init(&session->hlist); - - if (test_and_set_bit(0, &session->dead)) - goto again; - - write_unlock_bh(&tunnel->hlist_lock); - - __l2tp_session_unhash(session); - l2tp_session_queue_purge(session); - - if (session->session_close != NULL) - (*session->session_close)(session); - - l2tp_session_dec_refcount(session); - - write_lock_bh(&tunnel->hlist_lock); - - /* Now restart from the beginning of this hash - * chain. We always remove a session from the - * list so we are guaranteed to make forward - * progress. - */ - goto again; + l2tp_session_delete(session); } } write_unlock_bh(&tunnel->hlist_lock); @@ -1708,6 +1681,9 @@ static void l2tp_session_del_work(struct work_struct *work) struct l2tp_session *session = container_of(work, struct l2tp_session, del_work); + l2tp_info(session, L2TP_MSG_CONTROL, + "%s: closing session\n", session->name); + __l2tp_session_unhash(session); l2tp_session_queue_purge(session); if (session->session_close) From patchwork Fri Feb 9 15:00:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871430 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJDb0vDMz9s1h for ; Sat, 10 Feb 2018 02:01:35 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752285AbeBIPA5 (ORCPT ); Fri, 9 Feb 2018 10:00:57 -0500 Received: from mail.katalix.com ([82.103.140.233]:38424 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752110AbeBIPAm (ORCPT ); Fri, 9 Feb 2018 10:00:42 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id 408AC7E2DF for ; Fri, 9 Feb 2018 15:00:38 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 13/16] l2tp: refactor ppp session cleanup paths Date: Fri, 9 Feb 2018 15:00:29 +0000 Message-Id: <1518188432-9245-14-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Use l2tp core's session_free callback to drive the ppp session cleanup. PPP sessions are cleaned up by RCU. The PPP session socket is allowed to close only when the session is freed. With this patch, the following syzbot bug reports are finally fixed. Reported-by: syzbot+9df43faf09bd400f2993@syzkaller.appspotmail.com Reported-by: syzbot+6e6a5ec8de31a94cd015@syzkaller.appspotmail.com Reported-by: syzbot+19c09769f14b48810113@syzkaller.appspotmail.com Reported-by: syzbot+347bd5acde002e353a36@syzkaller.appspotmail.com Signed-off-by: James Chapman --- net/l2tp/l2tp_ppp.c | 98 ++++++++++++++++++++++++++--------------------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 90bdeb16a8c7..e50feb1479e6 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -447,18 +447,57 @@ static void pppol2tp_detach(struct l2tp_session *session, struct sock *sk) write_unlock_bh(&sk->sk_callback_lock); } -/* Called by l2tp_core when a session socket is being closed. +static void pppol2tp_session_free_rcu(struct rcu_head *head) +{ + struct pppol2tp_session *ps = container_of(head, typeof(*ps), rcu); + struct l2tp_session *session = container_of((void *)ps, + typeof(*session), priv); + + /* If session is invalid, something serious is wrong. Warn and + * leak the socket and session. + */ + WARN_ON(session->magic != L2TP_SESSION_MAGIC); + if (session->magic != L2TP_SESSION_MAGIC) + return; + + if (ps->__sk) + sock_put(ps->__sk); + kfree(session); +} + +/* Called by l2tp_core when a session is being freed. + */ +static void pppol2tp_session_free(struct l2tp_session *session) +{ + struct pppol2tp_session *ps = l2tp_session_priv(session); + + /* If session is invalid, something serious is wrong. Warn and + * leak the socket and session. + */ + WARN_ON(session->magic != L2TP_SESSION_MAGIC); + if (session->magic != L2TP_SESSION_MAGIC) + return; + + call_rcu(&ps->rcu, pppol2tp_session_free_rcu); +} + +/* Called by l2tp_core when a session is being closed. */ static void pppol2tp_session_close(struct l2tp_session *session) { struct sock *sk; BUG_ON(session->magic != L2TP_SESSION_MAGIC); - sk = pppol2tp_session_get_sock(session); if (sk) { - if (sk->sk_socket) - inet_shutdown(sk->sk_socket, SEND_SHUTDOWN); + struct pppol2tp_session *ps = l2tp_session_priv(session); + + mutex_lock(&ps->sk_lock); + ps->__sk = rcu_dereference_protected(ps->sk, + lockdep_is_held(&ps->sk_lock)); + RCU_INIT_POINTER(ps->sk, NULL); + pppol2tp_detach(session, sk); + mutex_unlock(&ps->sk_lock); sock_put(sk); } } @@ -468,34 +507,8 @@ static void pppol2tp_session_close(struct l2tp_session *session) */ static void pppol2tp_session_destruct(struct sock *sk) { - struct l2tp_session *session = sk->sk_user_data; - skb_queue_purge(&sk->sk_receive_queue); skb_queue_purge(&sk->sk_write_queue); - - if (session) { - write_lock_bh(&sk->sk_callback_lock); - rcu_assign_sk_user_data(sk, NULL); - write_unlock_bh(&sk->sk_callback_lock); - BUG_ON(session->magic != L2TP_SESSION_MAGIC); - l2tp_session_dec_refcount(session); - } -} - -static void pppol2tp_put_sk(struct rcu_head *head) -{ - struct pppol2tp_session *ps = container_of(head, typeof(*ps), rcu); - struct l2tp_session *session = container_of((void *)ps, - typeof(*session), priv); - struct sock *sk = ps->__sk; - - /* If session is invalid, something serious is wrong. Warn and - * leak the socket. - */ - WARN_ON(session->magic != L2TP_SESSION_MAGIC); - if (session->magic != L2TP_SESSION_MAGIC) - return; - sock_put(sk); } /* Called when the PPPoX socket (session) is closed. @@ -520,27 +533,13 @@ static int pppol2tp_release(struct socket *sock) sk->sk_state = PPPOX_DEAD; sock_orphan(sk); sock->sk = NULL; + release_sock(sk); - session = pppol2tp_sock_to_session(sk); - if (session != NULL) { - struct pppol2tp_session *ps; - + rcu_read_lock_bh(); + session = rcu_dereference_bh(__sk_user_data((sk))); + if (session) l2tp_session_delete(session); - - ps = l2tp_session_priv(session); - mutex_lock(&ps->sk_lock); - ps->__sk = rcu_dereference_protected(ps->sk, - lockdep_is_held(&ps->sk_lock)); - RCU_INIT_POINTER(ps->sk, NULL); - mutex_unlock(&ps->sk_lock); - call_rcu(&ps->rcu, pppol2tp_put_sk); - - /* Rely on the sock_put() call at the end of the function for - * dropping the reference held by pppol2tp_sock_to_session(). - * The last reference will be dropped by pppol2tp_put_sk(). - */ - } - release_sock(sk); + rcu_read_unlock_bh(); /* This will delete the session context via * pppol2tp_session_destruct() if the socket's refcnt drops to @@ -624,6 +623,7 @@ static void pppol2tp_session_init(struct l2tp_session *session) session->recv_skb = pppol2tp_recv; session->session_close = pppol2tp_session_close; + session->session_free = pppol2tp_session_free; #if IS_ENABLED(CONFIG_L2TP_DEBUGFS) session->show = pppol2tp_show; #endif From patchwork Fri Feb 9 15:00:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871426 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJCy6203z9s1h for ; Sat, 10 Feb 2018 02:01:02 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752331AbeBIPBA (ORCPT ); Fri, 9 Feb 2018 10:01:00 -0500 Received: from mail.katalix.com ([82.103.140.233]:38425 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752059AbeBIPAm (ORCPT ); Fri, 9 Feb 2018 10:00:42 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id 828787E373 for ; Fri, 9 Feb 2018 15:00:38 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 14/16] l2tp: remove redundant sk_user_data check when creating tunnels Date: Fri, 9 Feb 2018 15:00:30 +0000 Message-Id: <1518188432-9245-15-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org l2tp_tunnel_create now checks sk_user_data so this check is redundant Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index c909fe9273c9..a91cd384e397 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1488,14 +1488,6 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 break; } - /* Check if this socket has already been prepped */ - tunnel = l2tp_tunnel(sk); - if (tunnel != NULL) { - /* This socket has already been prepped */ - err = -EBUSY; - goto err; - } - tunnel = kzalloc(sizeof(struct l2tp_tunnel), GFP_KERNEL); if (tunnel == NULL) { err = -ENOMEM; From patchwork Fri Feb 9 15:00:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871435 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJFP5LgNz9s7f for ; Sat, 10 Feb 2018 02:02:17 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752498AbeBIPBo (ORCPT ); Fri, 9 Feb 2018 10:01:44 -0500 Received: from mail.katalix.com ([82.103.140.233]:38416 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752028AbeBIPAl (ORCPT ); Fri, 9 Feb 2018 10:00:41 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id CF7687E379 for ; Fri, 9 Feb 2018 15:00:38 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 15/16] l2tp: remove unwanted error message Date: Fri, 9 Feb 2018 15:00:31 +0000 Message-Id: <1518188432-9245-16-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org If when creating a new tunnel, the indicated fd is closed by another thread, we emit an error message about it. e.g. l2tp_core: tunl 4: sockfd_lookup(fd=3) returned -9 It's not useful so remove it. Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index a91cd384e397..28940cf9cc76 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1449,8 +1449,6 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 } else { sock = sockfd_lookup(fd, &err); if (!sock) { - pr_err("tunl %u: sockfd_lookup(fd=%d) returned %d\n", - tunnel_id, fd, err); err = -EBADF; goto err; } From patchwork Fri Feb 9 15:00:32 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Chapman X-Patchwork-Id: 871429 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zdJDV6yfXz9s1h for ; Sat, 10 Feb 2018 02:01:30 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752293AbeBIPA6 (ORCPT ); Fri, 9 Feb 2018 10:00:58 -0500 Received: from mail.katalix.com ([82.103.140.233]:38426 "EHLO mail.katalix.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752126AbeBIPAm (ORCPT ); Fri, 9 Feb 2018 10:00:42 -0500 Received: from katalix.com (82-69-108-24.dsl.in-addr.zen.co.uk [82.69.108.24]) (Authenticated sender: james) by mail.katalix.com (Postfix) with ESMTPSA id 269F17E380 for ; Fri, 9 Feb 2018 15:00:39 +0000 (GMT) From: James Chapman To: netdev@vger.kernel.org Subject: [PATCH net-next 16/16] l2tp: make __l2tp_session_unhash internal Date: Fri, 9 Feb 2018 15:00:32 +0000 Message-Id: <1518188432-9245-17-git-send-email-jchapman@katalix.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com> References: <1518188432-9245-1-git-send-email-jchapman@katalix.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org __l2tp_session_unhash is now only used internally so there is no reason to expose it to other l2tp modules. Rename it l2tp_session_unhash while we're at it. Signed-off-by: James Chapman --- net/l2tp/l2tp_core.c | 5 ++--- net/l2tp/l2tp_core.h | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 28940cf9cc76..34221c6763d8 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1642,7 +1642,7 @@ void l2tp_session_free(struct l2tp_session *session) * shutdown via. l2tp_session_delete and a pseudowire-specific session_close * callback. */ -void __l2tp_session_unhash(struct l2tp_session *session) +static void l2tp_session_unhash(struct l2tp_session *session) { struct l2tp_tunnel *tunnel = session->tunnel; @@ -1663,7 +1663,6 @@ void __l2tp_session_unhash(struct l2tp_session *session) } } } -EXPORT_SYMBOL_GPL(__l2tp_session_unhash); /* Workqueue session deletion function */ static void l2tp_session_del_work(struct work_struct *work) @@ -1674,7 +1673,7 @@ static void l2tp_session_del_work(struct work_struct *work) l2tp_info(session, L2TP_MSG_CONTROL, "%s: closing session\n", session->name); - __l2tp_session_unhash(session); + l2tp_session_unhash(session); l2tp_session_queue_purge(session); if (session->session_close) (*session->session_close)(session); diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 73c4ce79c708..faca18046518 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -239,7 +239,6 @@ struct l2tp_session *l2tp_session_create(int priv_size, int l2tp_session_register(struct l2tp_session *session, struct l2tp_tunnel *tunnel); -void __l2tp_session_unhash(struct l2tp_session *session); int l2tp_session_delete(struct l2tp_session *session); void l2tp_session_free(struct l2tp_session *session); void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,