From: James Chapman <jchapman@katalix.com>
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 [thread overview]
Message-ID: <1518188432-9245-14-git-send-email-jchapman@katalix.com> (raw)
In-Reply-To: <1518188432-9245-1-git-send-email-jchapman@katalix.com>
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 <jchapman@katalix.com>
---
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
--
1.9.1
next prev parent reply other threads:[~2018-02-09 15:00 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-09 15:00 [PATCH net-next 00/16] l2tp: fix API races discovered by syzbot James Chapman
2018-02-09 15:00 ` [PATCH net-next 01/16] l2tp: update sk_user_data while holding sk_callback_lock James Chapman
2018-02-09 15:00 ` [PATCH net-next 02/16] l2tp: add RCU read lock to protect tunnel ptr in ip socket destroy James Chapman
2018-02-09 15:00 ` [PATCH net-next 03/16] l2tp: don't use inet_shutdown on tunnel destroy James Chapman
2018-02-09 15:00 ` [PATCH net-next 04/16] l2tp: refactor tunnel lifetime handling wrt its socket James Chapman
2018-02-09 15:00 ` [PATCH net-next 05/16] l2tp: use tunnel closing flag James Chapman
2018-02-09 15:00 ` [PATCH net-next 06/16] l2tp: refactor session lifetime handling James Chapman
2018-02-09 15:00 ` [PATCH net-next 07/16] l2tp: hide sessions if they are closing James Chapman
2018-02-12 3:37 ` kbuild test robot
2018-02-09 15:00 ` [PATCH net-next 08/16] l2tp: hide session from pppol2tp_sock_to_session if it is closing James Chapman
2018-02-09 15:00 ` [PATCH net-next 09/16] l2tp: refactor pppol2tp_connect James Chapman
2018-02-09 15:00 ` [PATCH net-next 10/16] l2tp: add session_free callback James Chapman
2018-02-09 15:00 ` [PATCH net-next 11/16] l2tp: do session destroy using a workqueue James Chapman
2018-02-09 15:00 ` [PATCH net-next 12/16] l2tp: simplify l2tp_tunnel_closeall James Chapman
2018-02-09 15:00 ` James Chapman [this message]
2018-02-09 15:00 ` [PATCH net-next 14/16] l2tp: remove redundant sk_user_data check when creating tunnels James Chapman
2018-02-09 15:00 ` [PATCH net-next 15/16] l2tp: remove unwanted error message James Chapman
2018-02-09 15:00 ` [PATCH net-next 16/16] l2tp: make __l2tp_session_unhash internal James Chapman
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=1518188432-9245-14-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).