All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Chapman <jchapman@katalix.com>
To: netdev@vger.kernel.org
Subject: [PATCH net-next v3 06/16] l2tp: refactor session lifetime handling
Date: Mon, 12 Feb 2018 17:33:29 +0000	[thread overview]
Message-ID: <1518456819-22244-7-git-send-email-jchapman@katalix.com> (raw)
In-Reply-To: <1518456819-22244-1-git-send-email-jchapman@katalix.com>

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 <jchapman@katalix.com>
---
 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;
 }
-- 
1.9.1

  parent reply	other threads:[~2018-02-12 17:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-12 17:33 [PATCH net-next v3 00/16] l2tp: fix API races discovered by syzbot James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 01/16] l2tp: update sk_user_data while holding sk_callback_lock James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 02/16] l2tp: add RCU read lock to protect tunnel ptr in ip socket destroy James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 03/16] l2tp: don't use inet_shutdown on tunnel destroy James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 04/16] l2tp: refactor tunnel lifetime handling wrt its socket James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 05/16] l2tp: use tunnel closing flag James Chapman
2018-02-12 17:33 ` James Chapman [this message]
2018-02-12 17:33 ` [PATCH net-next v3 07/16] l2tp: hide sessions if they are closing James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 08/16] l2tp: hide session from pppol2tp_sock_to_session if it is closing James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 09/16] l2tp: refactor pppol2tp_connect James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 10/16] l2tp: add session_free callback James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 11/16] l2tp: do session destroy using a workqueue James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 12/16] l2tp: simplify l2tp_tunnel_closeall James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 13/16] l2tp: refactor ppp session cleanup paths James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 14/16] l2tp: remove redundant sk_user_data check when creating tunnels James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 15/16] l2tp: remove unwanted error message James Chapman
2018-02-12 17:33 ` [PATCH net-next v3 16/16] l2tp: make __l2tp_session_unhash internal James Chapman
2018-02-12 19:00 ` [PATCH net-next v3 00/16] l2tp: fix API races discovered by syzbot David Miller

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=1518456819-22244-7-git-send-email-jchapman@katalix.com \
    --to=jchapman@katalix.com \
    --cc=netdev@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.