[PATCH net] l2tp: use list_del_rcu in l2tp_session_unhash

[PATCH net] l2tp: use list_del_rcu in l2tp_session_unhash

[Michael Bommarito]

An unprivileged local user can pin a host CPU indefinitely in
l2tp_session_get_by_ifname() by issuing L2TP_CMD_SESSION_GET on
L2TP_ATTR_IFNAME concurrently with L2TP_CMD_SESSION_CREATE and
L2TP_CMD_SESSION_DELETE on the same tunnel. All three commands take
GENL_UNS_ADMIN_PERM, so CAP_NET_ADMIN in the netns user namespace
suffices; on any host that has l2tp_core loaded the trigger is
reachable from a standard `unshare -Urn` sandbox.

l2tp_session_unhash() removes a session from tunnel->session_list
with list_del_init(), but that list is walked by
l2tp_session_get_by_ifname() with list_for_each_entry_rcu() under
rcu_read_lock_bh(). list_del_init() leaves the deleted entry's
next/prev self-pointing; a reader that has loaded the entry and
then advances pos->list.next reads &session->list, container_of()s
back to the same session, and list_for_each_entry_rcu() never
reaches the list head. The CPU stays in strcmp() inside the
walker, with BH and preemption disabled, so RCU grace periods on
the host stall behind it and the wedged thread cannot be killed
(SIGKILL is delivered on syscall return).

Use list_del_rcu() to match the existing list_add_rcu() in
l2tp_session_register(); the deleted session remains visible to
in-flight walkers with consistent next/prev pointers until
kfree_rcu() in l2tp_session_free() releases it. tunnel->session_list
has exactly one list_del_init() call site; the list_del_init
(&session->clist) at l2tp_core.c:533 operates on the per-collision
list, which is not walked under RCU. list_empty(&session->list) is
not used anywhere in net/l2tp/ after the unhash point, so dropping
the post-delete self-init is safe; the fix has no userspace-visible
behavior change.

Fixes: 89b768ec2dfef ("l2tp: use rcu list add/del when updating lists")
Cc: stable@vger.kernel.org # 6.11+
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
Distro reachability:

l2tp_core / l2tp_netlink autoload via the genl family alias
net-pf-16-proto-16-family-l2tp on first AF_NETLINK CTRL_CMD_GETFAMILY
lookup; this autoload does not require CAP_NET_ADMIN. Verified on
Ubuntu 26.04 dev as uid 1000 with no capabilities: `ip l2tp show
tunnel` triggers the load.

The userns CAP_NET_ADMIN gate is open to a standard unprivileged user
on Debian 11/12, Ubuntu 22.04 LTS, Arch, Alpine, and (via
snap / podman / firejail with a userns-permitting AppArmor profile)
Ubuntu 23.10+ / 24.04+. RHEL 8/9 and Fedora 31+ blacklist
l2tp_netlink in /etc/modprobe.d/ by default, so those distros require
admin enablement before the surface is reachable; once the module is
loaded (e.g. on a host running xl2tpd, NetworkManager-l2tp, or any
L2TPv3 VPN endpoint), the gate is open there too.

Reproduced on QEMU/x86_64 (KASAN + LOCKDEP + PROVE_RCU + PREEMPT
(full), SMP=4) and on bare-metal Ubuntu 7.0.0-14-generic with a
small create / delete + ifname-get harness. Stock kernel: first
rcu_preempt self-detected stall at 27 s, recurring every ~80 s,
8 stalls in a 600 s run; bare metal produced softlockup BUG: CPU
stuck for 26 s with RIP at l2tp_session_get_by_ifname+0x90 within
30 s of harness start, and the CPU remained pinned after harness
exit. Patched kernel under the same harness for 600 s: 282943
create/delete pairs plus 15381066 getter calls completed with zero
anomalies. The kernel ships no tools/testing/selftests/net/l2tp/
binaries and no l2tp KUnit module. Harness source available on
request.

 net/l2tp/l2tp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 157fc23ce4e14..1455f67e01ddb 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1360,7 +1360,7 @@ static void l2tp_session_unhash(struct l2tp_session *session)
 		spin_lock_bh(&pn->l2tp_session_idr_lock);

 		/* Remove from the per-tunnel list */
-		list_del_init(&session->list);
+		list_del_rcu(&session->list);

 		/* Remove from per-net IDR */
 		if (tunnel->version == L2TP_HDR_VER_3) {
--
2.46.0

Re: [PATCH net] l2tp: use list_del_rcu in l2tp_session_unhash

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
l2tp: use list_del_rcu in l2tp_session_unhash

An unprivileged local user can pin a host CPU indefinitely in
l2tp_session_get_by_ifname() when l2tp_session_unhash() uses list_del_init()
while the list is being walked under RCU. This commit changes
l2tp_session_unhash() to use list_del_rcu() to match the existing
list_add_rcu(), preventing the infinite loop.

> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> index 157fc23ce4e14..1455f67e01ddb 100644
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -1360,7 +1360,7 @@ static void l2tp_session_unhash(struct l2tp_session *session)
>  		spin_lock_bh(&pn->l2tp_session_idr_lock);
>  
>  		/* Remove from the per-tunnel list */
> -		list_del_init(&session->list);
> +		list_del_rcu(&session->list);

Can this change lead to a use-after-free in l2tp_session_get_by_ifname()?

By switching to list_del_rcu(), the deleted session remains visible to
in-flight RCU readers. If l2tp_session_del_work() drops the session's
reference count to 0 concurrently, the session will be queued for
destruction via kfree_rcu().

If an in-flight RCU reader in l2tp_session_get_by_ifname() encounters
this session before the grace period ends, it will execute:

	list_for_each_entry_rcu(session, &tunnel->session_list, list) {
		if (!strcmp(session->ifname, ifname)) {
			refcount_inc(&session->ref_count);
			rcu_read_unlock_bh();
			return session;
		}
	}

Since refcount_inc() does not protect against zero-reference transitions,
the reader will obtain a pointer to a session that is already pending
destruction.

Once the RCU grace period expires, kfree_rcu() will free the session
memory, causing a use-after-free for the caller.

Should l2tp_session_get_by_ifname() use refcount_inc_not_zero() and skip
the session if it returns false, similar to other getters like
l2tp_v3_session_get()?

[ ... ]

Re: [PATCH net] l2tp: use list_del_rcu in l2tp_session_unhash

[Jakub Kicinski]

On Thu, 21 May 2026 15:36:53 +0100 Simon Horman wrote:
> From: 'Simon Horman' <horms@kernel.org>
> 
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> l2tp: use list_del_rcu in l2tp_session_unhash
> 
> An unprivileged local user can pin a host CPU indefinitely in
> l2tp_session_get_by_ifname() when l2tp_session_unhash() uses list_del_init()
> while the list is being walked under RCU. This commit changes
> l2tp_session_unhash() to use list_del_rcu() to match the existing
> list_add_rcu(), preventing the infinite loop.
> 
> > diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> > index 157fc23ce4e14..1455f67e01ddb 100644
> > --- a/net/l2tp/l2tp_core.c
> > +++ b/net/l2tp/l2tp_core.c
> > @@ -1360,7 +1360,7 @@ static void l2tp_session_unhash(struct l2tp_session *session)
> >  		spin_lock_bh(&pn->l2tp_session_idr_lock);
> >  
> >  		/* Remove from the per-tunnel list */
> > -		list_del_init(&session->list);
> > +		list_del_rcu(&session->list);  
> 
> Can this change lead to a use-after-free in l2tp_session_get_by_ifname()?
> 
> By switching to list_del_rcu(), the deleted session remains visible to
> in-flight RCU readers. If l2tp_session_del_work() drops the session's
> reference count to 0 concurrently, the session will be queued for
> destruction via kfree_rcu().

Saw this, but I don't get how list_del_init() -> list_del_rcu()
can change visibility to the readers :| Without digging into the code
this reads like Gemini reporting a different pre-existing issue.

Michael, could you investigate?

> If an in-flight RCU reader in l2tp_session_get_by_ifname() encounters
> this session before the grace period ends, it will execute:
> 
> 	list_for_each_entry_rcu(session, &tunnel->session_list, list) {
> 		if (!strcmp(session->ifname, ifname)) {
> 			refcount_inc(&session->ref_count);
> 			rcu_read_unlock_bh();
> 			return session;
> 		}
> 	}
> 
> Since refcount_inc() does not protect against zero-reference transitions,
> the reader will obtain a pointer to a session that is already pending
> destruction.
> 
> Once the RCU grace period expires, kfree_rcu() will free the session
> memory, causing a use-after-free for the caller.
> 
> Should l2tp_session_get_by_ifname() use refcount_inc_not_zero() and skip
> the session if it returns false, similar to other getters like
> l2tp_v3_session_get()?

Re: [PATCH net] l2tp: use list_del_rcu in l2tp_session_unhash

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 18 May 2026 14:34:47 -0400 you wrote:
> An unprivileged local user can pin a host CPU indefinitely in
> l2tp_session_get_by_ifname() by issuing L2TP_CMD_SESSION_GET on
> L2TP_ATTR_IFNAME concurrently with L2TP_CMD_SESSION_CREATE and
> L2TP_CMD_SESSION_DELETE on the same tunnel. All three commands take
> GENL_UNS_ADMIN_PERM, so CAP_NET_ADMIN in the netns user namespace
> suffices; on any host that has l2tp_core loaded the trigger is
> reachable from a standard `unshare -Urn` sandbox.
> 
> [...]

Here is the summary with links:
  - [net] l2tp: use list_del_rcu in l2tp_session_unhash
    https://git.kernel.org/netdev/net/c/979c017803c4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html