Re: [PATCH net 0/1] net: l2tp: ignore multicast notification errors in netlink commands

[Tom Parkin]

On  Tue, Aug 25, 2026 at 18:35:21 +0800, zihan xi wrote:
> On Tue, Aug 25, 2026 at 4:20 PM Tom Parkin <tparkin@xxxxxxxxxxx> wrote:
> >
> > On  Mon, Aug 17, 2026 at 16:53:17 +0000, Zihan Xi wrote:
> > > Hi Linux kernel maintainers,
> > >
> > > We found and validated a issue in net/l2tp/l2tp_netlink.c. The bug is reachable by a
> > > non-root user via user and net namespace.
> > > Here, that reachability statement refers to the finite state-commit trigger;
> > > the OOM transcript below is a separate root initramfs leak-mode run (UID 0,
> > > PID 1) used to make the leak and panic deterministic.
> > > We've tested it, and it should not affect any other functionality.
> > > Regression coverage includes the root namespace and an unprivileged user/net
> > > namespace, with notification-queue pressure and successful ACK paths for all
> > > three commands; no broader regression suite was run.
> > > The finite fixed-kernel runs returned ACK success for all three commands in
> > > both namespaces. The leak run returned ENOBUFS and reached OOM after 41984
> > > hidden Ethernet sessions.
> >
> > I think the underlying point about allowing l2tp_tunnel_notify and
> > l2tp_session_notify to impact the return from l2tp_nl_cmd_tunnel_create
> > and l2tp_nl_cmd_tunnel_create is not unreasonable.
> >
> > IMO it seems relatively silly to allow the notification to cause an error
> > response to be indicated to userspace for the create command when in
> > fact the instance creation was otherwise successful.
> >
> > That said, I think it would be worth clarifying the behaviour around
> > the "hidden" tunnel and session.  From my reading of the code, at the
> > point that the nl notification function is called in both tunnel and
> > session instantiation, the kernel has already performed checks on
> > input arguments, allocated the instance, and registered it.  Even if
> > the l2tp code then returns an error to userspace, the instance is
> > present in the kernel's tracking structures.  I would expect that if
> > one then listed tunnel and session instances the new instance would
> > show up.
> >
> Hi Tom,
> 
> Thanks for taking the time to review this.
> 
> > That being the case, it's not accurate IMO to say that the tunnel or
> > session instance is leaked, and the fact that you can cause OOM by
> > continuing to allocate new tunnel and session instances with new IDs
> > isn't surprising.
> 
> Agreed. “Leaked” was too broad a term here. The tunnel and session are
> already committed and remain present in the kernel after the notification
> returns -ENOBUFS.
> 
> The PoC does not use `ip l2tp show`; it sends generic-netlink GET requests
> for the newly created objects. On the unpatched kernel, the relevant output
> was:
> 
>     tunnel_create returned ENOBUFS at tunnel_id=1040
>     hidden tunnel is live: tunnel_id=1040 ...
>     session_create returned ENOBUFS at session_id=1000032 ...
>     hidden session is live: tunnel=1040 session=1000032 ...
>     session_modify returned ENOBUFS as expected
>     post-modify session state: recv_seq=1 send_seq=1 lns_mode=1
> 
> So the OOM result comes from continuing to create new live Ethernet sessions
> after userspace has received failure responses, rather than from an object
> being lost in the kernel's tracking structures.
> 
> The more precise description is therefore that a best-effort notification
> error overwrites the result of an already committed operation. This can
> cause userspace to retry and accumulate live L2TP objects.
> 
> The v2 wording was updated accordingly, and v2 has since been applied to
> netdev/net.git.
> 
> - v2 Link: https://lore.kernel.org/all/cover.1787247008.git.zihanx@xxxxxxxxxx/

Hi Zihan,

Sorry my review was late (I saw the v2 was applied right after I'd
responded to the v1!) -- but thank you for taking the time to expand
on the above.

Your explanation makes sense and aligns with what I was expecting, and
I'm glad the underlying issue is fixed which I agree is an improvement
on the previous state :-)

All the best,
Tom
-- 
Tom Parkin
Katalix Systems Ltd
https://katalix.com
Catalysts for your Embedded Linux software development

Attachment: signature.asc
Description: PGP signature