Skip to content

Commit fb7fb40

Browse files
ummakynesFlorian Westphal
authored andcommitted
netfilter: nf_tables: clone set on flush only
Syzbot with fault injection triggered a failing memory allocation with GFP_KERNEL which results in a WARN splat: iter.err WARNING: net/netfilter/nf_tables_api.c:845 at nft_map_deactivate+0x34e/0x3c0 net/netfilter/nf_tables_api.c:845, CPU#0: syz.0.17/5992 Modules linked in: CPU: 0 UID: 0 PID: 5992 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full) Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2026 RIP: 0010:nft_map_deactivate+0x34e/0x3c0 net/netfilter/nf_tables_api.c:845 Code: 8b 05 86 5a 4e 09 48 3b 84 24 a0 00 00 00 75 62 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc cc e8 63 6d fa f7 90 <0f> 0b 90 43 +80 7c 35 00 00 0f 85 23 fe ff ff e9 26 fe ff ff 89 d9 RSP: 0018:ffffc900045af780 EFLAGS: 00010293 RAX: ffffffff89ca45bd RBX: 00000000fffffff4 RCX: ffff888028111e40 RDX: 0000000000000000 RSI: 00000000fffffff4 RDI: 0000000000000000 RBP: ffffc900045af870 R08: 0000000000400dc0 R09: 00000000ffffffff R10: dffffc0000000000 R11: fffffbfff1d141db R12: ffffc900045af7e0 R13: 1ffff920008b5f24 R14: dffffc0000000000 R15: ffffc900045af920 FS: 000055557a6a5500(0000) GS:ffff888125496000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fb5ea271fc0 CR3: 000000003269e000 CR4: 00000000003526f0 Call Trace: <TASK> __nft_release_table+0xceb/0x11f0 net/netfilter/nf_tables_api.c:12115 nft_rcv_nl_event+0xc25/0xdb0 net/netfilter/nf_tables_api.c:12187 notifier_call_chain+0x19d/0x3a0 kernel/notifier.c:85 blocking_notifier_call_chain+0x6a/0x90 kernel/notifier.c:380 netlink_release+0x123b/0x1ad0 net/netlink/af_netlink.c:761 __sock_release net/socket.c:662 [inline] sock_close+0xc3/0x240 net/socket.c:1455 Restrict set clone to the flush set command in the preparation phase. Add NFT_ITER_UPDATE_CLONE and use it for this purpose, update the rbtree and pipapo backends to only clone the set when this iteration type is used. As for the existing NFT_ITER_UPDATE type, update the pipapo backend to use the existing set clone if available, otherwise use the existing set representation. After this update, there is no need to clone a set that is being deleted, this includes bound anonymous set. An alternative approach to NFT_ITER_UPDATE_CLONE is to add a .clone interface and call it from the flush set path. Reported-by: [email protected] Fixes: 3f1d886 ("netfilter: nft_set_pipapo: move cloning of match info to insert/removal path") Signed-off-by: Pablo Neira Ayuso <[email protected]> Signed-off-by: Florian Westphal <[email protected]>
1 parent def602e commit fb7fb40

5 files changed

Lines changed: 26 additions & 6 deletions

File tree

include/net/netfilter/nf_tables.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,11 +320,13 @@ static inline void *nft_elem_priv_cast(const struct nft_elem_priv *priv)
320320
* @NFT_ITER_UNSPEC: unspecified, to catch errors
321321
* @NFT_ITER_READ: read-only iteration over set elements
322322
* @NFT_ITER_UPDATE: iteration under mutex to update set element state
323+
* @NFT_ITER_UPDATE_CLONE: clone set before iteration under mutex to update element
323324
*/
324325
enum nft_iter_type {
325326
NFT_ITER_UNSPEC,
326327
NFT_ITER_READ,
327328
NFT_ITER_UPDATE,
329+
NFT_ITER_UPDATE_CLONE,
328330
};
329331

330332
struct nft_set;

net/netfilter/nf_tables_api.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,11 @@ static void nft_map_catchall_deactivate(const struct nft_ctx *ctx,
833833
}
834834
}
835835

836+
/* Use NFT_ITER_UPDATE iterator even if this may be called from the preparation
837+
* phase, the set clone might already exist from a previous command, or it might
838+
* be a set that is going away and does not require a clone. The netns and
839+
* netlink release paths also need to work on the live set.
840+
*/
836841
static void nft_map_deactivate(const struct nft_ctx *ctx, struct nft_set *set)
837842
{
838843
struct nft_set_iter iter = {
@@ -7903,9 +7908,12 @@ static int nft_set_catchall_flush(const struct nft_ctx *ctx,
79037908

79047909
static int nft_set_flush(struct nft_ctx *ctx, struct nft_set *set, u8 genmask)
79057910
{
7911+
/* The set backend might need to clone the set, do it now from the
7912+
* preparation phase, use NFT_ITER_UPDATE_CLONE iterator type.
7913+
*/
79067914
struct nft_set_iter iter = {
79077915
.genmask = genmask,
7908-
.type = NFT_ITER_UPDATE,
7916+
.type = NFT_ITER_UPDATE_CLONE,
79097917
.fn = nft_setelem_flush,
79107918
};
79117919

net/netfilter/nft_set_hash.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ static void nft_rhash_walk(const struct nft_ctx *ctx, struct nft_set *set,
374374
{
375375
switch (iter->type) {
376376
case NFT_ITER_UPDATE:
377+
case NFT_ITER_UPDATE_CLONE:
377378
/* only relevant for netlink dumps which use READ type */
378379
WARN_ON_ONCE(iter->skip != 0);
379380

net/netfilter/nft_set_pipapo.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2144,13 +2144,20 @@ static void nft_pipapo_walk(const struct nft_ctx *ctx, struct nft_set *set,
21442144
const struct nft_pipapo_match *m;
21452145

21462146
switch (iter->type) {
2147-
case NFT_ITER_UPDATE:
2147+
case NFT_ITER_UPDATE_CLONE:
21482148
m = pipapo_maybe_clone(set);
21492149
if (!m) {
21502150
iter->err = -ENOMEM;
21512151
return;
21522152
}
2153-
2153+
nft_pipapo_do_walk(ctx, set, m, iter);
2154+
break;
2155+
case NFT_ITER_UPDATE:
2156+
if (priv->clone)
2157+
m = priv->clone;
2158+
else
2159+
m = rcu_dereference_protected(priv->match,
2160+
nft_pipapo_transaction_mutex_held(set));
21542161
nft_pipapo_do_walk(ctx, set, m, iter);
21552162
break;
21562163
case NFT_ITER_READ:

net/netfilter/nft_set_rbtree.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -861,13 +861,15 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx,
861861
struct nft_rbtree *priv = nft_set_priv(set);
862862

863863
switch (iter->type) {
864-
case NFT_ITER_UPDATE:
865-
lockdep_assert_held(&nft_pernet(ctx->net)->commit_mutex);
866-
864+
case NFT_ITER_UPDATE_CLONE:
867865
if (nft_array_may_resize(set) < 0) {
868866
iter->err = -ENOMEM;
869867
break;
870868
}
869+
fallthrough;
870+
case NFT_ITER_UPDATE:
871+
lockdep_assert_held(&nft_pernet(ctx->net)->commit_mutex);
872+
871873
nft_rbtree_do_walk(ctx, set, iter);
872874
break;
873875
case NFT_ITER_READ:

0 commit comments

Comments
 (0)