[PATCH net] l2tp: fix use-after-free in l2tp_udp_encap_recv()

[PATCH net] l2tp: fix use-after-free in l2tp_udp_encap_recv()

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

l2tp_udp_encap_recv() derives the tunnel pointer from the session it has
just looked up, so the reference taken by l2tp_v2_session_get() or
l2tp_v3_session_get() is the only thing that keeps the tunnel alive:
l2tp_session_free() drops the tunnel reference, and l2tp_tunnel_free()
hands the tunnel to kfree_rcu().  Both error paths of that function drop
the reference before jumping to the invalid label, which still uses the
tunnel:

	tunnel = session->tunnel;

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

If the session is torn down concurrently, such a put can be the last
one, and the statistics update then writes into a freed tunnel.

The frees are RCU deferred, which hides the bug on the regular path
because __udp4_lib_rcv() holds rcu_read_lock().  It does not hide it on
the backlog path: __release_sock() drops sk_lock before invoking
sk_backlog_rcv(), takes no RCU read lock and may cond_resched() between
packets, so a grace period can complete before the statistics update.

Account the packet first and move the put to the invalid label, which
can only be reached while the reference is still held, so the put is
not applied twice.

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 without accounting for the tunnel going away
with the session.

Found by inspection; no reproducer.  Hitting the window needs a session
teardown racing a packet on the backlog path, and the RCU deferred frees
mean KASAN is only likely to report it with CONFIG_RCU_STRICT_GRACE_PERIOD.

Fixes: ff6a2ac23cb0 ("l2tp: refactor udp recv to lookup to not use sk_user_data")
Cc: stable@vger.kernel.org
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
 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..36804743c761 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