[PATCH net v2 1/1] ip: orphan prefetched skbs before multicast forwarding

[PATCH net v2 1/1] ip: orphan prefetched skbs before multicast forwarding

[Zhiling Zou]

IPv4 and IPv6 input preserve an skb->sk association installed by
bpf_sk_assign() so that local delivery can use the selected socket under
RCU. IPv6 can also get such an association from udp_v6_early_demux(),
which runs in ip6_rcv_finish_core(), after ip6_rcv_core().

The reproduced UDPv6 packet has a multicast IP destination but a unicast
destination MAC address. It is therefore classified as PACKET_HOST and
passes the UDP early-demux check.

When the multicast packet is not locally deliverable, IPv6 hands the
original skb to ip6_mr_input(). IPv4's ip_mr_input() similarly keeps the
original skb when local delivery is not needed. Either path can put the
skb on an unresolved multicast route queue or forward it after the
receive-side RCU section ends.

After the prefetched socket is destroyed, a later skb free invokes
sock_pfree() and dereferences the stale skb->sk. Orphan the skb before
each non-local multicast forwarding path. Local delivery retains the
original skb; the existing skb_clone() calls provide multicast forwarding
with a socket-free clone.

Fixes: cf7fbe660f2d ("bpf: Add socket assign support")
Fixes: 08842c43d016 ("udp: no longer touch sk->sk_refcnt in early demux")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
---
changes in v2:
- Cover the IPv4 multicast forwarding path.
- Correct the IPv6 early-demux ordering and document the unicast-MAC
  PACKET_HOST condition used by the reproducer.
- Add the BPF socket-assignment and UDP early-demux Fixes tags.
- v1 Link: https://lore.kernel.org/all/02db4590d0161e31a789dcdfa8d1be1a3212ec2e.1785724784.git.zhilinz@nebusec.ai/

 net/ipv4/ipmr.c      | 3 +++
 net/ipv6/ip6_input.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 1d9a4ac14fcef..e5f2b1c6150d2 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2213,6 +2213,9 @@ int ip_mr_input(struct sk_buff *skb)
 	if (IPCB(skb)->flags & IPSKB_FORWARDED)
 		goto dont_forward;
 
+	if (!local)
+		skb_orphan(skb);
+
 	mrt = ipmr_rt_fib_lookup(net, skb);
 	if (IS_ERR(mrt)) {
 		kfree_skb(skb);
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 8972863c93ee5..d332ec60f9150 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -622,6 +622,7 @@ int ip6_mc_input(struct sk_buff *skb)
 		if (deliver) {
 			skb2 = skb_clone(skb, GFP_ATOMIC);
 		} else {
+			skb_orphan(skb);
 			skb2 = skb;
 			skb = NULL;
 		}
-- 
2.43.0

Re: [PATCH net v2 1/1] ip: orphan prefetched skbs before multicast forwarding

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free (UAF) of `struct sock` on bridging and TC redirect paths due to non-refcounted sockets.
--

--- Patch [1]: [PATCH net v2 1/1] ip: orphan prefetched skbs before multicast forwarding ---
commit 8d7ee5d5781c74533a99f0b7cf5fb3db935e55f5
Author: Zhiling Zou <zhilinz@nebusec.ai>

ip: orphan prefetched skbs before multicast forwarding

This patch adds skb_orphan() calls before multicast forwarding in both
IPv4 and IPv6 to clear non-refcounted sockets assigned by bpf_sk_assign().
This prevents a use-after-free when the socket is destroyed before the
forwarded skb is freed.

Link: https://lore.kernel.org/all/02db4590d0161e31a789dcdfa8d1be1a3212ec2e.1785724784.git.zhilinz@nebusec.ai/

> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index 1d9a4ac14fcef..e5f2b1c6150d2 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -2213,6 +2213,9 @@ int ip_mr_input(struct sk_buff *skb)
>  	if (IPCB(skb)->flags & IPSKB_FORWARDED)
>  		goto dont_forward;
>  
> +	if (!local)
> +		skb_orphan(skb);
> +

[Severity: High]
This isn't a bug introduced by this patch, but since you are fixing the
use-after-free for multicast forwarding, do we have the exact same problem
in the TC redirect and bridge forwarding paths?

If a packet receives a non-refcounted socket via bpf_sk_assign() at TC
ingress, and the eBPF program returns TC_ACT_REDIRECT, it is redirected
to an egress qdisc:

net/core/filter.c:skb_do_redirect() {
    ...
    return __bpf_redirect(skb, dev, flags);
}

Alternatively, if it returns TC_ACT_OK and is processed by a bridge port,
it gets forwarded to an egress qdisc:

net/bridge/br_forward.c:br_forward() {
    ...
    dev_queue_xmit(skb);
}

In both paths, skb_orphan() is not called, so the SKB retains the socket
pointer while sitting asynchronously in the qdisc. If the socket's RCU grace
period expires while queued, wouldn't a later free cause the exact same
use-after-free when the destructor dereferences the freed skb->sk?

net/core/sock.c:sock_pfree() {
    struct sock *sk = skb->sk;

    if (!sk_is_refcounted(sk))
        return;
    ...
}

Should skb_orphan() be called before enqueuing to egress qdiscs in these
paths as well?

>  	mrt = ipmr_rt_fib_lookup(net, skb);
>  	if (IS_ERR(mrt)) {
>  		kfree_skb(skb);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/f968f497251dedbd1263c322a7c4e3e1a727161d.1786023177.git.zhilinz@nebusec.ai?part=1