Subject: Re: [PATCH nf] netfilter: nf_tables: unconditionally flush pending work before notifier

From: Florian Westphal <[email-masked]>
Date: Thu, 4 Jul 2024 12:54:18 +0200

Hillf Danton <[email-masked]> wrote:
> On Wed, 3 Jul 2024 15:01:07 +0200 Florian Westphal <[email-masked]>
> > Hillf Danton <[email-masked]> wrote:
> > > On Wed, 3 Jul 2024 12:52:15 +0200 Florian Westphal <[email-masked]>
> > > > Hillf Danton <[email-masked]> wrote:
> > > > > Given trans->table goes thru the lifespan of trans, your proposal is a bandaid
> > > > > if trans outlives table.
> > > > 
> > > > trans must never outlive table.
> > > > 
> > > What is preventing trans from being freed after closing sock, given
> > > trans is freed in workqueue?
> > > 
> > > 	close sock
> > > 	queue work
> > 
> > The notifier acquires the transaction mutex, locking out all other
> > transactions, so no further transactions requests referencing
> > the table can be queued.
> > 
> As per the syzbot report, trans->table could be instantiated before
> notifier acquires the transaction mutex. And in fact the lock helps
> trans outlive table even with your patch.
> 
> 	cpu1			cpu2
> 	---			---
> 	transB->table = A
> 				lock trans mutex
> 				flush work
> 				free A
> 				unlock trans mutex
> 
> 	queue work to free transB

Can you show a crash reproducer or explain how this assign
and queueing happens unordered wrt. cpu2?

This should look like this:

 	cpu1			cpu2
 	---			---
	lock trans mutex
  				lock trans mutex -> blocks
 	transB->table = A
  	queue work to free transB
	unlock trans mutex
				lock trans mutex returns
  				flush work
  				free A
  				unlock trans mutex