netdev - Re: [PATCH net] netfilter: nf_reject: don't leak dst refcount for loopback packets

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aKXKpE35H7KBzdBa@calendula>
Date: Wed, 20 Aug 2025 15:16:20 +0200
From: Pablo Neira Ayuso <pablo@...filter.org>
To: Florian Westphal <fw@...len.de>
Cc: netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, netfilter-devel@...r.kernel.org
Subject: Re: [PATCH net] netfilter: nf_reject: don't leak dst refcount for
 loopback packets

Hi Florian,

On Wed, Aug 20, 2025 at 02:37:07PM +0200, Florian Westphal wrote:
> recent patches to add a WARN() when replacing skb dst entry found an
> old bug:
> 
> WARNING: include/linux/skbuff.h:1165 skb_dst_check_unset include/linux/skbuff.h:1164 [inline]
> WARNING: include/linux/skbuff.h:1165 skb_dst_set include/linux/skbuff.h:1210 [inline]
> WARNING: include/linux/skbuff.h:1165 nf_reject_fill_skb_dst+0x2a4/0x330 net/ipv4/netfilter/nf_reject_ipv4.c:234
> [..]
> Call Trace:
>  nf_send_unreach+0x17b/0x6e0 net/ipv4/netfilter/nf_reject_ipv4.c:325
>  nft_reject_inet_eval+0x4bc/0x690 net/netfilter/nft_reject_inet.c:27
>  expr_call_ops_eval net/netfilter/nf_tables_core.c:237 [inline]
>  ..
> 
> This is because blamed commit forgot about loopback packets.
> Such packets already have a dst_entry attached, even at PRE_ROUTING stage.
> 
> Instead of checking hook just check if the skb already has a route
> attached to it.

Quick question: does inconditional route lookup work for br_netfilter?

> Fixes: f53b9b0bdc59 ("netfilter: introduce support for reject at prerouting stage")
> Signed-off-by: Florian Westphal <fw@...len.de>
> ---
>  Sending this instead of a pull request. the only other two
>  candidates for -net are still under review.
> 
>  Let me know if you prefer a normal pull request even in this case.
>  Thanks!
> 
>  net/ipv4/netfilter/nf_reject_ipv4.c | 6 ++----
>  net/ipv6/netfilter/nf_reject_ipv6.c | 5 ++---
>  2 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
> index 87fd945a0d27..0d3cb2ba6fc8 100644
> --- a/net/ipv4/netfilter/nf_reject_ipv4.c
> +++ b/net/ipv4/netfilter/nf_reject_ipv4.c
> @@ -247,8 +247,7 @@ void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,
>  	if (!oth)
>  		return;
>  
> -	if ((hook == NF_INET_PRE_ROUTING || hook == NF_INET_INGRESS) &&
> -	    nf_reject_fill_skb_dst(oldskb) < 0)
> +	if (!skb_dst(oldskb) && nf_reject_fill_skb_dst(oldskb) < 0)
>  		return;
>  
>  	if (skb_rtable(oldskb)->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))
> @@ -321,8 +320,7 @@ void nf_send_unreach(struct sk_buff *skb_in, int code, int hook)
>  	if (iph->frag_off & htons(IP_OFFSET))
>  		return;
>  
> -	if ((hook == NF_INET_PRE_ROUTING || hook == NF_INET_INGRESS) &&
> -	    nf_reject_fill_skb_dst(skb_in) < 0)
> +	if (!skb_dst(skb_in) && nf_reject_fill_skb_dst(skb_in) < 0)
>  		return;
>  
>  	if (skb_csum_unnecessary(skb_in) ||
> diff --git a/net/ipv6/netfilter/nf_reject_ipv6.c b/net/ipv6/netfilter/nf_reject_ipv6.c
> index 838295fa32e3..cb2d38e80de9 100644
> --- a/net/ipv6/netfilter/nf_reject_ipv6.c
> +++ b/net/ipv6/netfilter/nf_reject_ipv6.c
> @@ -293,7 +293,7 @@ void nf_send_reset6(struct net *net, struct sock *sk, struct sk_buff *oldskb,
>  	fl6.fl6_sport = otcph->dest;
>  	fl6.fl6_dport = otcph->source;
>  
> -	if (hook == NF_INET_PRE_ROUTING || hook == NF_INET_INGRESS) {
> +	if (!skb_dst(oldskb)) {
>  		nf_ip6_route(net, &dst, flowi6_to_flowi(&fl6), false);
>  		if (!dst)
>  			return;
> @@ -397,8 +397,7 @@ void nf_send_unreach6(struct net *net, struct sk_buff *skb_in,
>  	if (hooknum == NF_INET_LOCAL_OUT && skb_in->dev == NULL)
>  		skb_in->dev = net->loopback_dev;
>  
> -	if ((hooknum == NF_INET_PRE_ROUTING || hooknum == NF_INET_INGRESS) &&
> -	    nf_reject6_fill_skb_dst(skb_in) < 0)
> +	if (!skb_dst(skb_in) && nf_reject6_fill_skb_dst(skb_in) < 0)
>  		return;
>  
>  	icmpv6_send(skb_in, ICMPV6_DEST_UNREACH, code, 0);
> -- 
> 2.49.1
>