[PATCH 6.6.y] Revert "l2tp: do not use sock_hold() in pppol2tp_session_get_sock()"

Gaosheng Cui posted 1 patch 1 month, 1 week ago
net/l2tp/l2tp_ppp.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
[PATCH 6.6.y] Revert "l2tp: do not use sock_hold() in pppol2tp_session_get_sock()"
Posted by Gaosheng Cui 1 month, 1 week ago
This reverts commit ce63943f9bce64df1be9b6a65b04fa6e1d99ec2c.

Upstream commit 9b8c88f875c0 ("l2tp: do not use sock_hold() in
pppol2tp_session_get_sock()") was backported to v6.6.130.

The blamed commit c5cbaef992d6 ("l2tp: refactor ppp socket/session
relationship") was introduced in v6.12 and was never backported to 6.6.y.

Revert it from 6.6.y to avoid incorrect reference counting and
potential use-after-free.

This is a revert of a backport, so there is no upstream commit.

Fixes: ce63943f9bce ("l2tp: do not use sock_hold() in pppol2tp_session_get_sock()")
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
Cc: <stable@vger.kernel.org> # 6.6
---
 net/l2tp/l2tp_ppp.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 34d8582c0c07..6146e4e67bbb 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -130,12 +130,22 @@ static const struct ppp_channel_ops pppol2tp_chan_ops = {
 
 static const struct proto_ops pppol2tp_ops;
 
-/* Retrieves the pppol2tp socket associated to a session. */
+/* Retrieves the pppol2tp socket associated to a session.
+ * A reference is held on the returned socket, so this function must be paired
+ * with sock_put().
+ */
 static struct sock *pppol2tp_session_get_sock(struct l2tp_session *session)
 {
 	struct pppol2tp_session *ps = l2tp_session_priv(session);
+	struct sock *sk;
 
-	return rcu_dereference(ps->sk);
+	rcu_read_lock();
+	sk = rcu_dereference(ps->sk);
+	if (sk)
+		sock_hold(sk);
+	rcu_read_unlock();
+
+	return sk;
 }
 
 /* Helpers to obtain tunnel/session contexts from sockets.
@@ -201,13 +211,14 @@ static int pppol2tp_recvmsg(struct socket *sock, struct msghdr *msg,
 
 static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int data_len)
 {
-	struct sock *sk;
+	struct pppol2tp_session *ps = l2tp_session_priv(session);
+	struct sock *sk = NULL;
 
 	/* If the socket is bound, send it in to PPP's input queue. Otherwise
 	 * queue it on the session socket.
 	 */
 	rcu_read_lock();
-	sk = pppol2tp_session_get_sock(session);
+	sk = rcu_dereference(ps->sk);
 	if (!sk)
 		goto no_sock;
 
@@ -517,14 +528,13 @@ static void pppol2tp_show(struct seq_file *m, void *arg)
 	struct l2tp_session *session = arg;
 	struct sock *sk;
 
-	rcu_read_lock();
 	sk = pppol2tp_session_get_sock(session);
 	if (sk) {
 		struct pppox_sock *po = pppox_sk(sk);
 
 		seq_printf(m, "   interface %s\n", ppp_dev_name(&po->chan));
+		sock_put(sk);
 	}
-	rcu_read_unlock();
 }
 
 static void pppol2tp_session_init(struct l2tp_session *session)
@@ -1530,7 +1540,6 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v)
 		port = ntohs(inet->inet_sport);
 	}
 
-	rcu_read_lock();
 	sk = pppol2tp_session_get_sock(session);
 	if (sk) {
 		state = sk->sk_state;
@@ -1566,8 +1575,8 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v)
 		struct pppox_sock *po = pppox_sk(sk);
 
 		seq_printf(m, "   interface %s\n", ppp_dev_name(&po->chan));
+		sock_put(sk);
 	}
-	rcu_read_unlock();
 }
 
 static int pppol2tp_seq_show(struct seq_file *m, void *v)
-- 
2.43.0
Re: [PATCH 6.6.y] Revert "l2tp: do not use sock_hold() in pppol2tp_session_get_sock()"
Posted by Qingfang Deng 1 month, 1 week ago
Hi, Gaosheng,

On Wed, May 6, 2026 at 8:01 PM Gaosheng Cui <cuigaosheng1@huawei.com> wrote:
>
> This reverts commit ce63943f9bce64df1be9b6a65b04fa6e1d99ec2c.
>
> Upstream commit 9b8c88f875c0 ("l2tp: do not use sock_hold() in
> pppol2tp_session_get_sock()") was backported to v6.6.130.
>
> The blamed commit c5cbaef992d6 ("l2tp: refactor ppp socket/session
> relationship") was introduced in v6.12 and was never backported to 6.6.y.
>
> Revert it from 6.6.y to avoid incorrect reference counting and
> potential use-after-free.
>
> This is a revert of a backport, so there is no upstream commit.
>
> Fixes: ce63943f9bce ("l2tp: do not use sock_hold() in pppol2tp_session_get_sock()")
> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>

Sorry for the confusion. The backport is meant to fix another issue,
but the commit message was not updated to reflect the actual use.
Link: https://lore.kernel.org/stable/20260318012653.232518-1-dqfext@gmail.com/

The backport is correct and should not be reverted.

Regards,
Qingfang