Re: [PATCH net 01/10] netfilter: flowtable: publish HW_DEAD after worker is done

Pablo Neira Ayuso <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.netfilter-devel
Message-ID <aqz4cRSbfcJI1Gvs@chamomile>
On Thu, Sep 17, 2026 at 07:04:06PM -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> netfilter: flowtable: publish HW_DEAD after worker is done
> 
> The patch moves the NF_FLOW_HW_DEAD publication out of
> flow_offload_work_del() into the common epilogue of
> flow_offload_work_handler(), so that it happens after
> clear_bit(NF_FLOW_HW_PENDING) rather than before it.  The stated goal is
> to make setting HW_DEAD the last flow access performed by destroy work,
> so garbage collection cannot free the flow while the worker is still
> touching it.
> 
> > diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> > index 801a3dd9ceea3..6757fd89c1f1a 100644
> > --- a/net/netfilter/nf_flow_table_offload.c
> > +++ b/net/netfilter/nf_flow_table_offload.c
> > @@ -995,7 +995,6 @@ static void flow_offload_work_del(struct flow_offload_work *offload)
> >  	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_ORIGINAL);
> >  	if (test_bit(NF_FLOW_HW_BIDIRECTIONAL, &offload->flow->flags))
> >  		flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_REPLY);
> > -	set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
> >  }
> >  
> >  static void flow_offload_tuple_stats(struct flow_offload_work *offload,
> > @@ -1059,6 +1058,12 @@ static void flow_offload_work_handler(struct work_struct *work)
> >  	}
> >  
> >  	clear_bit(NF_FLOW_HW_PENDING, &offload->flow->flags);
> > +	if (offload->cmd == FLOW_CLS_DESTROY) {
> > +		/* Publish after the worker's last flow access. */
> > +		smp_mb__before_atomic();
> > +		set_bit(NF_FLOW_HW_DEAD, &offload->flow->flags);
> > +	}
> > +
> 
> [Severity: High]
> Does this reordering close the whole use-after-free, or only the
> FLOW_CLS_DESTROY part of it?

This describes a pre-existing issue. I posted patches for this I have
to pick up.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.