diff mbox series

[net,03/11] netfilter: nft_flow_offload: drop flowtable reference on init error path

Message ID 20260927220816.268206-4-pablo@netfilter.org
State Changes Requested
Headers show
Series [net,01/11] netfilter: ipset: do not update comments from kernel-side adds | expand

Commit Message

Pablo Neira Ayuso Sept. 27, 2026, 10:08 p.m. UTC 
From: Aohan Mei <henrymei@tencent.com>

nft_flow_offload_init() bumps the flowtable use count with
nft_use_inc() before calling nf_ct_netns_get().  When the latter
fails, the error is returned as-is and the reference is leaked.

The upper layers do not balance it either: nf_tables_newexpr()
clears expr->ops when the expression init callback fails, so the
nft_expr_more() iteration in nft_rule_expr_deactivate() and
nf_tables_rule_destroy() stops right before the failed expression
and its ->destroy callback, which would drop the reference, never
runs.

Each failed rule addition therefore leaks one flowtable reference
and the flowtable can no longer be removed: NFT_MSG_DELFLOWTABLE
keeps reporting -EBUSY even though no rule references it.

Save the nf_ct_netns_get() return value and undo the nft_use_inc()
when it fails, restoring the inc/dec pairing within
nft_flow_offload_init() itself.

Fixes: a3c90f7a2323 ("netfilter: nf_tables: flow offload expression")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_flow_offload.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

netdev-bot+sashiko@kernel.org Sept. 28, 2026, 11:55 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] One sentence in the commit message names the wrong callback.

--

netfilter: nft_flow_offload: drop flowtable reference on init error path

In nft_flow_offload_init(), nft_use_inc() raises the flowtable use count
before nf_ct_netns_get() is called. If nf_ct_netns_get() fails, that count
is never dropped. The patch saves the return value of nf_ct_netns_get() and
calls nft_use_dec() on failure, so the inc/dec pairing is restored inside
the init callback.

> The upper layers do not balance it either: nf_tables_newexpr()
> clears expr->ops when the expression init callback fails, so the
> nft_expr_more() iteration in nft_rule_expr_deactivate() and
> nf_tables_rule_destroy() stops right before the failed expression
> and its ->destroy callback, which would drop the reference, never
> runs.

[Severity: Low]
This isn't a bug, but does this paragraph name the wrong callback? In
net/netfilter/nft_flow_offload.c, the ->destroy callback only drops the
conntrack netns reference:

static void nft_flow_offload_destroy(const struct nft_ctx *ctx,
				     const struct nft_expr *expr)
{
	nf_ct_netns_put(ctx->net, ctx->family);
}

The flowtable use count is dropped through ->deactivate instead.
nft_flow_offload_deactivate() calls nf_tables_deactivate_flowtable(), and
that function does the decrement:

net/netfilter/nf_tables_api.c:nf_tables_deactivate_flowtable() {
	switch (phase) {
	case NFT_TRANS_PREPARE_ERROR:
	case NFT_TRANS_PREPARE:
	case NFT_TRANS_ABORT:
	case NFT_TRANS_RELEASE:
		nft_use_dec(&flowtable->use);
	...
}

On the nf_tables_newrule() error path, the callback that gets skipped for
the failed expression is this one:

	nft_rule_expr_deactivate(&ctx, rule, NFT_TRANS_PREPARE_ERROR);

It is skipped because nf_tables_newexpr() cleared expr->ops, which ends the
nft_expr_more() walk.

The code change looks correct. Could the commit message say that the
skipped ->deactivate callback is what would drop the reference, rather than
->destroy? That would help anyone checking the fix against
nft_flow_offload_destroy(), for example during a stable backport.
diff mbox series

Patch

diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index 32b4281038dd..d3c5651dd699 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -160,6 +160,7 @@  static int nft_flow_offload_init(const struct nft_ctx *ctx,
 	struct nft_flow_offload *priv = nft_expr_priv(expr);
 	u8 genmask = nft_genmask_next(ctx->net);
 	struct nft_flowtable *flowtable;
+	int err;
 
 	if (!tb[NFTA_FLOW_TABLE_NAME])
 		return -EINVAL;
@@ -174,7 +175,11 @@  static int nft_flow_offload_init(const struct nft_ctx *ctx,
 
 	priv->flowtable = flowtable;
 
-	return nf_ct_netns_get(ctx->net, ctx->family);
+	err = nf_ct_netns_get(ctx->net, ctx->family);
+	if (err < 0)
+		nft_use_dec(&flowtable->use);
+
+	return err;
 }
 
 static void nft_flow_offload_deactivate(const struct nft_ctx *ctx,