Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/805928/?format=api
{ "id": 805928, "url": "http://patchwork.ozlabs.org/api/patches/805928/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/patch/11ba48ce2320c0e9fc70872c448e3e3b9276516d.1503671776.git.g.nault@alphalink.fr/", "project": { "id": 7, "url": "http://patchwork.ozlabs.org/api/projects/7/?format=api", "name": "Linux network development", "link_name": "netdev", "list_id": "netdev.vger.kernel.org", "list_email": "netdev@vger.kernel.org", "web_url": null, "scm_url": null, "webscm_url": null, "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<11ba48ce2320c0e9fc70872c448e3e3b9276516d.1503671776.git.g.nault@alphalink.fr>", "list_archive_url": null, "date": "2017-08-25T14:51:40", "name": "[net,1/5] l2tp: hold tunnel while looking up sessions in l2tp_netlink", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "952e035491e8735723cf58d06e37f4238cbf5ac3", "submitter": { "id": 22975, "url": "http://patchwork.ozlabs.org/api/people/22975/?format=api", "name": "Guillaume Nault", "email": "g.nault@alphalink.fr" }, "delegate": { "id": 34, "url": "http://patchwork.ozlabs.org/api/users/34/?format=api", "username": "davem", "first_name": "David", "last_name": "Miller", "email": "davem@davemloft.net" }, "mbox": "http://patchwork.ozlabs.org/project/netdev/patch/11ba48ce2320c0e9fc70872c448e3e3b9276516d.1503671776.git.g.nault@alphalink.fr/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/805928/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/805928/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<netdev-owner@vger.kernel.org>", "X-Original-To": "patchwork-incoming@ozlabs.org", "Delivered-To": "patchwork-incoming@ozlabs.org", "Authentication-Results": "ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)", "Received": [ "from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xf3yw5HHsz9sN5\n\tfor <patchwork-incoming@ozlabs.org>;\n\tSat, 26 Aug 2017 00:51:52 +1000 (AEST)", "(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1757288AbdHYOvu (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tFri, 25 Aug 2017 10:51:50 -0400", "from zimbra.alphalink.fr ([217.15.80.77]:53407 \"EHLO\n\tzimbra.alphalink.fr\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1755006AbdHYOvr (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Fri, 25 Aug 2017 10:51:47 -0400", "from localhost (localhost [127.0.0.1])\n\tby mail-2-cbv2.admin.alphalink.fr (Postfix) with ESMTP id\n\t00F1C2B5206F; Fri, 25 Aug 2017 16:51:45 +0200 (CEST)", "from zimbra.alphalink.fr ([127.0.0.1])\n\tby localhost (mail-2-cbv2.admin.alphalink.fr [127.0.0.1])\n\t(amavisd-new, port 10032)\n\twith ESMTP id LWRYC8gJ_BOq; Fri, 25 Aug 2017 16:51:41 +0200 (CEST)", "from localhost (localhost [127.0.0.1])\n\tby mail-2-cbv2.admin.alphalink.fr (Postfix) with ESMTP id\n\t340DD2B52097; Fri, 25 Aug 2017 16:51:41 +0200 (CEST)", "from zimbra.alphalink.fr ([127.0.0.1])\n\tby localhost (mail-2-cbv2.admin.alphalink.fr [127.0.0.1])\n\t(amavisd-new, port 10026)\n\twith ESMTP id AUBFXNXgVwDz; Fri, 25 Aug 2017 16:51:41 +0200 (CEST)", "from c-dev-0.admin.alphalink.fr (94-84-15-217.reverse.alphalink.fr\n\t[217.15.84.94])\n\tby mail-2-cbv2.admin.alphalink.fr (Postfix) with ESMTP id\n\t0434C2B5206F; Fri, 25 Aug 2017 16:51:41 +0200 (CEST)", "by c-dev-0.admin.alphalink.fr (Postfix, from userid 1000)\n\tid DC1D260176; Fri, 25 Aug 2017 16:51:40 +0200 (CEST)" ], "X-Virus-Scanned": "amavisd-new at mail-2-cbv2.admin.alphalink.fr", "Date": "Fri, 25 Aug 2017 16:51:40 +0200", "From": "Guillaume Nault <g.nault@alphalink.fr>", "To": "netdev@vger.kernel.org", "Cc": "James Chapman <jchapman@katalix.com>", "Subject": "[PATCH net 1/5] l2tp: hold tunnel while looking up sessions in\n\tl2tp_netlink", "Message-ID": "<11ba48ce2320c0e9fc70872c448e3e3b9276516d.1503671776.git.g.nault@alphalink.fr>", "References": "<cover.1503671776.git.g.nault@alphalink.fr>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=us-ascii", "Content-Disposition": "inline", "In-Reply-To": "<cover.1503671776.git.g.nault@alphalink.fr>", "X-Mutt-Fcc": "=Sent", "User-Agent": "NeoMutt/20170609 (1.8.3)", "Sender": "netdev-owner@vger.kernel.org", "Precedence": "bulk", "List-ID": "<netdev.vger.kernel.org>", "X-Mailing-List": "netdev@vger.kernel.org" }, "content": "l2tp_tunnel_find() doesn't take a reference on the returned tunnel.\nTherefore, it's unsafe to use it because the returned tunnel can go\naway on us anytime.\n\nFix this by defining l2tp_tunnel_get(), which works like\nl2tp_tunnel_find(), but takes a reference on the returned tunnel.\nCaller then has to drop this reference using l2tp_tunnel_dec_refcount().\n\nAs l2tp_tunnel_dec_refcount() needs to be moved to l2tp_core.h, let's\nsimplify the patch and not move the L2TP_REFCNT_DEBUG part. This code\nhas been broken (not even compiling) in May 2012 by\ncommit a4ca44fa578c (\"net: l2tp: Standardize logging styles\")\nand fixed more than two years later by\ncommit 29abe2fda54f (\"l2tp: fix missing line continuation\"). So it\ndoesn't appear to be used by anyone.\n\nSame thing for l2tp_tunnel_free(); instead of moving it to l2tp_core.h,\nlet's just simplify things and call kfree_rcu() directly in\nl2tp_tunnel_dec_refcount(). Extra assertions and debugging code\nprovided by l2tp_tunnel_free() didn't help catching any of the\nreference counting and socket handling issues found while working on\nthis series.\n\nFixes: 309795f4bec2 (\"l2tp: Add netlink control API for L2TP\")\nSigned-off-by: Guillaume Nault <g.nault@alphalink.fr>\n---\n net/l2tp/l2tp_core.c | 66 ++++++++++++++++---------------------------------\n net/l2tp/l2tp_core.h | 13 ++++++++++\n net/l2tp/l2tp_netlink.c | 6 +++--\n 3 files changed, 38 insertions(+), 47 deletions(-)", "diff": "diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c\nindex b0c2d4ae781d..1adebfb9bde2 100644\n--- a/net/l2tp/l2tp_core.c\n+++ b/net/l2tp/l2tp_core.c\n@@ -113,7 +113,6 @@ struct l2tp_net {\n \tspinlock_t l2tp_session_hlist_lock;\n };\n \n-static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel);\n \n static inline struct l2tp_tunnel *l2tp_tunnel(struct sock *sk)\n {\n@@ -127,39 +126,6 @@ static inline struct l2tp_net *l2tp_pernet(const struct net *net)\n \treturn net_generic(net, l2tp_net_id);\n }\n \n-/* Tunnel reference counts. Incremented per session that is added to\n- * the tunnel.\n- */\n-static inline void l2tp_tunnel_inc_refcount_1(struct l2tp_tunnel *tunnel)\n-{\n-\trefcount_inc(&tunnel->ref_count);\n-}\n-\n-static inline void l2tp_tunnel_dec_refcount_1(struct l2tp_tunnel *tunnel)\n-{\n-\tif (refcount_dec_and_test(&tunnel->ref_count))\n-\t\tl2tp_tunnel_free(tunnel);\n-}\n-#ifdef L2TP_REFCNT_DEBUG\n-#define l2tp_tunnel_inc_refcount(_t)\t\t\t\t\t\\\n-do {\t\t\t\t\t\t\t\t\t\\\n-\tpr_debug(\"l2tp_tunnel_inc_refcount: %s:%d %s: cnt=%d\\n\",\t\\\n-\t\t __func__, __LINE__, (_t)->name,\t\t\t\\\n-\t\t refcount_read(&_t->ref_count));\t\t\t\\\n-\tl2tp_tunnel_inc_refcount_1(_t);\t\t\t\t\t\\\n-} while (0)\n-#define l2tp_tunnel_dec_refcount(_t)\t\t\t\t\t\\\n-do {\t\t\t\t\t\t\t\t\t\\\n-\tpr_debug(\"l2tp_tunnel_dec_refcount: %s:%d %s: cnt=%d\\n\",\t\\\n-\t\t __func__, __LINE__, (_t)->name,\t\t\t\\\n-\t\t refcount_read(&_t->ref_count));\t\t\t\\\n-\tl2tp_tunnel_dec_refcount_1(_t);\t\t\t\t\t\\\n-} while (0)\n-#else\n-#define l2tp_tunnel_inc_refcount(t) l2tp_tunnel_inc_refcount_1(t)\n-#define l2tp_tunnel_dec_refcount(t) l2tp_tunnel_dec_refcount_1(t)\n-#endif\n-\n /* Session hash global list for L2TPv3.\n * The session_id SHOULD be random according to RFC3931, but several\n * L2TP implementations use incrementing session_ids. So we do a real\n@@ -229,6 +195,27 @@ l2tp_session_id_hash(struct l2tp_tunnel *tunnel, u32 session_id)\n \treturn &tunnel->session_hlist[hash_32(session_id, L2TP_HASH_BITS)];\n }\n \n+/* Lookup a tunnel. A new reference is held on the returned tunnel. */\n+struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id)\n+{\n+\tconst struct l2tp_net *pn = l2tp_pernet(net);\n+\tstruct l2tp_tunnel *tunnel;\n+\n+\trcu_read_lock_bh();\n+\tlist_for_each_entry_rcu(tunnel, &pn->l2tp_tunnel_list, list) {\n+\t\tif (tunnel->tunnel_id == tunnel_id) {\n+\t\t\tl2tp_tunnel_inc_refcount(tunnel);\n+\t\t\trcu_read_unlock_bh();\n+\n+\t\t\treturn tunnel;\n+\t\t}\n+\t}\n+\trcu_read_unlock_bh();\n+\n+\treturn NULL;\n+}\n+EXPORT_SYMBOL_GPL(l2tp_tunnel_get);\n+\n /* Lookup a session. A new reference is held on the returned session.\n * Optionally calls session->ref() too if do_ref is true.\n */\n@@ -1348,17 +1335,6 @@ static void l2tp_udp_encap_destroy(struct sock *sk)\n \t}\n }\n \n-/* Really kill the tunnel.\n- * Come here only when all sessions have been cleared from the tunnel.\n- */\n-static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel)\n-{\n-\tBUG_ON(refcount_read(&tunnel->ref_count) != 0);\n-\tBUG_ON(tunnel->sock != NULL);\n-\tl2tp_info(tunnel, L2TP_MSG_CONTROL, \"%s: free...\\n\", tunnel->name);\n-\tkfree_rcu(tunnel, rcu);\n-}\n-\n /* Workqueue tunnel deletion function */\n static void l2tp_tunnel_del_work(struct work_struct *work)\n {\ndiff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h\nindex cdb6e3327f74..9101297f27ad 100644\n--- a/net/l2tp/l2tp_core.h\n+++ b/net/l2tp/l2tp_core.h\n@@ -231,6 +231,8 @@ static inline struct l2tp_tunnel *l2tp_sock_to_tunnel(struct sock *sk)\n \treturn tunnel;\n }\n \n+struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id);\n+\n struct l2tp_session *l2tp_session_get(const struct net *net,\n \t\t\t\t struct l2tp_tunnel *tunnel,\n \t\t\t\t u32 session_id, bool do_ref);\n@@ -269,6 +271,17 @@ int l2tp_nl_register_ops(enum l2tp_pwtype pw_type,\n void l2tp_nl_unregister_ops(enum l2tp_pwtype pw_type);\n int l2tp_ioctl(struct sock *sk, int cmd, unsigned long arg);\n \n+static inline void l2tp_tunnel_inc_refcount(struct l2tp_tunnel *tunnel)\n+{\n+\trefcount_inc(&tunnel->ref_count);\n+}\n+\n+static inline void l2tp_tunnel_dec_refcount(struct l2tp_tunnel *tunnel)\n+{\n+\tif (refcount_dec_and_test(&tunnel->ref_count))\n+\t\tkfree_rcu(tunnel, rcu);\n+}\n+\n /* Session reference counts. Incremented when code obtains a reference\n * to a session.\n */\ndiff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c\nindex 12cfcd0ca807..27ee94b5c189 100644\n--- a/net/l2tp/l2tp_netlink.c\n+++ b/net/l2tp/l2tp_netlink.c\n@@ -65,10 +65,12 @@ static struct l2tp_session *l2tp_nl_session_get(struct genl_info *info,\n \t\t (info->attrs[L2TP_ATTR_CONN_ID])) {\n \t\ttunnel_id = nla_get_u32(info->attrs[L2TP_ATTR_CONN_ID]);\n \t\tsession_id = nla_get_u32(info->attrs[L2TP_ATTR_SESSION_ID]);\n-\t\ttunnel = l2tp_tunnel_find(net, tunnel_id);\n-\t\tif (tunnel)\n+\t\ttunnel = l2tp_tunnel_get(net, tunnel_id);\n+\t\tif (tunnel) {\n \t\t\tsession = l2tp_session_get(net, tunnel, session_id,\n \t\t\t\t\t\t do_ref);\n+\t\t\tl2tp_tunnel_dec_refcount(tunnel);\n+\t\t}\n \t}\n \n \treturn session;\n", "prefixes": [ "net", "1/5" ] }