[PATCH v2] netfilter: nf_tables: defer object destruction on abort past commit_mutex

[Nguyen Ngoc Thang]

__nf_tables_abort() calls synchronize_rcu() while holding
nft_net->commit_mutex, on every aborted batch. The commit path had
the same problem and was already fixed by deferring destruction to
nft_net->destroy_work so the mutex is dropped before waiting for the
grace period (see the comment in nf_tables_commit_release()). The
abort path was never given the same treatment.

On syzbot's images every synchronize_rcu() runs as
synchronize_rcu_expedited(), because CONFIG_CMDLINE bakes in
rcupdate.rcu_expedited=1. Under fuzzing-rate aborted batches this
turns each abort into an expedited grace-period wait taken while
commit_mutex is held, serializing every other nf_tables netlink
request behind it. syzbot reports this as a hung task in
nf_tables_valid_genid(), whose lockdep "locks held" dump shows the
commit_mutex owner parked inside synchronize_rcu_expedited() rather
than deadlocked on a lock.

Reuse the existing destroy_work machinery for the abort path too:
splice the still-mutex-protected commit_list onto destroy_list and
schedule the work, instead of waiting for the grace period inline.
Since abort transactions carry NEW*-type objects (versus DEL*/DESTROY*
for commits), nf_tables_abort_release() gains the put_net() handling
nft_commit_release() already had, and a new trans->aborted bit tells
the shared work function which of the two release paths to use for
each transaction.

nft_trans_list_del() also unlinks bindable NEWSET/NEWCHAIN
transactions from nft_net->binding_list, a list only ever walked
under commit_mutex. That unlink is kept synchronous, under the mutex,
in __nf_tables_abort() itself; only the RCU-gated free is deferred to
the workqueue.

Root cause identified from syzbot's crash report (lockdep holder vs.
waiter pair) and code inspection, cross-checked against syzkaller's
own dashboard/config/linux/bits/base.yml for the rcu_expedited=1
CONFIG_CMDLINE.

Verified by booting both the unpatched and patched kernel in QEMU
(2 vCPUs, rcupdate.rcu_expedited=1, CONFIG_DEFAULT_HUNG_TASK_TIMEOUT=140
to match syzbot's environment) and driving nf_tables abort with a
script that forces a guaranteed abort-with-content every iteration
(a freshly-named table + chain + rule, then a delete of a nonexistent
chain in the same nft(8) batch, so the whole batch is always rolled
back with a brand-new, never-committed table/chain transaction still
on commit_list). Measuring the time commit_mutex is held across this
path, same script and iteration count on both kernels:

                  unpatched          patched
  avg               72.8 us            20.2 us
  max                2.2 ms             1.4 ms
  min               18.1 us             8.1 us

The unpatched number is the full inline synchronize_rcu_expedited()
wait; the patched number is what's left once that wait is off the
mutex (splice + schedule_work() only), which is scheduling noise in a
2-vCPU VM, not a grace-period wait -- the deferred path never calls
synchronize_rcu() while holding commit_mutex, by construction.

Reported-by: syzbot+83439cb981624bd8d068@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=83439cb981624bd8d068
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@xxxxxxxxx>
Co-Authored-By: Claude Sonnet 5 <noreply@xxxxxxxxxxxxx>
---
v2: shortened two in-code comments that were needlessly long for what
    they explain; no functional change from v1.

 include/net/netfilter/nf_tables.h |  1 +
 net/netfilter/nf_tables_api.c     | 40 +++++++++++++++++++++++++------
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 9d597482363d..e77831e3494c 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1672,6 +1672,7 @@ struct nft_trans {
 	u16				flags;
 	u8				report:1;
 	u8				put_net:1;
+	u8				aborted:1;
 };
 
 /**
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c0b754a2d45b..a8b3bf969bd8 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -146,6 +146,7 @@ static bool nft_chain_vstate_valid(const struct nft_ctx *ctx,
 }
 
 static void nf_tables_trans_destroy_work(struct work_struct *w);
+static void nf_tables_abort_release(struct nft_trans *trans);
 
 static void nft_trans_gc_work(struct work_struct *work);
 static DECLARE_WORK(trans_gc_work, nft_trans_gc_work);
@@ -10288,8 +10289,12 @@ static void nf_tables_trans_destroy_work(struct work_struct *w)
 	synchronize_rcu();
 
 	list_for_each_entry_safe(trans, next, &head, list) {
-		nft_trans_list_del(trans);
-		nft_commit_release(trans);
+		/* binding_list unlink, if any, already done under commit_mutex. */
+		list_del(&trans->list);
+		if (trans->aborted)
+			nf_tables_abort_release(trans);
+		else
+			nft_commit_release(trans);
 	}
 }
 
@@ -11278,6 +11283,10 @@ static void nf_tables_abort_release(struct nft_trans *trans)
 			nf_tables_flowtable_destroy(nft_trans_flowtable(trans));
 		break;
 	}
+
+	if (trans->put_net)
+		put_net(trans->net);
+
 	kfree(trans);
 }
 
@@ -11482,12 +11491,29 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 
 	nft_set_abort_update(nft_net);
 
-	synchronize_rcu();
+	/* Defer past the GP like nf_tables_commit_release(); keep the
+	 * binding_list unlink here, under commit_mutex (see nft_trans_list_del()).
+	 */
+	if (!list_empty(&nft_net->commit_list)) {
+		LIST_HEAD(head);
+
+		list_for_each_entry_safe_reverse(trans, next,
+						 &nft_net->commit_list, list) {
+			trans->aborted = 1;
+			nft_trans_list_del(trans);
+			list_add_tail(&trans->list, &head);
+		}
+
+		trans = list_last_entry(&head, struct nft_trans, list);
+		get_net(trans->net);
+		WARN_ON_ONCE(trans->put_net);
+		trans->put_net = true;
+
+		spin_lock(&nf_tables_destroy_list_lock);
+		list_splice_tail(&head, &nft_net->destroy_list);
+		spin_unlock(&nf_tables_destroy_list_lock);
 
-	list_for_each_entry_safe_reverse(trans, next,
-					 &nft_net->commit_list, list) {
-		nft_trans_list_del(trans);
-		nf_tables_abort_release(trans);
+		schedule_work(&nft_net->destroy_work);
 	}
 
 	return err;
-- 
2.43.0