netdev - Re: [net-next,v5,10/11] net: sit: convert ipip6_tunnel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260118003613.1042452-1-kuba@kernel.org>
Date: Sat, 17 Jan 2026 16:36:13 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: mmietus97@...oo.com
Cc: Jakub Kicinski <kuba@...nel.org>,
	netdev@...r.kernel.org,
	sd@...asysnail.net,
	Jason@...c4.com
Subject: Re: [net-next,v5,10/11] net: sit: convert ipip6_tunnel_xmit to use a noref dst

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
    net: sit: convert ipip6_tunnel_xmit to use a noref dst

This patch converts ipip6_tunnel_xmit to use dst_cache_get_ip4_rcu() and
dst_cache_steal_ip4() to avoid redundant refcount increments when using
the dst cache.

> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> index a0d699082747..e9183e502242 100644
> --- a/net/ipv6/sit.c
> +++ b/net/ipv6/sit.c
> @@ -933,14 +933,14 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
>  			   IPPROTO_IPV6, 0, dst, tiph->saddr, 0, 0,
>  			   sock_net_uid(tunnel->net, NULL));
>
> -	rt = dst_cache_get_ip4(&tunnel->dst_cache, &fl4.saddr);
> +	rt = dst_cache_get_ip4_rcu(&tunnel->dst_cache, &fl4.saddr);
>  	if (!rt) {
>  		rt = ip_route_output_flow(tunnel->net, &fl4, NULL);
>  		if (IS_ERR(rt)) {
>  			DEV_STATS_INC(dev, tx_carrier_errors);
>  			goto tx_error_icmp;
>  		}
> -		dst_cache_set_ip4(&tunnel->dst_cache, &rt->dst, fl4.saddr);
> +		dst_cache_steal_ip4(&tunnel->dst_cache, &rt->dst, fl4.saddr);
>  	}
>
>  	if (rt->rt_type != RTN_UNICAST && rt->rt_type != RTN_LOCAL) {
>  		ip_rt_put(rt);
>  		DEV_STATS_INC(dev, tx_carrier_errors);
>  		goto tx_error_icmp;
>  	}

When rt is obtained from dst_cache_get_ip4_rcu() (cache hit path), no
reference is taken on the dst. However, the ip_rt_put(rt) call here was
not removed. If the cache returns a valid rt but the rt_type check fails,
this will call dst_release() on a dst we do not own a reference to.

Could this cause a refcount underflow? The correct pattern (as seen in
ip_tunnel_xmit()) uses a noref boolean to track whether we own a reference,
and only calls ip_rt_put() when !noref.

[ ... ]