From patchwork Tue Apr 10 19:01:13 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guillaume Nault X-Patchwork-Id: 896902 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@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=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=alphalink.fr Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 40LGjg11Grz9s0v for ; Wed, 11 Apr 2018 05:01:27 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752626AbeDJTBY (ORCPT ); Tue, 10 Apr 2018 15:01:24 -0400 Received: from zimbra.alphalink.fr ([217.15.80.77]:40397 "EHLO zimbra.alphalink.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752134AbeDJTBX (ORCPT ); Tue, 10 Apr 2018 15:01:23 -0400 Received: from localhost (localhost [127.0.0.1]) by mail-2-cbv2.admin.alphalink.fr (Postfix) with ESMTP id 9B2BF2B52097; Tue, 10 Apr 2018 21:01:22 +0200 (CEST) Received: from zimbra.alphalink.fr ([127.0.0.1]) by localhost (mail-2-cbv2.admin.alphalink.fr [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id UFvnvgIIGHTT; Tue, 10 Apr 2018 21:01:13 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by mail-2-cbv2.admin.alphalink.fr (Postfix) with ESMTP id D8B6A2B52113; Tue, 10 Apr 2018 21:01:13 +0200 (CEST) X-Virus-Scanned: amavisd-new at mail-2-cbv2.admin.alphalink.fr Received: from zimbra.alphalink.fr ([127.0.0.1]) by localhost (mail-2-cbv2.admin.alphalink.fr [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 4rv3-QEsnHU5; Tue, 10 Apr 2018 21:01:13 +0200 (CEST) Received: from c-dev-0.admin.alphalink.fr (94-84-15-217.reverse.alphalink.fr [217.15.84.94]) by mail-2-cbv2.admin.alphalink.fr (Postfix) with ESMTP id A89132B52097; Tue, 10 Apr 2018 21:01:13 +0200 (CEST) Received: by c-dev-0.admin.alphalink.fr (Postfix, from userid 1000) id 8C884601E2; Tue, 10 Apr 2018 21:01:13 +0200 (CEST) Date: Tue, 10 Apr 2018 21:01:13 +0200 From: Guillaume Nault To: netdev@vger.kernel.org Cc: James Chapman , Tom Parkin Subject: [PATCH net 2/2] l2tp: fix race in duplicate tunnel detection Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Mutt-Fcc: =Sent User-Agent: Mutt/1.9.4 (2018-02-28) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org We can't use l2tp_tunnel_find() to prevent l2tp_nl_cmd_tunnel_create() from creating a duplicate tunnel. A tunnel can be concurrently registered after l2tp_tunnel_find() returns. Therefore, searching for duplicates must be done at registration time. Finally, remove l2tp_tunnel_find() entirely as it isn't use anywhere anymore. Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP") Signed-off-by: Guillaume Nault --- net/l2tp/l2tp_core.c | 35 ++++++++++++++--------------------- net/l2tp/l2tp_core.h | 1 - net/l2tp/l2tp_netlink.c | 6 ------ 3 files changed, 14 insertions(+), 28 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index afb42d142807..0fbd3ee26165 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -335,26 +335,6 @@ int l2tp_session_register(struct l2tp_session *session, } EXPORT_SYMBOL_GPL(l2tp_session_register); -/* Lookup a tunnel by id - */ -struct l2tp_tunnel *l2tp_tunnel_find(const struct net *net, u32 tunnel_id) -{ - struct l2tp_tunnel *tunnel; - struct l2tp_net *pn = l2tp_pernet(net); - - rcu_read_lock_bh(); - list_for_each_entry_rcu(tunnel, &pn->l2tp_tunnel_list, list) { - if (tunnel->tunnel_id == tunnel_id) { - rcu_read_unlock_bh(); - return tunnel; - } - } - rcu_read_unlock_bh(); - - return NULL; -} -EXPORT_SYMBOL_GPL(l2tp_tunnel_find); - struct l2tp_tunnel *l2tp_tunnel_find_nth(const struct net *net, int nth) { struct l2tp_net *pn = l2tp_pernet(net); @@ -1501,6 +1481,7 @@ static int l2tp_validate_socket(const struct sock *sk, const struct net *net, int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net, struct l2tp_tunnel_cfg *cfg) { + struct l2tp_tunnel *tunnel_walk; struct l2tp_net *pn; struct socket *sock; struct sock *sk; @@ -1529,7 +1510,16 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net, tunnel->l2tp_net = net; pn = l2tp_pernet(net); + spin_lock_bh(&pn->l2tp_tunnel_list_lock); + list_for_each_entry(tunnel_walk, &pn->l2tp_tunnel_list, list) { + if (tunnel_walk->tunnel_id == tunnel->tunnel_id) { + spin_unlock_bh(&pn->l2tp_tunnel_list_lock); + + ret = -EEXIST; + goto err_sock; + } + } list_add_rcu(&tunnel->list, &pn->l2tp_tunnel_list); spin_unlock_bh(&pn->l2tp_tunnel_list_lock); @@ -1558,7 +1548,10 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net, return 0; err_sock: - sockfd_put(sock); + if (tunnel->fd < 0) + sock_release(sock); + else + sockfd_put(sock); err: return ret; } diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h index 12f0fa82f162..ba33cbec71eb 100644 --- a/net/l2tp/l2tp_core.h +++ b/net/l2tp/l2tp_core.h @@ -220,7 +220,6 @@ struct l2tp_session *l2tp_session_get(const struct net *net, struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth); struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net, const char *ifname); -struct l2tp_tunnel *l2tp_tunnel_find(const struct net *net, u32 tunnel_id); struct l2tp_tunnel *l2tp_tunnel_find_nth(const struct net *net, int nth); int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c index 45db9b73eb1a..b05dbd9ffcb2 100644 --- a/net/l2tp/l2tp_netlink.c +++ b/net/l2tp/l2tp_netlink.c @@ -236,12 +236,6 @@ static int l2tp_nl_cmd_tunnel_create(struct sk_buff *skb, struct genl_info *info if (info->attrs[L2TP_ATTR_DEBUG]) cfg.debug = nla_get_u32(info->attrs[L2TP_ATTR_DEBUG]); - tunnel = l2tp_tunnel_find(net, tunnel_id); - if (tunnel != NULL) { - ret = -EEXIST; - goto out; - } - ret = -EINVAL; switch (cfg.encap) { case L2TP_ENCAPTYPE_UDP: