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

[Jakub Kicinski <kuba@kernel.org>]

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()?