Re: [PATCH net] ipv4: Fix device use-after-free in ip_skb_dst_mtu()

[netdev-bot+sashiko@kernel.org]

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

Critical: 0 · High: 1 · 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] This fix is incomplete.

--

ipv4: Fix device use-after-free in ip_skb_dst_mtu()

This patch wraps the dev->mtu read in ip_skb_dst_mtu() in
rcu_read_lock()/rcu_read_unlock() and switches it to dst_dev_rcu(). It
targets a KASAN-reported net_device use-after-free on the multicast and
broadcast output path, where ip_mc_output() reaches ip_finish_output()
without an RCU read lock held.

> diff --git a/include/net/ip.h b/include/net/ip.h
> index 6f602df72ee62..14d6f77f3bb89 100644
> --- a/include/net/ip.h
> +++ b/include/net/ip.h
> @@ -543,7 +543,9 @@ static inline unsigned int ip_skb_dst_mtu(struct sock *sk,
>  		return ip_dst_mtu_maybe_forward(dst, forwarding);
>  	}
>  
> -	mtu = min(READ_ONCE(dst_dev(dst)->mtu), IP_MAX_MTU);
> +	rcu_read_lock();
> +	mtu = min(READ_ONCE(dst_dev_rcu(dst)->mtu), IP_MAX_MTU);
> +	rcu_read_unlock();

[Severity: High]
Is this fix complete? The commit message names the root cause as
ip_mc_output() reaching ip_finish_output() with no RCU read lock held.
That is still the case after this patch, so the other device reads on the
same path are still unprotected.

ip_mc_output() in net/ipv4/ip_output.c does a plain load of the device.
It takes no RCU and no reference, then stores the pointer in skb->dev and
passes it as outdev:

net/ipv4/ip_output.c:ip_mc_output() {
	struct rtable *rt = skb_rtable(skb);
	struct net_device *dev = rt->dst.dev;
	...
	skb->dev = dev;
	...
	return NF_HOOK_COND(NFPROTO_IPV4, NF_INET_POST_ROUTING,
			    net, sk, skb, NULL, skb->dev,
			    ip_finish_output,
			    !(IPCB(skb)->flags & IPSKB_REROUTED));
}

Right after ip_skb_dst_mtu() returns, __ip_finish_output()->
ip_finish_output2() reads the device again, before its own
rcu_read_lock():

net/ipv4/ip_output.c:ip_finish_output2() {
	struct net_device *dev = dst_dev(dst);
	unsigned int hh_len = LL_RESERVED_SPACE(dev);
	...
	if (unlikely(skb_headroom(skb) < hh_len && dev->header_ops)) {
	...
	rcu_read_lock();
	neigh = ip_neigh_for_gw(rt, skb, &is_v6gw);
	...
}

Can the same race described in the commit message now hit freed memory in
ip_finish_output2() instead of in ip_skb_dst_mtu()?

The same seems to apply to ip_do_fragment(). It calls ip_skb_dst_mtu() and
then reads the device outside RCU when reached from ip_mc_output() via
ip_fragment():

	ll_rs = LL_RESERVED_SPACE(rt->dst.dev);

There are more users of the pointer that ip_mc_output() saved in skb->dev.
On the GSO path, ip_finish_output_gso() calls netif_skb_features(skb).
The multicast and broadcast loopback clones go through
ip_mc_finish_output()->dev_loopback_xmit().

No caller on the UDP path from the KASAN trace provides RCU either.
ip_send_skb() calls ip_local_out() directly:

net/ipv4/ip_output.c:ip_send_skb() {
	err = ip_local_out(net, skb->sk, skb);
	...
}

ip6_send_skb() wraps ip6_local_out() in rcu_read_lock().

The unicast path already has the full-coverage pattern. It was added to
ip_output() by commit 1dbf1d590d10a6 ("net: Add locking to protect
skb->dev access in ip_output"):

	rcu_read_lock();
	dev = skb_dst_dev_rcu(skb);
	skb->dev = dev;
	...
	ret_val = NF_HOOK_COND(NFPROTO_IPV4, NF_INET_POST_ROUTING,
				net, sk, skb, indev, dev,
				ip_finish_output,
				!(IPCB(skb)->flags & IPSKB_REROUTED));
	rcu_read_unlock();

Would it be better to apply the same outer RCU section in ip_mc_output(),
and maybe also in ip_send_skb() to match ip6_send_skb()? That would cover
the whole ip_finish_output() chain instead of only the dev->mtu read here.

>  	return mtu - lwtunnel_headroom(dst->lwtstate, mtu);
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927071051.3693368-1-nicoyip.dev%40gmail.com