[PATCH net] l2tp: use refcount_inc_not_zero in l2tp_session_get_by_ifname

Michael Bommarito posted 1 patch 1 day, 16 hours ago
net/l2tp/l2tp_core.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
[PATCH net] l2tp: use refcount_inc_not_zero in l2tp_session_get_by_ifname
Posted by Michael Bommarito 1 day, 16 hours ago
A reader in l2tp_session_get_by_ifname() can return a pointer to a
session whose refcount has reached zero. The getter takes its
reference with plain refcount_inc(), but every other session getter
in the same file (l2tp_v2_session_get, l2tp_v3_session_get, and the
corresponding _get_next variants) uses refcount_inc_not_zero()
because the IDR/RCU lookup can race with refcount_dec_and_test() ->
l2tp_session_free() -> kfree_rcu(). The ifname getter is the only
outlier; the inconsistency was raised on-list after 979c017803c4
("l2tp: use list_del_rcu in l2tp_session_unhash").

A reader inside rcu_read_lock_bh() that matches session->ifname can
be preempted between the strcmp() and the refcount_inc(). If the
last reference drops on another CPU in that window, the reader's
refcount_inc() runs on a counter that has reached zero. refcount_t
catches the addition-on-zero, prints "refcount_t: addition on 0;
use-after-free", saturates the counter, and returns the saturated
pointer to the caller. Session memory is held live by the in-flight
RCU read section, but the kfree_rcu() callback queued from
l2tp_session_free() will free it once the grace period closes; a
caller that dereferences the returned session past that point hits
a slab-use-after-free. On PREEMPT_RT local_bh_disable() is a per-CPU
sleeping lock and the preemption window is real; on stock PREEMPT
kernels local_bh_disable() is a preempt_count increment that closes
the cross-CPU race in practice (see below).

Use refcount_inc_not_zero() and continue the list walk on failure,
matching the other session getters in the file. The ifname getter
is the only session getter in net/l2tp/ that still uses the bare
refcount_inc() pattern; this change restores file-internal
consistency. The success path is unchanged.

Fixes: abe7a1a7d0b6 ("l2tp: improve tunnel/session refcount helpers")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
Distro reachability:

l2tp_core and l2tp_netlink autoload via the genl family alias
net-pf-16-proto-16-family-l2tp on first AF_NETLINK CTRL_CMD_GETFAMILY
lookup; pppol2tp autoloads on demand via net-l2tp-type-7 from
L2TP_CMD_SESSION_CREATE with pw_type=L2TP_PWTYPE_PPP. Neither
autoload requires CAP_SYS_MODULE. SESSION_GET / SESSION_CREATE /
SESSION_DELETE are GENL_UNS_ADMIN_PERM, so CAP_NET_ADMIN in the
netns user namespace suffices and is reachable from `unshare -Urn`
on Debian 11/12, Ubuntu 22.04 LTS / 23.10+ / 24.04+, Arch, Alpine;
RHEL/Fedora blacklist l2tp_netlink by default but on hosts running
NetworkManager-l2tp, xl2tpd, or any L2TPv3 endpoint the gate is open
there too. PREEMPT_RT ships on Ubuntu Pro Realtime, RHEL 9 RT, SUSE
RT, and Yocto-RT.

Reproduced on x86_64 QEMU/KVM. Under tracefs-kprobe widening at the
refcount-inc instruction, a PREEMPT_RT build hits "refcount_t:
addition on 0; use-after-free" through l2tp_session_get_by_ifname at
533 s; patched runs 1800 s clean. A stock PREEMPT (non-RT) build
with the same widening ran 1800 s without firing: local_bh_disable()
on non-RT prevents preemption inside the RCU section so the
cross-CPU window does not open at observable rates.

 net/l2tp/l2tp_core.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 1455f67e01ddb..0000000000000 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -441,12 +441,13 @@ struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net,
 	idr_for_each_entry_ul(&pn->l2tp_tunnel_idr, tunnel, tmp, tunnel_id) {
 		if (tunnel) {
 			list_for_each_entry_rcu(session, &tunnel->session_list, list) {
-				if (!strcmp(session->ifname, ifname)) {
-					refcount_inc(&session->ref_count);
-					rcu_read_unlock_bh();
+				if (strcmp(session->ifname, ifname))
+					continue;
+				if (!refcount_inc_not_zero(&session->ref_count))
+					continue;
+				rcu_read_unlock_bh();

-					return session;
-				}
+				return session;
 			}
 		}
 	}
--
2.46.0