[PATCH] ipv6: Protect the cork MTU read with RCU

[PATCH] ipv6: Protect the cork MTU read with RCU

[Cen Zhang]

ip6_setup_cork() must keep the metrics image alive until the MTU read
completes. Its destination reference protects the rt6_info allocation,
but dst6_mtu() reads the metrics through a raw pointer outside RCU.

A route with dynamically allocated metrics can share them with a
Redirect-created RTF_CACHE exception. This also happens when RTAX_MTU
is zero, for example on a route with an RTT metric. A UDP send using
that exception can overlap a Packet Too Big update for the same flow
and deletion of the owning FIB route in the following order:

1. udpv6_sendmsg() calls ip6_make_skb() and ip6_setup_cork(). The MTU
   lookup loads the old metrics pointer in dst_metric_raw().
2. icmpv6_err() calls ip6_update_pmtu(), which finds the same exception.
   rt6_do_update_pmtu() calls dst_metric_set(); COW installs private
   metrics and drops the destination's old metrics reference.
3. After Packet Too Big processing returns, fib6_del_route() purges the
   exception. ip6_dst_ifdown() clears rt->from and releases its FIB
   reference while the send still holds the destination.
4. After the remaining owners drain, fib6_info_destroy_rcu() drops the
   last reference to the old metrics and frees them.
5. The send reads RTAX_MTU through the saved pointer to freed memory.

The MTU helper's existing RCU section starts after the raw metrics
read, so it cannot prevent this use-after-free. Enclose MTU selection
in an RCU read-side critical section in ip6_setup_cork(). Since COW
replaces the metrics pointer before route deletion, the existing FIB
and destination RCU callbacks must wait for this read before releasing
the remaining old metrics owners.

KASAN report as below:

    BUG: KASAN: slab-use-after-free in ip6_mtu+0x6c8/0x790
    Read of size 4 at addr ffff888101348c84 by task pmbd-send/551

    CPU: 2 UID: 0 PID: 551 Comm: pmbd-send Not tainted 7.2.0-rc5-pmb-bt-functional-v1+ #1 PREEMPT(lazy)
    Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
    Call Trace:
     <TASK>
     dump_stack_lvl+0x93/0xd0
     print_report+0xce/0x630
     ? ip6_mtu+0x6c8/0x790
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __virt_addr_valid+0x20d/0x410
     ? ip6_mtu+0x6c8/0x790
     kasan_report+0xe0/0x110
     ? ip6_mtu+0x6c8/0x790
     ip6_mtu+0x6c8/0x790
     ? __pfx_ip6_mtu+0x10/0x10
     ip6_setup_cork+0xf50/0x1470
     ip6_make_skb+0x228/0x390
     ? __pfx_ip_generic_getfrag+0x10/0x10
     ? __pfx_ip6_make_skb+0x10/0x10
     ? find_held_lock+0x2b/0x80
     ? ip6_dst_hoplimit+0xd1/0x450
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? lock_release+0xc8/0x280
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? udpv6_sendmsg+0x1f5e/0x2a00
     udpv6_sendmsg+0x1f5e/0x2a00
     ? __pfx_udpv6_sendmsg+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __lock_acquire+0x466/0x2260
     ? __lock_acquire+0x466/0x2260
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? sock_has_perm+0x278/0x320
     ? ip6_datagram_release_cb+0x26d/0x510
     ? __pfx_udpv6_sendmsg+0x10/0x10
     ? inet6_sendmsg+0xff/0x140
     inet6_sendmsg+0xff/0x140
     __sys_sendto+0x320/0x470
     ? __pfx___sys_sendto+0x10/0x10
     ? __pfx___do_sys_prctl+0x10/0x10
     __x64_sys_sendto+0xe5/0x1c0
     ? lockdep_hardirqs_on_prepare+0xea/0x1a0
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? trace_hardirqs_on+0x18/0x160
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f
    RIP: 0033:0x7fbdea3b3687
    Code: 48 89 fa 4c 89 df e8 58 b3 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5b> c3 0f 1f 80 00 00 00 00 83 e2 39 83 fa 08 75 de e8 23 ff ff ff
    RSP: 002b:00007ffd43d66d40 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
    RAX: ffffffffffffffda RBX: 00007fbdea31f780 RCX: 00007fbdea3b3687
    RDX: 0000000000000017 RSI: 00007fbdea10bbd0 RDI: 0000000000000003
    RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
    R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
    R13: 0000000000000000 R14: 00000000006fe390 R15: 0000000000a83590
     </TASK>

    Allocated by task 542:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     __kasan_kmalloc+0xaa/0xb0
     __kmalloc_cache_noprof+0x251/0x630
     ip_fib_metrics_init+0xc1/0x6b0
     ip6_route_info_create+0x1a0/0x8c0
     ip6_route_add.part.0+0x29/0x150
     inet6_rtm_newroute+0x16a/0x180
     rtnetlink_rcv_msg+0x7b9/0xce0
     netlink_rcv_skb+0x133/0x390
     netlink_unicast+0x504/0x840
     netlink_sendmsg+0x7f7/0xcf0
     ____sys_sendmsg+0x88b/0x9d0
     ___sys_sendmsg+0x125/0x1d0
     __sys_sendmsg+0x13e/0x1e0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    Freed by task 0:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     kasan_save_free_info+0x3b/0x60
     __kasan_slab_free+0x5f/0x80
     kfree+0x236/0x5a0
     fib6_info_destroy_rcu+0x1fa/0x240
     rcu_core+0x661/0x1d10
     handle_softirqs+0x201/0x930
     __irq_exit_rcu+0x110/0x1e0
     irq_exit_rcu+0xe/0x20
     sysvec_apic_timer_interrupt+0x6c/0x80
     asm_sysvec_apic_timer_interrupt+0x1a/0x20

    The buggy address belongs to the object at ffff888101348c80
     which belongs to the cache kmalloc-96 of size 96
    The buggy address is located 4 bytes inside of
     freed 96-byte region [ffff888101348c80, ffff888101348ce0)

    The buggy address belongs to the physical page:
    page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x101348
    flags: 0x200000000000000(node=0|zone=2)
    page_type: f5(slab)
    raw: 0200000000000000 ffff888100042280 dead000000000100 dead000000000122
    raw: 0000000000000000 0000000000200020 00000000f5000000 0000000000000000
    page dumped because: kasan: bad access detected

    Memory state around the buggy address:
     ffff888101348b80: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
     ffff888101348c00: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
    >ffff888101348c80: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
                       ^
     ffff888101348d00: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
     ffff888101348d80: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
    ==================================================================

