mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linkui Xiao <xiaolinkui@126.com>
To: davem@davemloft.net, edumazet@kernel.org, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, michael.bommarito@gmail.com,
	gnault@redhat.com, alice.kernel@fastmail.im, kuniyu@google.com,
	nicoyip.dev@gmail.com, mail@david-bauer.net
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Linkui Xiao <xiaolinkui@kylinos.cn>
Subject: [PATCH net-next v2] l2tp: bump tunnel stats before dropping the session reference
Date: Tue,  3 Nov 2026 16:42:49 +0800	[thread overview]
Message-ID: <20261103084249.1337657-1-xiaolinkui@126.com> (raw)

From: Linkui Xiao <xiaolinkui@kylinos.cn>

l2tp_udp_encap_recv() derives the tunnel pointer from the session it
has just looked up, so the session reference is the only thing keeping
the tunnel alive.  Both error paths of the function drop the reference
before jumping to the invalid label, which still dereferences the
tunnel:

	tunnel = session->tunnel;

	if (version != tunnel->version) {
		l2tp_session_put(session);
		goto invalid;
	}
	...
invalid:
	atomic_long_inc(&tunnel->stats.rx_invalid);

Reorder the error paths so the statistics are updated before the
reference is dropped, removing the dependency on the tunnel staying
allocated after its last reference is gone.

Currently this is safe by accident: the only live entry point is the
UDP encap_rcv hook, which is invoked under rcu_read_lock(), and the
tunnel is freed by kfree_rcu(), so the tunnel memory outlives the
dereference.  Making the order explicitly correct keeps the function
safe regardless of the calling context, instead of relying on every
future caller holding an RCU read-side reference.

The put on the version mismatch path came later, with commit
4d10edfd1475 ("l2tp: Fix memleak in l2tp_udp_encap_recv()."), which
plugged a session leak.

Found by inspection.
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
Changes in v2:
- Describe the change as hardening instead of a reachable
  use-after-free: the socket backlog path pointed at by the v1 commit
  message cannot be reached, and the live receive path always holds an
  RCU read-side reference, so the old ordering is currently safe
  (Sashiko AI review).
- Drop the backport tags (the Fixes tag and the Cc to stable)
  accordingly and retitle the patch.
- Correct the index line: the v1 patch carried the blob hashes of an
  earlier draft of this change, so its new hash did not match the file
  the diff actually produces.
v1:
- Link: https://lore.kernel.org/all/20261027063657.1359170-1-xiaolinkui@126.com/

 net/l2tp/l2tp_core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 7e042a3e648c..b7183b527fc4 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1083,16 +1083,12 @@ int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 	tunnel = session->tunnel;
 
 	/* Check protocol version */
-	if (version != tunnel->version) {
-		l2tp_session_put(session);
+	if (version != tunnel->version)
 		goto invalid;
-	}
 
 	if (version == L2TP_HDR_VER_3 &&
-	    l2tp_v3_ensure_opt_in_linear(session, skb, &ptr, &optr)) {
-		l2tp_session_put(session);
+	    l2tp_v3_ensure_opt_in_linear(session, skb, &ptr, &optr))
 		goto invalid;
-	}
 
 	l2tp_recv_common(session, skb, ptr, optr, hdrflags, length);
 	l2tp_session_put(session);
@@ -1100,7 +1096,11 @@ int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 	return 0;
 
 invalid:
+	/* The session reference is the only thing keeping the tunnel alive
+	 * here, so account the invalid packet before dropping it.
+	 */
 	atomic_long_inc(&tunnel->stats.rx_invalid);
+	l2tp_session_put(session);
 
 pass:
 	/* Put UDP header back */
-- 
2.25.1


                 reply	other threads:[~2026-11-03  8:43 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20261103084249.1337657-1-xiaolinkui@126.com \
    --to=xiaolinkui@126.com \
    --cc=alice.kernel@fastmail.im \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=gnault@redhat.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mail@david-bauer.net \
    --cc=michael.bommarito@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nicoyip.dev@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=xiaolinkui@kylinos.cn \
    /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

all inboxes | Powered by JetHome®