Skip to content

Commit ccb8c8f

Browse files
ummakynesSasha Levin
authored andcommitted
netfilter: nf_tables: unconditionally bump set->nelems before insertion
[ Upstream commit def602e ] In case that the set is full, a new element gets published then removed without waiting for the RCU grace period, while RCU reader can be walking over it already. To address this issue, add the element transaction even if set is full, but toggle the set_full flag to report -ENFILE so the abort path safely unwinds the set to its previous state. As for element updates, decrement set->nelems to restore it. A simpler fix is to call synchronize_rcu() in the error path. However, with a large batch adding elements to already maxed-out set, this could cause noticeable slowdown of such batches. Fixes: 35d0ac9 ("netfilter: nf_tables: fix set->nelems counting with no NLM_F_EXCL") Reported-by: Inseo An <y0un9sa@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent b85a1ac commit ccb8c8f

1 file changed

Lines changed: 16 additions & 14 deletions

File tree

net/netfilter/nf_tables_api.c

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7169,6 +7169,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
71697169
struct nft_data_desc desc;
71707170
enum nft_registers dreg;
71717171
struct nft_trans *trans;
7172+
bool set_full = false;
71727173
u64 expiration;
71737174
u64 timeout;
71747175
int err, i;
@@ -7455,10 +7456,18 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
74557456
if (err < 0)
74567457
goto err_elem_free;
74577458

7459+
if (!(flags & NFT_SET_ELEM_CATCHALL)) {
7460+
unsigned int max = nft_set_maxsize(set), nelems;
7461+
7462+
nelems = atomic_inc_return(&set->nelems);
7463+
if (nelems > max)
7464+
set_full = true;
7465+
}
7466+
74587467
trans = nft_trans_elem_alloc(ctx, NFT_MSG_NEWSETELEM, set);
74597468
if (trans == NULL) {
74607469
err = -ENOMEM;
7461-
goto err_elem_free;
7470+
goto err_set_size;
74627471
}
74637472

74647473
ext->genmask = nft_genmask_cur(ctx->net);
@@ -7510,7 +7519,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
75107519

75117520
ue->priv = elem_priv;
75127521
nft_trans_commit_list_add_elem(ctx->net, trans);
7513-
goto err_elem_free;
7522+
goto err_set_size;
75147523
}
75157524
}
75167525
}
@@ -7528,23 +7537,16 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
75287537
goto err_element_clash;
75297538
}
75307539

7531-
if (!(flags & NFT_SET_ELEM_CATCHALL)) {
7532-
unsigned int max = nft_set_maxsize(set);
7533-
7534-
if (!atomic_add_unless(&set->nelems, 1, max)) {
7535-
err = -ENFILE;
7536-
goto err_set_full;
7537-
}
7538-
}
7539-
75407540
nft_trans_container_elem(trans)->elems[0].priv = elem.priv;
75417541
nft_trans_commit_list_add_elem(ctx->net, trans);
7542-
return 0;
75437542

7544-
err_set_full:
7545-
nft_setelem_remove(ctx->net, set, elem.priv);
7543+
return set_full ? -ENFILE : 0;
7544+
75467545
err_element_clash:
75477546
kfree(trans);
7547+
err_set_size:
7548+
if (!(flags & NFT_SET_ELEM_CATCHALL))
7549+
atomic_dec(&set->nelems);
75487550
err_elem_free:
75497551
nf_tables_set_elem_destroy(ctx, set, elem.priv);
75507552
err_parse_data:

0 commit comments

Comments
 (0)