Fixes: f5b51fe804ec ("ipv6: route: purge exception on removal")
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 5509650589915cac54ea7767f0cb237c6c1d71e5..0440acc3dec0b0bdb8fe484810a9c84f1af67881 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1421,12 +1421,14 @@ static int ip6_setup_cork(struct sock *sk, struct inet_cork_full *cork,
 	v6_cork->hop_limit = ipc6->hlimit;
 	v6_cork->tclass = ipc6->tclass;
 	v6_cork->dontfrag = ipc6->dontfrag;
+	rcu_read_lock();
 	if (rt->dst.flags & DST_XFRM_TUNNEL)
 		mtu = READ_ONCE(np->pmtudisc) >= IPV6_PMTUDISC_PROBE ?
 		      READ_ONCE(rt->dst.dev->mtu) : dst6_mtu(&rt->dst);
 	else
 		mtu = READ_ONCE(np->pmtudisc) >= IPV6_PMTUDISC_PROBE ?
 			READ_ONCE(rt->dst.dev->mtu) : dst6_mtu(xfrm_dst_path(&rt->dst));
+	rcu_read_unlock();
 
 	frag_size = READ_ONCE(np->frag_size);
 	if (frag_size && frag_size < mtu)

Re: [PATCH] ipv6: Protect the cork MTU read with RCU

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH] ipv6: Protect the cork MTU read with RCU

[netdev-bot+sashiko]

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

Critical: 0 · High: 2 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete.
- [High] The new rcu_read_lock() in ip6_setup_cork() only helps if the
  final free of the old shared metrics is deferred by a grace period…

--

ipv6: Protect the cork MTU read with RCU

