Re: [PATCH net v2] ipv6: raw: fix use-after-free in rawv6_send_hdrinc()

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · 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).

- [Medium] In VRF redirect mode, rawv6_send_hdrinc() (net/ipv6/raw.c) no
  longer charges per-interface IPv6 SNMP counters to the slave interface.

--

ipv6: raw: fix use-after-free in rawv6_send_hdrinc()

This patch reloads rt from skb_dst(skb) after l3mdev_ip6_out() returns in
rawv6_send_hdrinc(). The VRF output path can replace the skb dst and
release the original route, which leaves the cached pointer stale. It fixes
a KASAN slab-use-after-free reported in rawv6_sendmsg().

> diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
> index b965258cf9e5f..bdea825434ea7 100644
> --- a/net/ipv6/raw.c
> +++ b/net/ipv6/raw.c
> @@ -654,6 +654,9 @@ static int rawv6_send_hdrinc(struct sock *sk, struct msghdr *msg, int length,
>  	if (unlikely(!skb))
>  		return 0;
>  
> +	/* l3mdev_ip6_out() may have replaced the skb dst. */
> +	rt = dst_rt6_info(skb_dst(skb));
> +

[Severity: Medium]
Does this reload move the per-interface IPv6 counters off the slave device
in VRF redirect mode?

When the VRF device has a non-default qdisc and the packet is not
xfrm-transformed, vrf_ip6_out() calls vrf_ip6_out_redirect(), which swaps
in the VRF's private route:

drivers/net/vrf.c:vrf_ip6_out_redirect() {
    ...
	skb_dst_drop(skb);
	skb_dst_set(skb, &rt6->dst);
    ...
}

That vrf->rt6 comes from ip6_dst_alloc() in vrf_rt6_create().
rt6_info_init() zeroes everything after dst, and rt6i_idev is never set
afterwards. So after this reload, rt->rt6i_idev is NULL when the function
gets here:

	IP6_INC_STATS(net, rt->rt6i_idev, IPSTATS_MIB_OUTREQUESTS);
	...
		IP6_INC_STATS(net, rt->rt6i_idev, IPSTATS_MIB_OUTDISCARDS);

_DEVINC handles a NULL idev by skipping the per-device counter, so this
does not crash. Only the netns-wide counter gets bumped. The second
LOCAL_OUT pass in vrf_process_v6_outbound()->vrf_ip6_local_out() does no
OUTREQUESTS accounting either.

As a result, raw IPV6_HDRINCL packets sent on a VRF slave in this mode
would not show up in /proc/net/dev_snmp6/<slave> Ip6OutRequests or
Ip6OutDiscards, and no other device counts them.

The other IPv6 senders count OUTREQUESTS against the original route's idev
before the l3mdev diversion. ip6_xmit() does it before l3mdev_ip6_out().
__ip6_make_skb() does it before ip6_local_out(), and that covers raw
sockets without HDRINCL too.

Would it work to follow the ip6_xmit() pattern instead? That means taking
rcu_read_lock() before l3mdev_ip6_out() and counting OUTREQUESTS while the
skb still holds its reference on the original rt.

dst_release() frees through call_rcu_hurry(dst_destroy_rcu), and that
callback is also where rt6i_idev is dropped. So the original rt and its
idev should stay valid inside the read-side section. This would fix the
use-after-free without changing which device gets the stats.

>  	/* Acquire rcu_read_lock() in case we need to use rt->rt6i_idev
>  	 * in the error path. Since skb has been freed, the dst could
>  	 * have been queued for deletion.

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