From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Florian Westphal <fw@strlen.de>, Sasha Levin <sashal@kernel.org>,
pablo@netfilter.org, kadlec@netfilter.org,
netfilter-devel@vger.kernel.org, coreteam@netfilter.org
Subject: [PATCH AUTOSEL 6.17] netfilter: nf_tables: all transaction allocations can now sleep
Date: Sat, 25 Oct 2025 11:57:29 -0400 [thread overview]
Message-ID: <20251025160905.3857885-218-sashal@kernel.org> (raw)
In-Reply-To: <20251025160905.3857885-1-sashal@kernel.org>
From: Florian Westphal <fw@strlen.de>
[ Upstream commit 3d95a2e016abab29ccb6f384576b2038e544a5a8 ]
Now that nft_setelem_flush is not called with rcu read lock held or
disabled softinterrupts anymore this can now use GFP_KERNEL too.
This is the last atomic allocation of transaction elements, so remove
all gfp_t arguments and the wrapper function.
This makes attempts to delete large sets much more reliable, before
this was prone to transient memory allocation failures.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
YES
Explanation
What changed
- Switched all nf_tables transaction element allocations to use
sleepable allocations and removed gfp plumbing.
- `nft_trans_alloc()` now always uses `kzalloc(..., GFP_KERNEL)`
instead of parameterized gfp or `GFP_ATOMIC` via wrapper removal:
net/netfilter/nf_tables_api.c:154.
- Set-element transaction collapsing uses `krealloc(..., GFP_KERNEL)`:
net/netfilter/nf_tables_api.c:457.
- Removed gfp_t parameters from commit-list helpers and collapse
logic; `nft_trans_commit_list_add_elem()` no longer takes/propagates
gfp and simply collapses or enqueues:
net/netfilter/nf_tables_api.c:535.
- Bulk element flush paths allocate transactions and enqueue them
without any atomic gfp flags:
- `nft_setelem_flush()` allocates with `nft_trans_alloc(...
GFP_KERNEL)` and enqueues via `nft_trans_commit_list_add_elem()`:
net/netfilter/nf_tables_api.c:7872,
net/netfilter/nf_tables_api.c:7893.
- Catchall flush similarly enqueues with the new helper:
net/netfilter/nf_tables_api.c:7906,
net/netfilter/nf_tables_api.c:7912.
Why this is safe now (sleepable paths are guaranteed)
- The iter callback used for set-element flushing (`.fn =
nft_setelem_flush`) runs in a context where sleeping is allowed during
UPDATE walks:
- Hash backend creates a snapshot under RCU and then iterates that
snapshot with `commit_mutex` held so the callback “can sleep”
(explicitly documented): net/netfilter/nft_set_hash.c:324–332,
372–383.
- Rbtree backend asserts `commit_mutex` for UPDATE walks and calls
`iter->fn` under that mutex (no BH/RCU read section):
net/netfilter/nft_set_rbtree.c:609–627.
- Bitmap backend traverses with RCU but protected by `commit_mutex`
(writer lock allowed for traversal) and calls `iter->fn` under that
protection (sleepable): net/netfilter/nft_set_bitmap.c:230–241.
- The bulk delete entry point (`nf_tables_delsetelem`) sets up UPDATE
walk (`.type = NFT_ITER_UPDATE`) and uses `nft_set_flush()` which
wires `nft_setelem_flush` as the callback, ensuring it executes in the
above sleepable contexts: net/netfilter/nf_tables_api.c:7940–7955.
- Transaction element additions/deletions are performed from netlink
processing paths (process context), not hardirq/softirq handlers, so
allocating with `GFP_KERNEL` is appropriate in all call sites shown by
the new helpers: net/netfilter/nf_tables_api.c:7568–7597, 7847–7895,
7906–7912.
User-visible impact (bug fix)
- Deleting large sets previously used `GFP_ATOMIC` along the
flush/commit-item path, making allocations prone to transient failures
under memory pressure. By switching to `GFP_KERNEL` and permitting
sleeping, large set deletions become substantially more reliable,
aligning with the commit message intent.
- The collapsing path (`nft_trans_collapse_set_elem`) that may
`krealloc` the transaction to coalesce elements now does so with
`GFP_KERNEL`, reducing failure rates:
net/netfilter/nf_tables_api.c:457–468.
Scope and risk
- Scope: confined to nf_tables internal transaction allocation/path; no
UAPI changes, no semantic changes to ruleset behavior.
- Architectural changes: none; this is a cleanup following prior design
changes that made iter callbacks sleepable.
- Side effects: allocations may now sleep. This is intended and correct
given the current walk/flush call paths hold `commit_mutex` or operate
on snapshot lists designed for sleepable callbacks, as shown above.
- Regression risk: low in trees that already have the reworked set
walk/flush semantics (snapshot under RCU or commit_mutex-protected
UPDATE walks). If a target stable tree does not include those enabling
changes, backporting this patch alone would be unsafe because it could
sleep in contexts that used to run under RCU read lock or with
softirqs disabled. In such cases, this commit should be backported
together with the prerequisite walk/flush changes (e.g., the
hash/rbtree/bitmap UPDATE-walk designs that explicitly allow
sleeping).
Conclusion
- This is a contained reliability fix that removes the last atomic
allocation along nf_tables transaction element paths and is consistent
with the current, sleepable UPDATE-walk design. It reduces transient
ENOMEM failures when deleting large sets with minimal risk, provided
the prerequisite sleepable-walk changes are present in the target
stable series. Recommended for stable backport with that dependency
consideration.
net/netfilter/nf_tables_api.c | 47 ++++++++++++++---------------------
1 file changed, 19 insertions(+), 28 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c3c73411c40c4..eed434e0a9702 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -151,12 +151,12 @@ static void nft_ctx_init(struct nft_ctx *ctx,
bitmap_zero(ctx->reg_inited, NFT_REG32_NUM);
}
-static struct nft_trans *nft_trans_alloc_gfp(const struct nft_ctx *ctx,
- int msg_type, u32 size, gfp_t gfp)
+static struct nft_trans *nft_trans_alloc(const struct nft_ctx *ctx,
+ int msg_type, u32 size)
{
struct nft_trans *trans;
- trans = kzalloc(size, gfp);
+ trans = kzalloc(size, GFP_KERNEL);
if (trans == NULL)
return NULL;
@@ -172,12 +172,6 @@ static struct nft_trans *nft_trans_alloc_gfp(const struct nft_ctx *ctx,
return trans;
}
-static struct nft_trans *nft_trans_alloc(const struct nft_ctx *ctx,
- int msg_type, u32 size)
-{
- return nft_trans_alloc_gfp(ctx, msg_type, size, GFP_KERNEL);
-}
-
static struct nft_trans_binding *nft_trans_get_binding(struct nft_trans *trans)
{
switch (trans->msg_type) {
@@ -442,8 +436,7 @@ static bool nft_trans_collapse_set_elem_allowed(const struct nft_trans_elem *a,
static bool nft_trans_collapse_set_elem(struct nftables_pernet *nft_net,
struct nft_trans_elem *tail,
- struct nft_trans_elem *trans,
- gfp_t gfp)
+ struct nft_trans_elem *trans)
{
unsigned int nelems, old_nelems = tail->nelems;
struct nft_trans_elem *new_trans;
@@ -466,9 +459,11 @@ static bool nft_trans_collapse_set_elem(struct nftables_pernet *nft_net,
/* krealloc might free tail which invalidates list pointers */
list_del_init(&tail->nft_trans.list);
- new_trans = krealloc(tail, struct_size(tail, elems, nelems), gfp);
+ new_trans = krealloc(tail, struct_size(tail, elems, nelems),
+ GFP_KERNEL);
if (!new_trans) {
- list_add_tail(&tail->nft_trans.list, &nft_net->commit_list);
+ list_add_tail(&tail->nft_trans.list,
+ &nft_net->commit_list);
return false;
}
@@ -484,7 +479,7 @@ static bool nft_trans_collapse_set_elem(struct nftables_pernet *nft_net,
}
static bool nft_trans_try_collapse(struct nftables_pernet *nft_net,
- struct nft_trans *trans, gfp_t gfp)
+ struct nft_trans *trans)
{
struct nft_trans *tail;
@@ -501,7 +496,7 @@ static bool nft_trans_try_collapse(struct nftables_pernet *nft_net,
case NFT_MSG_DELSETELEM:
return nft_trans_collapse_set_elem(nft_net,
nft_trans_container_elem(tail),
- nft_trans_container_elem(trans), gfp);
+ nft_trans_container_elem(trans));
}
return false;
@@ -537,17 +532,14 @@ static void nft_trans_commit_list_add_tail(struct net *net, struct nft_trans *tr
}
}
-static void nft_trans_commit_list_add_elem(struct net *net, struct nft_trans *trans,
- gfp_t gfp)
+static void nft_trans_commit_list_add_elem(struct net *net, struct nft_trans *trans)
{
struct nftables_pernet *nft_net = nft_pernet(net);
WARN_ON_ONCE(trans->msg_type != NFT_MSG_NEWSETELEM &&
trans->msg_type != NFT_MSG_DELSETELEM);
- might_alloc(gfp);
-
- if (nft_trans_try_collapse(nft_net, trans, gfp)) {
+ if (nft_trans_try_collapse(nft_net, trans)) {
kfree(trans);
return;
}
@@ -7573,7 +7565,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
}
ue->priv = elem_priv;
- nft_trans_commit_list_add_elem(ctx->net, trans, GFP_KERNEL);
+ nft_trans_commit_list_add_elem(ctx->net, trans);
goto err_elem_free;
}
}
@@ -7597,7 +7589,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
}
nft_trans_container_elem(trans)->elems[0].priv = elem.priv;
- nft_trans_commit_list_add_elem(ctx->net, trans, GFP_KERNEL);
+ nft_trans_commit_list_add_elem(ctx->net, trans);
return 0;
err_set_full:
@@ -7863,7 +7855,7 @@ static int nft_del_setelem(struct nft_ctx *ctx, struct nft_set *set,
nft_setelem_data_deactivate(ctx->net, set, elem.priv);
nft_trans_container_elem(trans)->elems[0].priv = elem.priv;
- nft_trans_commit_list_add_elem(ctx->net, trans, GFP_KERNEL);
+ nft_trans_commit_list_add_elem(ctx->net, trans);
return 0;
fail_ops:
@@ -7888,9 +7880,8 @@ static int nft_setelem_flush(const struct nft_ctx *ctx,
if (!nft_set_elem_active(ext, iter->genmask))
return 0;
- trans = nft_trans_alloc_gfp(ctx, NFT_MSG_DELSETELEM,
- struct_size_t(struct nft_trans_elem, elems, 1),
- GFP_ATOMIC);
+ trans = nft_trans_alloc(ctx, NFT_MSG_DELSETELEM,
+ struct_size_t(struct nft_trans_elem, elems, 1));
if (!trans)
return -ENOMEM;
@@ -7901,7 +7892,7 @@ static int nft_setelem_flush(const struct nft_ctx *ctx,
nft_trans_elem_set(trans) = set;
nft_trans_container_elem(trans)->nelems = 1;
nft_trans_container_elem(trans)->elems[0].priv = elem_priv;
- nft_trans_commit_list_add_elem(ctx->net, trans, GFP_ATOMIC);
+ nft_trans_commit_list_add_elem(ctx->net, trans);
return 0;
}
@@ -7918,7 +7909,7 @@ static int __nft_set_catchall_flush(const struct nft_ctx *ctx,
nft_setelem_data_deactivate(ctx->net, set, elem_priv);
nft_trans_container_elem(trans)->elems[0].priv = elem_priv;
- nft_trans_commit_list_add_elem(ctx->net, trans, GFP_KERNEL);
+ nft_trans_commit_list_add_elem(ctx->net, trans);
return 0;
}
--
2.51.0
next parent reply other threads:[~2025-10-25 16:19 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20251025160905.3857885-1-sashal@kernel.org>
2025-10-25 15:57 ` Sasha Levin [this message]
2025-10-25 16:01 ` [PATCH AUTOSEL 6.17-5.15] netfilter: nf_reject: don't reply to icmp error messages Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251025160905.3857885-218-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=coreteam@netfilter.org \
--cc=fw@strlen.de \
--cc=kadlec@netfilter.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.org \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).