Re: [PATCH 4/9 nf-next] netfilter: conntrack: use DEBUG_NET_WARN_ON_ONCE on packet paths

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

On Thu, Jun 18, 2026 at 07:32:50PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > diff --git a/net/netfilter/nf_conntrack_proto_icmp.c b/net/netfilter/nf_conntrack_proto_icmp.c
> > > index 32148a3a8509..0f39cb147c4f 100644
> > > --- a/net/netfilter/nf_conntrack_proto_icmp.c
> > > +++ b/net/netfilter/nf_conntrack_proto_icmp.c
> > > @@ -117,7 +117,8 @@ int nf_conntrack_inet_error(struct nf_conn *tmpl, struct sk_buff *skb,
> > >  	enum ip_conntrack_dir dir;
> > >  	struct nf_conn *ct;
> > >  
> > > -	WARN_ON(skb_nfct(skb));
> > > +	if (unlikely(skb_nfct(skb)))
> > > +		DEBUG_NET_WARN_ON_ONCE(1);
> 
> Should be
> DEBUG_NET_WARN_ON_ONCE(skb_nfct(skb)));
> ?
> 
> > nf_conntrack_in
> >  [ reset skb->nfct ]
> >  nf_conntrack_handle_icmp
> >   nf_conntrack_icmpv4_error
> >    nf_conntrack_inet_error
> > 
> > There is nf_conntrack_inet_error() which performs the ct lookup.
> > There is resolve_normal_ct() too, but these two are coming later.
> > 
> > [ ... snippet that resets skb->nfct ... ]
> > unsigned int
> > nf_conntrack_in(struct sk_buff *skb, const struct nf_hook_state *state)
> > {
> >         enum ip_conntrack_info ctinfo;
> >         struct nf_conn *ct, *tmpl;
> >         u_int8_t protonum;
> >         int dataoff, ret;
> >  
> >         tmpl = nf_ct_get(skb, &ctinfo);
> >         if (tmpl || ctinfo == IP_CT_UNTRACKED) {
> >                 /* Previously seen (loopback or untracked)?  Ignore. */
> >                 if ((tmpl && !nf_ct_is_template(tmpl)) ||
> >                      ctinfo == IP_CT_UNTRACKED)
> >                         return NF_ACCEPT;
> >                 skb->_nfct = 0; <---------  this is reset here.
> >         }
> > [ end of snippet ]
> > 
> > I don't remember to have seen this WARN_ON, so remove it.
> 
> I would keep the DEBUG_NET_WARN_ON_ONCE(), else this gives a
> refcount leak.
> 
> Or, move it closer to the end:
> 
> 191         /* Update skb to refer to this connection */
> 	HERE.
> 192         nf_ct_set(skb, ct, ctinfo);
> 193         return NF_ACCEPT;
> 194 }

OK, let's keep it around. Thanks.