All of lore.kernel.org
 help / color / mirror / Atom feed
From: Giuliano Procida <gprocida@google.com>
To: greg@kroah.com
Cc: stable@vger.kernel.org, Guillaume Nault <g.nault@alphalink.fr>,
	"David S . Miller" <davem@davemloft.net>,
	Amit Pundir <amit.pundir@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Giuliano Procida <gprocida@google.com>
Subject: [PATCH 04/27] l2tp: take a reference on sessions used in genetlink handlers
Date: Fri, 22 May 2020 00:57:17 +0100	[thread overview]
Message-ID: <20200521235740.191338-5-gprocida@google.com> (raw)
In-Reply-To: <20200521235740.191338-1-gprocida@google.com>

From: Guillaume Nault <g.nault@alphalink.fr>

commit 2777e2ab5a9cf2b4524486c6db1517a6ded25261 upstream.

Callers of l2tp_nl_session_find() need to hold a reference on the
returned session since there's no guarantee that it isn't going to
disappear from under them.

Relying on the fact that no l2tp netlink message may be processed
concurrently isn't enough: sessions can be deleted by other means
(e.g. by closing the PPPOL2TP socket of a ppp pseudowire).

l2tp_nl_cmd_session_delete() is a bit special: it runs a callback
function that may require a previous call to session->ref(). In
particular, for ppp pseudowires, the callback is l2tp_session_delete(),
which then calls pppol2tp_session_close() and dereferences the PPPOL2TP
socket. The socket might already be gone at the moment
l2tp_session_delete() calls session->ref(), so we need to take a
reference during the session lookup. So we need to pass the do_ref
variable down to l2tp_session_get() and l2tp_session_get_by_ifname().

Since all callers have to be updated, l2tp_session_find_by_ifname() and
l2tp_nl_session_find() are renamed to reflect their new behaviour.

Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP")
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 net/l2tp/l2tp_core.c    |  9 +++++++--
 net/l2tp/l2tp_core.h    |  3 ++-
 net/l2tp/l2tp_netlink.c | 39 ++++++++++++++++++++++++++-------------
 3 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 8cbccddc0b1e..08377af93552 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -355,7 +355,8 @@ EXPORT_SYMBOL_GPL(l2tp_session_get_nth);
 /* Lookup a session by interface name.
  * This is very inefficient but is only used by management interfaces.
  */
-struct l2tp_session *l2tp_session_find_by_ifname(struct net *net, char *ifname)
+struct l2tp_session *l2tp_session_get_by_ifname(struct net *net, char *ifname,
+						bool do_ref)
 {
 	struct l2tp_net *pn = l2tp_pernet(net);
 	int hash;
@@ -365,7 +366,11 @@ struct l2tp_session *l2tp_session_find_by_ifname(struct net *net, char *ifname)
 	for (hash = 0; hash < L2TP_HASH_SIZE_2; hash++) {
 		hlist_for_each_entry_rcu(session, &pn->l2tp_session_hlist[hash], global_hlist) {
 			if (!strcmp(session->ifname, ifname)) {
+				l2tp_session_inc_refcount(session);
+				if (do_ref && session->ref)
+					session->ref(session);
 				rcu_read_unlock_bh();
+
 				return session;
 			}
 		}
@@ -375,7 +380,7 @@ struct l2tp_session *l2tp_session_find_by_ifname(struct net *net, char *ifname)
 
 	return NULL;
 }
-EXPORT_SYMBOL_GPL(l2tp_session_find_by_ifname);
+EXPORT_SYMBOL_GPL(l2tp_session_get_by_ifname);
 
 static int l2tp_session_add_to_tunnel(struct l2tp_tunnel *tunnel,
 				      struct l2tp_session *session)
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 06323a12d62c..ee59d9961d1f 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -252,7 +252,8 @@ struct l2tp_session *l2tp_session_find(struct net *net,
 				       u32 session_id);
 struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth,
 					  bool do_ref);
-struct l2tp_session *l2tp_session_find_by_ifname(struct net *net, char *ifname);
+struct l2tp_session *l2tp_session_get_by_ifname(struct net *net, char *ifname,
+						bool do_ref);
 struct l2tp_tunnel *l2tp_tunnel_find(struct net *net, u32 tunnel_id);
 struct l2tp_tunnel *l2tp_tunnel_find_nth(struct net *net, int nth);
 
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 43f0af1c4697..63a2430ff40a 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -55,7 +55,8 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq,
 /* Accessed under genl lock */
 static const struct l2tp_nl_cmd_ops *l2tp_nl_cmd_ops[__L2TP_PWTYPE_MAX];
 
