netdev - Re: [net-next,v6,04/11] net: tunnel: allow noref dsts in udp_tunnel{,6}_dst

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: <20260122022106.2528669-1-kuba@kernel.org>
Date: Wed, 21 Jan 2026 18:21:06 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: mmietus97@...oo.com
Cc: Jakub Kicinski <kuba@...nel.org>,
	netdev@...r.kernel.org,
	Jason@...c4.com,
	sd@...asysnail.net
Subject: Re: [net-next,v6,04/11] net: tunnel: allow noref dsts in udp_tunnel{,6}_dst_lookup

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: tunnel: allow noref dsts in udp_tunnel{,6}_dst_lookup

This commit updates udp_tunnel{,6}_dst_lookup to return noref dsts when
possible. All callers are updated to properly use the new noref argument
and conditionally release the dst based on whether they own a reference.

> diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c
> index 92ee4a36f86f..1aa3d5d74a84 100644
> --- a/drivers/net/bareudp.c
> +++ b/drivers/net/bareudp.c
> @@ -315,6 +315,7 @@ static int bareudp_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  	int min_headroom;
>  	__u8 tos, ttl;
>  	__be32 saddr;
> +	bool noref;
>  	int err;

[ ... ]

> @@ -329,7 +330,8 @@ static int bareudp_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  	rt = udp_tunnel_dst_lookup(skb, dev, bareudp->net, 0, &saddr, &info->key,
>  				   sport, bareudp->port, key->tos,
>  				   use_cache ?
> -				   (struct dst_cache *)&info->dst_cache : NULL);
> +				   (struct dst_cache *)&info->dst_cache : NULL,
> +				   &noref);

[ ... ]

> @@ -364,7 +366,8 @@ static int bareudp_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  			    !net_eq(bareudp->net, dev_net(bareudp->dev)),
>  			    !test_bit(IP_TUNNEL_CSUM_BIT, info->key.tun_flags),
>  			    0);
> -	ip_rt_put(rt);
> +	if (!noref)
> +		ip_rt_put(rt);
>  	return 0;
>
>  free_dst:
>  	dst_release(&rt->dst);
            ^^^^^^^^^^^^^^^^^^^^

The success path above correctly checks if (!noref) before calling
ip_rt_put(), but the free_dst error path still unconditionally calls
dst_release(). When the dst comes from the cache (noref == true) and an
error occurs in the operations between the lookup and xmit, this would
decrement a refcount we do not own.

For comparison, bareudp6_xmit_skb() in this same patch correctly wraps
both its success and error paths with if (!noref) checks:

> @@ -434,11 +439,13 @@ static int bareudp6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
>  			     !test_bit(IP_TUNNEL_CSUM_BIT,
>  				       info->key.tun_flags),
>  			     0);
> -	dst_release(dst);
> +	if (!noref)
> +		dst_release(dst);
>  	return 0;
>
>  free_dst:
> -	dst_release(dst);
> +	if (!noref)
> +		dst_release(dst);
>  	return err;

Should the free_dst label in bareudp_xmit_skb() also be wrapped with
if (!noref)?