| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1518456819-22244-14-git-send-email-jchapman@katalix.com>
Date: Mon, 12 Feb 2018 17:33:36 +0000
From: James Chapman <jchapman@...alix.com>
To: netdev@...r.kernel.org
Subject: [PATCH net-next v3 13/16] l2tp: refactor ppp session cleanup paths
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@...kaller.appspotmail.com
Reported-by: syzbot+6e6a5ec8de31a94cd015@...kaller.appspotmail.com
Reported-by: syzbot+19c09769f14b48810113@...kaller.appspotmail.com
Reported-by: syzbot+347bd5acde002e353a36@...kaller.appspotmail.com
Signed-off-by: James Chapman <jchapman@...alix.com>
---
net/l2tp/l2tp_ppp.c | 109 +++++++++++++++++++++++++++++-----------------------
1 file changed, 60 insertions(+), 49 deletions(-)
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 19efb56620ab..e50feb1479e6 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -436,18 +436,68 @@ static void pppol2tp_attach(struct l2tp_session *session, struct sock *sk)
rcu_assign_pointer(ps->sk, sk);
}
-/* Called by l2tp_core when a session socket is being closed.
+/* called with ps->sk_lock */
+static void pppol2tp_detach(struct l2tp_session *session, struct sock *sk)
+{
+ struct pppol2tp_session *ps = l2tp_session_priv(session);
+
+ rcu_assign_pointer(ps->sk, NULL);
+ write_lock_bh(&sk->sk_callback_lock);
+ rcu_assign_sk_user_data(sk, NULL);
+ write_unlock_bh(&sk->sk_callback_lock);
+}
+
+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);
}
}
@@ -457,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.
@@ -509,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
@@ -613,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
Powered by blists - more mailing lists