-static struct l2tp_session *l2tp_nl_session_find(struct genl_info *info)
+static struct l2tp_session *l2tp_nl_session_get(struct genl_info *info,
+						bool do_ref)
 {
 	u32 tunnel_id;
 	u32 session_id;
@@ -66,14 +67,15 @@ static struct l2tp_session *l2tp_nl_session_find(struct genl_info *info)
 
 	if (info->attrs[L2TP_ATTR_IFNAME]) {
 		ifname = nla_data(info->attrs[L2TP_ATTR_IFNAME]);
-		session = l2tp_session_find_by_ifname(net, ifname);
+		session = l2tp_session_get_by_ifname(net, ifname, do_ref);
 	} else if ((info->attrs[L2TP_ATTR_SESSION_ID]) &&
 		   (info->attrs[L2TP_ATTR_CONN_ID])) {
 		tunnel_id = nla_get_u32(info->attrs[L2TP_ATTR_CONN_ID]);
 		session_id = nla_get_u32(info->attrs[L2TP_ATTR_SESSION_ID]);
 		tunnel = l2tp_tunnel_find(net, tunnel_id);
 		if (tunnel)
-			session = l2tp_session_find(net, tunnel, session_id);
+			session = l2tp_session_get(net, tunnel, session_id,
+						   do_ref);
 	}
 
 	return session;
@@ -644,7 +646,7 @@ static int l2tp_nl_cmd_session_delete(struct sk_buff *skb, struct genl_info *inf
 	struct l2tp_session *session;
 	u16 pw_type;
 
-	session = l2tp_nl_session_find(info);
+	session = l2tp_nl_session_get(info, true);
 	if (session == NULL) {
 		ret = -ENODEV;
 		goto out;
@@ -658,6 +660,10 @@ static int l2tp_nl_cmd_session_delete(struct sk_buff *skb, struct genl_info *inf
 		if (l2tp_nl_cmd_ops[pw_type] && l2tp_nl_cmd_ops[pw_type]->session_delete)
 			ret = (*l2tp_nl_cmd_ops[pw_type]->session_delete)(session);
 
+	if (session->deref)
+		session->deref(session);
+	l2tp_session_dec_refcount(session);
+
 out:
 	return ret;
 }
@@ -667,7 +673,7 @@ static int l2tp_nl_cmd_session_modify(struct sk_buff *skb, struct genl_info *inf
 	int ret = 0;
 	struct l2tp_session *session;
 
-	session = l2tp_nl_session_find(info);
+	session = l2tp_nl_session_get(info, false);
 	if (session == NULL) {
 		ret = -ENODEV;
 		goto out;
@@ -702,6 +708,8 @@ static int l2tp_nl_cmd_session_modify(struct sk_buff *skb, struct genl_info *inf
 	ret = l2tp_session_notify(&l2tp_nl_family, info,
 				  session, L2TP_CMD_SESSION_MODIFY);
 
+	l2tp_session_dec_refcount(session);
+
 out:
 	return ret;
 }
@@ -788,29 +796,34 @@ static int l2tp_nl_cmd_session_get(struct sk_buff *skb, struct genl_info *info)
 	struct sk_buff *msg;
 	int ret;
 
-	session = l2tp_nl_session_find(info);
+	session = l2tp_nl_session_get(info, false);
 	if (session == NULL) {
 		ret = -ENODEV;
-		goto out;
+		goto err;
 	}
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg) {
 		ret = -ENOMEM;
-		goto out;
+		goto err_ref;
 	}
 
 	ret = l2tp_nl_session_send(msg, info->snd_portid, info->snd_seq,
 				   0, session, L2TP_CMD_SESSION_GET);
 	if (ret < 0)
-		goto err_out;
+		goto err_ref_msg;
 
-	return genlmsg_unicast(genl_info_net(info), msg, info->snd_portid);
+	ret = genlmsg_unicast(genl_info_net(info), msg, info->snd_portid);
 
-err_out:
-	nlmsg_free(msg);
+	l2tp_session_dec_refcount(session);
 
-out:
+	return ret;
+
+err_ref_msg:
+	nlmsg_free(msg);
+err_ref:
+	l2tp_session_dec_refcount(session);
+err:
 	return ret;
 }
 
-- 
2.27.0.rc0.183.gde8f92d652-goog


  parent reply	other threads:[~2020-05-21 23:58 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-21 23:57 [PATCH 00/27] more l2tp locking and ordering fixes Giuliano Procida
2020-05-21 23:57 ` [PATCH 01/27] l2tp: lock socket before checking flags in connect() Giuliano Procida
2020-05-21 23:57 ` [PATCH 02/27] l2tp: fix racy socket lookup in l2tp_ip and l2tp_ip6 bind() Giuliano Procida
2020-05-21 23:57 ` [PATCH 03/27] l2tp: hold session while sending creation notifications Giuliano Procida
2020-05-21 23:57 ` Giuliano Procida [this message]
2020-05-21 23:57 ` [PATCH 05/27] l2tp: don't use l2tp_tunnel_find() in l2tp_ip and l2tp_ip6 Giuliano Procida
2020-05-21 23:57 ` [PATCH 06/27] net: l2tp: export debug flags to UAPI Giuliano Procida
2020-05-21 23:57 ` [PATCH 07/27] net: l2tp: deprecate PPPOL2TP_MSG_* in favour of L2TP_MSG_* Giuliano Procida
2020-05-26 16:17   ` Asbjørn Sloth Tønnesen
2020-05-21 23:57 ` [PATCH 08/27] net: l2tp: ppp: change PPPOL2TP_MSG_* => L2TP_MSG_* Giuliano Procida
2020-05-21 23:57 ` [PATCH 09/27] New kernel function to get IP overhead on a socket Giuliano Procida
2020-05-21 23:57 ` [PATCH 10/27] L2TP:Adjust intf MTU, add underlay L3, L2 hdrs Giuliano Procida
2020-05-21 23:57 ` [PATCH 11/27] l2tp: remove useless duplicate session detection in l2tp_netlink Giuliano Procida
2020-05-21 23:57 ` [PATCH 12/27] l2tp: remove l2tp_session_find() Giuliano Procida
2020-05-21 23:57 ` [PATCH 13/27] l2tp: define parameters of l2tp_session_get*() as "const" Giuliano Procida
2020-05-21 23:57 ` [PATCH 14/27] l2tp: define parameters of l2tp_tunnel_find*() " Giuliano Procida
2020-05-21 23:57 ` [PATCH 15/27] l2tp: initialise session's refcount before making it reachable Giuliano Procida
2020-05-21 23:57 ` [PATCH 16/27] l2tp: hold tunnel while looking up sessions in l2tp_netlink Giuliano Procida
2020-05-21 23:57 ` [PATCH 17/27] l2tp: hold tunnel while processing genl delete command Giuliano Procida
2020-05-21 23:57 ` [PATCH 18/27] l2tp: hold tunnel while handling genl tunnel updates Giuliano Procida
2020-05-21 23:57 ` [PATCH 19/27] l2tp: hold tunnel while handling genl TUNNEL_GET commands Giuliano Procida
2020-05-21 23:57 ` [PATCH 20/27] l2tp: hold tunnel used while creating sessions with netlink Giuliano Procida
2020-05-21 23:57 ` [PATCH 21/27] l2tp: prevent creation of sessions on terminated tunnels Giuliano Procida
2020-05-21 23:57 ` [PATCH 22/27] l2tp: pass tunnel pointer to ->session_create() Giuliano Procida
2020-05-21 23:57 ` [PATCH 23/27] l2tp: fix l2tp_eth module loading Giuliano Procida
2020-05-21 23:57 ` [PATCH 24/27] l2tp: don't register sessions in l2tp_session_create() Giuliano Procida
2020-05-21 23:57 ` [PATCH 25/27] l2tp: initialise l2tp_eth sessions before registering them Giuliano Procida
2020-05-21 23:57 ` [PATCH 26/27] l2tp: protect sock pointer of struct pppol2tp_session with RCU Giuliano Procida
2020-05-21 23:57 ` [PATCH 27/27] l2tp: initialise PPP sessions before registering them Giuliano Procida
2020-05-26 10:54 ` [PATCH 00/27] more l2tp locking and ordering fixes Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200521235740.191338-5-gprocida@google.com \
    --to=gprocida@google.com \
    --cc=amit.pundir@linaro.org \
    --cc=davem@davemloft.net \
    --cc=g.nault@alphalink.fr \
    --cc=greg@kroah.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.