This patch wraps the MTU selection in ip6_setup_cork() in an RCU read-side
section. The goal is to keep a metrics image that is shared with a
Redirect-created exception alive until dst6_mtu() has finished reading
it. The target is a KASAN use-after-free in ip6_mtu() on the UDP send
path, seen when a Packet Too Big update and route deletion run at the
same time as the send.

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 5509650589915..0440acc3dec0b 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1421,12 +1421,14 @@ static int ip6_setup_cork(struct sock *sk, struct inet_cork_full *cork,
>  	v6_cork->hop_limit = ipc6->hlimit;
>  	v6_cork->tclass = ipc6->tclass;
>  	v6_cork->dontfrag = ipc6->dontfrag;
> +	rcu_read_lock();
>  	if (rt->dst.flags & DST_XFRM_TUNNEL)
>  		mtu = READ_ONCE(np->pmtudisc) >= IPV6_PMTUDISC_PROBE ?
>  		      READ_ONCE(rt->dst.dev->mtu) : dst6_mtu(&rt->dst);
>  	else
>  		mtu = READ_ONCE(np->pmtudisc) >= IPV6_PMTUDISC_PROBE ?
>  			READ_ONCE(rt->dst.dev->mtu) : dst6_mtu(xfrm_dst_path(&rt->dst));
> +	rcu_read_unlock();

[Severity: High]
Does this fully close the window the commit message describes? The
message names the defect as "The MTU helper's existing RCU section
starts after the raw metrics read". The helper itself is not changed:

include/net/ip6_route.h:ip6_dst_mtu_maybe_forward() {
    ...
	if (!forwarding || dst_metric_locked(dst, RTAX_MTU)) {
		mtu = dst_metric_raw(dst, RTAX_MTU);
		if (mtu)
			goto out;
	}

	mtu = IPV6_MIN_MTU;
	rcu_read_lock();
    ...
}

ip6_dst_hoplimit() in net/ipv6/ip6_output.c has the same shape:

int ip6_dst_hoplimit(struct dst_entry *dst)
{
	int hoplimit = dst_metric_raw(dst, RTAX_HOPLIMIT);

	rcu_read_lock();
    ...

udpv6_sendmsg() calls it on the same send path and the same exception
dst. The call happens before ip6_make_skb() and without RCU held:

	if (ipc6.hlimit < 0)
		ipc6.hlimit = ip6_sk_dst_hoplimit(np, fl6, dst);

np->hop_limit defaults to -1, so this reaches ip6_dst_hoplimit(). Suppose
the PTB COW and route deletion from the commit message happen while the
sender sits between dst_metric_raw() and the read, for example while
preempted outside any RCU section.

Can the sender then read RTAX_HOPLIMIT, not RTAX_MTU, from the freed
kmalloc-96 metrics? Raw, ping and l2tp_ip6 senders call the same helper.

Other dst6_mtu() callers that run without RCU look exposed too. One
example is tcp_v6_mtu_reduced() in net/ipv6/tcp_ipv6.c:

	dmtu = dst6_mtu(dst);

It runs from tcp_release_cb right after inet6_csk_update_pmtu() may have
COWed the metrics.

Would it be better to widen the RCU section inside
ip6_dst_mtu_maybe_forward() and ip6_dst_hoplimit() so it covers the
dst_metric_raw() read, instead of adding it at single call sites?

[Severity: High]
The commit message says:

    Since COW replaces the metrics pointer before route deletion, the
    existing FIB and destination RCU callbacks must wait for this read
    before releasing the remaining old metrics owners.

Does anything enforce that ordering? __ip6_rt_update_pmtu() in
net/ipv6/route.c COWs the metrics on any RTF_CACHE dst and never checks
rt->from:

	if (!rt6_cache_allowed_for_pmtu(rt6)) {
		rt6_do_update_pmtu(rt6, mtu);

So the COW can run on an exception that route deletion has already
purged. One way is inet6_csk_update_pmtu() or skb_dst_update_pmtu()
acting on a dst that is still held. ip6_dst_ifdown() clears rt->from and
releases the fib6_info, but dst->_metrics still points at the shared
image.

First ordering: the route is deleted, and fib6_info_destroy_rcu() drops
the FIB's metrics reference. The exception dst now holds the last
reference. A COW then frees the old image synchronously in
dst_cow_metrics_generic():

	} else if (prev & DST_METRICS_REFCOUNTED) {
		if (refcount_dec_and_test(&old_p->refcnt))
			kfree(old_p);

Can a sender inside the new rcu_read_lock() section in ip6_setup_cork()
load old_p just before this and then read freed memory? A read-side RCU
section cannot delay a plain kfree().

Second ordering: route deletion queues call_rcu_hurry(&f6i->rcu,
fib6_info_destroy_rcu) before the sender enters rcu_read_lock(). The
sender then loads the old pointer, which dst->_metrics still publishes.
A later COW drops the dst's reference from 2 to 1.

The grace period started before the sender's read-side section, so it
does not wait for the sender. Could
fib6_info_destroy_rcu()->ip_fib_metrics_put() then kfree() the metrics
while ip6_mtu() is still reading them?

dst->_metrics is not an RCU-published pointer. ip_fib_metrics_put(),
ip_dst_metrics_put() and dst_cow_metrics_generic() all free immediately.

For the reader-side section to be enough, would the metrics need
RCU-deferred freeing, such as kfree_rcu() in those put and COW paths? The
alternative would be to enforce the COW-before-purge ordering.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/pm-ip-core-objects-candidate-0007-v2-46f5bb8aa2cc9824a047%40gmail.com