Re: [PATCH] net: l2tp: fix refcount leak in pppol2tp_tunnel_get()

[Tom Parkin <tparkin@katalix.com>]

[-- Attachment #1: Type: text/plain, Size: 2574 bytes --]

On  Fri, Jun 12, 2026 at 00:45:42 +0800, WenTao Liang wrote:
> pppol2tp_tunnel_get() increments the tunnel's refcount via
> refcount_inc() on the tunnel returned from l2tp_tunnel_create(). If
> l2tp_tunnel_register() subsequently fails, the error path frees the
> tunnel via kfree(), bypassing the refcount entirely. At that point the
> refcount is 2 (one from l2tp_tunnel_create() and one from the
> refcount_inc()), so the reference counts leak and the memory is freed
> while still referenced, creating use-after-free potential.

l2tp_tunnel_register only adds the newly created tunnel instance to
the per-net IDR once all the validation and initialisation steps have
completed successfully: there's no way for it to add the tunnel to the
IDR and still return an error.

Hence the new tunnel instance isn't visible to the rest of the system
if l2tp_tunnel_register returns an error.

As such, there's no potential for UAF when directly freeing the tunnel
structure in the way the code currently does.

Possibly there's value to altering the error path since it'll free the
tunnel instance via. l2tp_tunnel_free which will perform tidyup that
calling kfree() directly doesn't.

If the latter is a motivation it'd make more sense to say so in the
commit comment IMO since I don't think the UAF argument holds in the
current codebase.

> Fix this by using the standard reference counting API: replace the
> kfree() with two l2tp_tunnel_put() calls to properly release both
> references. This mirrors the cleanup used in other error paths within
> the same function.

There's no other double l2tp_tunnel_put in pppol2tp_tunnel_get that I
can see.

> 
> Cc: stable@vger.kernel.org
> Fixes: 6b9f34239b00 ("l2tp: fix races in tunnel creation")
> Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
> ---
>  net/l2tp/l2tp_ppp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
> index e0b1915be1a6..0ddfc1893121 100644
> --- a/net/l2tp/l2tp_ppp.c
> +++ b/net/l2tp/l2tp_ppp.c
> @@ -661,7 +661,8 @@ static struct l2tp_tunnel *pppol2tp_tunnel_get(struct net *net,
>  			refcount_inc(&tunnel->ref_count);
>  			error = l2tp_tunnel_register(tunnel, net, &tcfg);
>  			if (error < 0) {
> -				kfree(tunnel);
> +				l2tp_tunnel_put(tunnel);
> +				l2tp_tunnel_put(tunnel);
>  				return ERR_PTR(error);
>  			}
>  
> -- 
> 2.50.1 (Apple Git-155)
-- 
Tom Parkin
Katalix Systems Ltd
https://katalix.com
Catalysts for your Embedded Linux software development

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]