Re: [PATCH net 06/10] netfilter: nf_reject: initialize IPCB at inet ingress

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Oct 20, 2026 at 03:37:35PM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)
> 
> 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] The new IPv6 initializer in nf_send_unreach6() sets
>   IP6CB(skb_in)->iif = skb_in->skb_iif unconditionally.
> 
> Pre-existing issues:
> - [Low] At NF_INET_INGRESS, the reject helpers do not check the L4
>   checksum of CHECKSUM_COMPLETE packets.
> 
> --
> 
> netfilter: nf_reject: initialize IPCB at inet ingress
> 
> The inet ingress hook runs before ip_rcv_core() and ip6_rcv_core(), so
> skb->cb can still hold stale data from an earlier layer. When called from
> NF_INET_INGRESS, nf_send_unreach() and nf_send_unreach6() now clear
> IPCB/IP6CB and restore iif. This stops stale IPv4 option offsets from
> overflowing the stack buffer in __ip_options_echo().
> 
> > diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
> > index 59ec465a9df9..7cb33af671e2 100644
> > --- a/net/ipv4/netfilter/nf_reject_ipv4.c
> > +++ b/net/ipv4/netfilter/nf_reject_ipv4.c
> > @@ -357,6 +357,11 @@ void nf_send_unreach(struct sk_buff *skb_in, int code, int hook)
> >  	if (!skb_valid_dst(skb_in) && nf_reject_fill_skb_dst(skb_in) < 0)
> >  		return;
> >  
> > +	if (hook == NF_INET_INGRESS) {
> > +		memset(IPCB(skb_in), 0, sizeof(*IPCB(skb_in)));
> > +		IPCB(skb_in)->iif = skb_in->skb_iif;
> > +	}
> > +
> >  	if (skb_csum_unnecessary(skb_in) ||
> >  	    !nf_reject_verify_csum(skb_in, dataoff, proto)) {
> >  		icmp_send(skb_in, ICMP_DEST_UNREACH, code, 0);
> 
> [Severity: Low]
> This is a pre-existing issue and was not introduced by this patch. Since
> this path now has NF_INET_INGRESS handling, though: is the L4 checksum of
> a CHECKSUM_COMPLETE packet ever checked at this hook?
> 
> Further down, nf_send_unreach() does:
> 
> 	if (nf_ip_checksum(skb_in, hook, dataoff, proto) == 0)
> 		icmp_send(skb_in, ICMP_DEST_UNREACH, code, 0);
> 
> nf_ip_checksum() in net/netfilter/utils.c only verifies for PRE_ROUTING
> and LOCAL_IN:
> 
> 	case CHECKSUM_COMPLETE:
> 		if (hook != NF_INET_PRE_ROUTING && hook != NF_INET_LOCAL_IN)
> 			break;
> 
> With hook == NF_INET_INGRESS, csum stays 0, and the reject code reads that
> as a good checksum.
> 
> nf_ip6_checksum() has the same check. It affects
> reject6_csum_ok()->icmpv6_send() in nf_send_unreach6(), and also the TCP
> RST path through nf_reject_ip6_tcphdr_get().
> 
> So can an inet ingress reject rule answer a packet with a bad L4 checksum
> with an ICMP/ICMPv6 unreachable or a TCP RST? The same packet would get no
> reply at PRE_ROUTING, or at netdev ingress (NF_NETDEV_INGRESS has the same
> value as NF_INET_PRE_ROUTING).
> 
> Adding NF_INET_INGRESS to that hook list may not be enough on its own. At
> ingress, skb->len can still include link-layer trailer padding, which
> ip_rcv_core()/ip6_rcv_core() would normally trim.
> 
> This came in with commit 117ca1f8920c ("netfilter: nft_reject_inet: allow
> to use reject from inet ingress").
> 
> > diff --git a/net/ipv6/netfilter/nf_reject_ipv6.c b/net/ipv6/netfilter/nf_reject_ipv6.c
> > index 07cdaa10da0d..5e08d8529cc8 100644
> > --- a/net/ipv6/netfilter/nf_reject_ipv6.c
> > +++ b/net/ipv6/netfilter/nf_reject_ipv6.c
> > @@ -444,6 +444,11 @@ 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_INGRESS) {
> > +		memset(IP6CB(skb_in), 0, sizeof(*IP6CB(skb_in)));
> > +		IP6CB(skb_in)->iif = skb_in->skb_iif;
> > +	}
> 
> [Severity: Medium]
> The commit message says this matches ip6_rcv_core(). Does that hold for
> packets looped back through lo? ip6_rcv_core() chooses iif based on the
> dst:
> 
> 	IP6CB(skb)->iif = skb_valid_dst(skb) ?
> 				ip6_dst_idev(skb_dst(skb))->dev->ifindex :
> 				dev->ifindex;

There is no dst in the inet/ingress reject path.

This currently does not dst lookup.

This fix aims at a minimal cure for uninitialized access to skb->cb
area.

This fix address the issue reported here by moving this block

        if (hooknum == NF_INET_INGRESS) {
                memset(IP6CB(skb_in), 0, sizeof(*IP6CB(skb_in)));
                IP6CB(skb_in)->iif = skb_in->skb_iif;
        }

after:

        if (!skb_valid_dst(skb_in) && nf_reject6_fill_skb_dst(skb_in) < 0)
                return;

in nf_send_unreach6().

That should fix this feature in loopback.

But I prefer this patch gets in because it is already improving the
situation.