Skip to content

Commit def602e

Browse files
ummakynesFlorian Westphal
authored andcommitted
netfilter: nf_tables: unconditionally bump set->nelems before insertion
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>
1 parent b824c3e commit def602e

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
@@ -7170,6 +7170,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
71707170
struct nft_data_desc desc;
71717171
enum nft_registers dreg;
71727172
struct nft_trans *trans;
7173+
bool set_full = false;
71737174
u64 expiration;
71747175
u64 timeout;
71757176
int err, i;
@@ -7461,10 +7462,18 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
74617462
if (err < 0)
74627463
goto err_elem_free;
74637464

7465+
if (!(flags & NFT_SET_ELEM_CATCHALL)) {
7466+
unsigned int max = nft_set_maxsize(set), nelems;
7467+
7468+
nelems = atomic_inc_return(&set->nelems);
7469+
if (nelems > max)
7470+
set_full = true;
7471+
}
7472+
74647473
trans = nft_trans_elem_alloc(ctx, NFT_MSG_NEWSETELEM, set);
74657474
if (trans == NULL) {
74667475
err = -ENOMEM;
7467-
goto err_elem_free;
7476+
goto err_set_size;
74687477
}
74697478

74707479
ext->genmask = nft_genmask_cur(ctx->net);
@@ -7516,7 +7525,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
75167525

75177526
ue->priv = elem_priv;
75187527
nft_trans_commit_list_add_elem(ctx->net, trans);
7519-
goto err_elem_free;
7528+
goto err_set_size;
75207529
}
75217530
}
75227531
}
@@ -7534,23 +7543,16 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
75347543
goto err_element_clash;
75357544
}
75367545

7537-
if (!(flags & NFT_SET_ELEM_CATCHALL)) {
7538-
unsigned int max = nft_set_maxsize(set);
7539-
7540-
if (!atomic_add_unless(&set->nelems, 1, max)) {
7541-
err = -ENFILE;
7542-
goto err_set_full;
7543-
}
7544-
}
7545-
75467546
nft_trans_container_elem(trans)->elems[0].priv = elem.priv;
75477547
nft_trans_commit_list_add_elem(ctx->net, trans);
7548-
return 0;
75497548

7550-
err_set_full:
7551-
nft_setelem_remove(ctx->net, set, elem.priv);
7549+
return set_full ? -ENFILE : 0;
7550+
75527551
err_element_clash:
75537552
kfree(trans);
7553+
err_set_size:
7554+
if (!(flags & NFT_SET_ELEM_CATCHALL))
7555+
atomic_dec(&set->nelems);
75547556
err_elem_free:
75557557
nf_tables_set_elem_destroy(ctx, set, elem.priv);
75567558
err_parse_data:

0 commit comments

Comments
 (0)