Skip to content

Commit 6826131

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 6624d17 commit 6826131

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
@@ -7288,6 +7288,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
72887288
struct nft_data_desc desc;
72897289
enum nft_registers dreg;
72907290
struct nft_trans *trans;
7291+
bool set_full = false;
72917292
u64 expiration;
72927293
u64 timeout;
72937294
int err, i;
@@ -7574,10 +7575,18 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
75747575
if (err < 0)
75757576
goto err_elem_free;
75767577

7578+
if (!(flags & NFT_SET_ELEM_CATCHALL)) {
7579+
unsigned int max = nft_set_maxsize(set), nelems;
7580+
7581+
nelems = atomic_inc_return(&set->nelems);
7582+
if (nelems > max)
7583+
set_full = true;
7584+
}
7585+
75777586
trans = nft_trans_elem_alloc(ctx, NFT_MSG_NEWSETELEM, set);
75787587
if (trans == NULL) {
75797588
err = -ENOMEM;
7580-
goto err_elem_free;
7589+
goto err_set_size;
75817590
}
75827591

75837592
ext->genmask = nft_genmask_cur(ctx->net);
@@ -7629,7 +7638,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
76297638

76307639
ue->priv = elem_priv;
76317640
nft_trans_commit_list_add_elem(ctx->net, trans);
7632-
goto err_elem_free;
7641+
goto err_set_size;
76337642
}
76347643
}
76357644
}
@@ -7647,23 +7656,16 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
76477656
goto err_element_clash;
76487657
}
76497658

7650-
if (!(flags & NFT_SET_ELEM_CATCHALL)) {
7651-
unsigned int max = nft_set_maxsize(set);
7652-
7653-
if (!atomic_add_unless(&set->nelems, 1, max)) {
7654-
err = -ENFILE;
7655-
goto err_set_full;
7656-
}
7657-
}
7658-
76597659
nft_trans_container_elem(trans)->elems[0].priv = elem.priv;
76607660
nft_trans_commit_list_add_elem(ctx->net, trans);
7661-
return 0;
76627661

7663-
err_set_full:
7664-
nft_setelem_remove(ctx->net, set, elem.priv);
7662+
return set_full ? -ENFILE : 0;
7663+
76657664
err_element_clash:
76667665
kfree(trans);
7666+
err_set_size:
7667+
if (!(flags & NFT_SET_ELEM_CATCHALL))
7668+
atomic_dec(&set->nelems);
76677669
err_elem_free:
76687670
nf_tables_set_elem_destroy(ctx, set, elem.priv);
76697671
err_parse_data:

0 commit comments

Comments
 (0)