Re: [PATCH net] l2tp: fix use-after-free in l2tp_udp_encap_recv()

[Linkui Xiao]

On 2026/10/31 06:40, netdev-bot+sashiko@xxxxxxxxxx wrote:
> - [Medium] The commit message describes a use-after-free that no code
>   path in the current tree can trigger.
>
> Can the backlog path described here actually be reached?

Agreed -- it cannot.  pppol2tp_backlog_recv() is the only
sk_backlog_rcv callback that calls l2tp_udp_encap_recv(), and nothing
ever queues skbs on a PPPoL2TP socket's backlog: sk_add_backlog() is
only reached from __sk_receive_skb(), tcp, sctp, tipc and x25, and the
__sk_receive_skb() callers (pppoe, pptp, l2tp_ip, l2tp_ip6, phonet and
vmci) all target their own socket families, never a PF_PPPOX /
PX_PROTO_OL2TP socket.  pppol2tp_recv() delivers payload with
sock_queue_rcv_skb(), which does not use the backlog either.

> The live entry point is the UDP encap hook set up in
> l2tp_tunnel_register():
>
> 	.encap_rcv = l2tp_udp_encap_recv,

Right.  That hook is only called from udp_queue_rcv_one_skb() and
udpv6_queue_rcv_one_skb(), under the rcu_read_lock() taken in
ip_local_deliver_finish() and ip6_input(), so the receive path always
holds an RCU read-side reference.  Combined with the kfree_rcu() in
l2tp_tunnel_free(), the tunnel memory cannot go away while it is being
dereferenced, and a strict grace period still cannot end inside a
read-side section, so CONFIG_RCU_STRICT_GRACE_PERIOD cannot expose it
either.  The description of __release_sock() in my commit message was
accurate in itself, but the path is unreachable, so presenting this as
a reachable use-after-free was wrong.

> The reordering itself looks correct as hardening. Would it be more
> accurate to describe it that way, and to drop or justify the
> Cc: stable tag, rather than present it as a reachable use-after-free?

Agreed.  I will respin with the reorder described as hardening: it
removes the dependency on every caller of l2tp_udp_encap_recv()
holding an RCU read-side reference, which holds today but is fragile.
The subject changes accordingly and I drop the backport tags, since
there is no reachable bug to backport.

> Also, pppol2tp_backlog_recv() appears to be unreachable, and it passes a
> PPPoX sk to a function that expects a UDP tunnel socket. Could it be
> removed as part of a respin?

Agreed that it is dead code.  Since it is a cleanup rather than a fix,
I would rather not mix it into this series.  Its removal is already
covered by Qingfang Deng's in-flight net-next series ("l2tp: ppp:
remove unused backlog callback"), so I will leave it to that thread
instead of posting a duplicate.

pw-bot: cr