Re: [PATCH net-next v2] l2tp: bump tunnel stats before dropping the session reference

[Simon Horman]

On Tue, Nov 03, 2026 at 04:42:49PM +0800, Linkui Xiao wrote:
> From: Linkui Xiao <xiaolinkui@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
> 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@xxxxxxx/

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>