Skip to content

Commit 628bd3e

Browse files
ummakynesummakynes
committed
netfilter: nf_tables: drop map element references from preparation phase
set .destroy callback releases the references to other objects in maps. This is very late and it results in spurious EBUSY errors. Drop refcount from the preparation phase instead, update set backend not to drop reference counter from set .destroy path. Exceptions: NFT_TRANS_PREPARE_ERROR does not require to drop the reference counter because the transaction abort path releases the map references for each element since the set is unbound. The abort path also deals with releasing reference counter for new elements added to unbound sets. Fixes: 5910544 ("netfilter: nf_tables: revisit chain/object refcounting from elements") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
1 parent 26b5a57 commit 628bd3e

File tree

6 files changed

+167
-32
lines changed

6 files changed

+167
-32
lines changed

‎include/net/netfilter/nf_tables.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,8 @@ struct nft_set_ops {
472472
int (*init)(const struct nft_set *set,
473473
const struct nft_set_desc *desc,
474474
const struct nlattr * const nla[]);
475-
void (*destroy)(const struct nft_set *set);
475+
void (*destroy)(const struct nft_ctx *ctx,
476+
const struct nft_set *set);
476477
void (*gc_init)(const struct nft_set *set);
477478

478479
unsigned int elemsize;
@@ -809,6 +810,8 @@ int nft_set_elem_expr_clone(const struct nft_ctx *ctx, struct nft_set *set,
809810
struct nft_expr *expr_array[]);
810811
void nft_set_elem_destroy(const struct nft_set *set, void *elem,
811812
bool destroy_expr);
813+
void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
814+
const struct nft_set *set, void *elem);
812815

813816
/**
814817
* struct nft_set_gc_batch_head - nf_tables set garbage collection batch

‎net/netfilter/nf_tables_api.c‎

Lines changed: 130 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,58 @@ static int nft_trans_set_add(const struct nft_ctx *ctx, int msg_type,
559559
return __nft_trans_set_add(ctx, msg_type, set, NULL);
560560
}
561561

562+
static void nft_setelem_data_deactivate(const struct net *net,
563+
const struct nft_set *set,
564+
struct nft_set_elem *elem);
565+
566+
static int nft_mapelem_deactivate(const struct nft_ctx *ctx,
567+
struct nft_set *set,
568+
const struct nft_set_iter *iter,
569+
struct nft_set_elem *elem)
570+
{
571+
nft_setelem_data_deactivate(ctx->net, set, elem);
572+
573+
return 0;
574+
}
575+
576+
struct nft_set_elem_catchall {
577+
struct list_head list;
578+
struct rcu_head rcu;
579+
void *elem;
580+
};
581+
582+
static void nft_map_catchall_deactivate(const struct nft_ctx *ctx,
583+
struct nft_set *set)
584+
{
585+
u8 genmask = nft_genmask_next(ctx->net);
586+
struct nft_set_elem_catchall *catchall;
587+
struct nft_set_elem elem;
588+
struct nft_set_ext *ext;
589+
590+
list_for_each_entry(catchall, &set->catchall_list, list) {
591+
ext = nft_set_elem_ext(set, catchall->elem);
592+
if (!nft_set_elem_active(ext, genmask))
593+
continue;
594+
595+
elem.priv = catchall->elem;
596+
nft_setelem_data_deactivate(ctx->net, set, &elem);
597+
break;
598+
}
599+
}
600+
601+
static void nft_map_deactivate(const struct nft_ctx *ctx, struct nft_set *set)
602+
{
603+
struct nft_set_iter iter = {
604+
.genmask = nft_genmask_next(ctx->net),
605+
.fn = nft_mapelem_deactivate,
606+
};
607+
608+
set->ops->walk(ctx, set, &iter);
609+
WARN_ON_ONCE(iter.err);
610+
611+
nft_map_catchall_deactivate(ctx, set);
612+
}
613+
562614
static int nft_delset(const struct nft_ctx *ctx, struct nft_set *set)
563615
{
564616
int err;
@@ -567,6 +619,9 @@ static int nft_delset(const struct nft_ctx *ctx, struct nft_set *set)
567619
if (err < 0)
568620
return err;
569621

622+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
623+
nft_map_deactivate(ctx, set);
624+
570625
nft_deactivate_next(ctx->net, set);
571626
ctx->table->use--;
572627

@@ -3659,12 +3714,6 @@ int nft_setelem_validate(const struct nft_ctx *ctx, struct nft_set *set,
36593714
return 0;
36603715
}
36613716

3662-
struct nft_set_elem_catchall {
3663-
struct list_head list;
3664-
struct rcu_head rcu;
3665-
void *elem;
3666-
};
3667-
36683717
int nft_set_catchall_validate(const struct nft_ctx *ctx, struct nft_set *set)
36693718
{
36703719
u8 genmask = nft_genmask_next(ctx->net);
@@ -4997,7 +5046,7 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info,
49975046
for (i = 0; i < set->num_exprs; i++)
49985047
nft_expr_destroy(&ctx, set->exprs[i]);
49995048
err_set_destroy:
5000-
ops->destroy(set);
5049+
ops->destroy(&ctx, set);
50015050
err_set_init:
50025051
kfree(set->name);
50035052
err_set_name:
@@ -5012,7 +5061,7 @@ static void nft_set_catchall_destroy(const struct nft_ctx *ctx,
50125061

50135062
list_for_each_entry_safe(catchall, next, &set->catchall_list, list) {
50145063
list_del_rcu(&catchall->list);
5015-
nft_set_elem_destroy(set, catchall->elem, true);
5064+
nf_tables_set_elem_destroy(ctx, set, catchall->elem);
50165065
kfree_rcu(catchall, rcu);
50175066
}
50185067
}
@@ -5027,7 +5076,7 @@ static void nft_set_destroy(const struct nft_ctx *ctx, struct nft_set *set)
50275076
for (i = 0; i < set->num_exprs; i++)
50285077
nft_expr_destroy(ctx, set->exprs[i]);
50295078

5030-
set->ops->destroy(set);
5079+
set->ops->destroy(ctx, set);
50315080
nft_set_catchall_destroy(ctx, set);
50325081
kfree(set->name);
50335082
kvfree(set);
@@ -5192,10 +5241,60 @@ static void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
51925241
}
51935242
}
51945243

5244+
static void nft_setelem_data_activate(const struct net *net,
5245+
const struct nft_set *set,
5246+
struct nft_set_elem *elem);
5247+
5248+
static int nft_mapelem_activate(const struct nft_ctx *ctx,
5249+
struct nft_set *set,
5250+
const struct nft_set_iter *iter,
5251+
struct nft_set_elem *elem)
5252+
{
5253+
nft_setelem_data_activate(ctx->net, set, elem);
5254+
5255+
return 0;
5256+
}
5257+
5258+
static void nft_map_catchall_activate(const struct nft_ctx *ctx,
5259+
struct nft_set *set)
5260+
{
5261+
u8 genmask = nft_genmask_next(ctx->net);
5262+
struct nft_set_elem_catchall *catchall;
5263+
struct nft_set_elem elem;
5264+
struct nft_set_ext *ext;
5265+
5266+
list_for_each_entry(catchall, &set->catchall_list, list) {
5267+
ext = nft_set_elem_ext(set, catchall->elem);
5268+
if (!nft_set_elem_active(ext, genmask))
5269+
continue;
5270+
5271+
elem.priv = catchall->elem;
5272+
nft_setelem_data_activate(ctx->net, set, &elem);
5273+
break;
5274+
}
5275+
}
5276+
5277+
static void nft_map_activate(const struct nft_ctx *ctx, struct nft_set *set)
5278+
{
5279+
struct nft_set_iter iter = {
5280+
.genmask = nft_genmask_next(ctx->net),
5281+
.fn = nft_mapelem_activate,
5282+
};
5283+
5284+
set->ops->walk(ctx, set, &iter);
5285+
WARN_ON_ONCE(iter.err);
5286+
5287+
nft_map_catchall_activate(ctx, set);
5288+
}
5289+
51955290
void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set)
51965291
{
5197-
if (nft_set_is_anonymous(set))
5292+
if (nft_set_is_anonymous(set)) {
5293+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5294+
nft_map_activate(ctx, set);
5295+
51985296
nft_clear(ctx->net, set);
5297+
}
51995298

52005299
set->use++;
52015300
}
@@ -5214,13 +5313,20 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
52145313
set->use--;
52155314
break;
52165315
case NFT_TRANS_PREPARE:
5217-
if (nft_set_is_anonymous(set))
5218-
nft_deactivate_next(ctx->net, set);
5316+
if (nft_set_is_anonymous(set)) {
5317+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5318+
nft_map_deactivate(ctx, set);
52195319

5320+
nft_deactivate_next(ctx->net, set);
5321+
}
52205322
set->use--;
52215323
return;
52225324
case NFT_TRANS_ABORT:
52235325
case NFT_TRANS_RELEASE:
5326+
if (nft_set_is_anonymous(set) &&
5327+
set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5328+
nft_map_deactivate(ctx, set);
5329+
52245330
set->use--;
52255331
fallthrough;
52265332
default:
@@ -5973,6 +6079,7 @@ static void nft_set_elem_expr_destroy(const struct nft_ctx *ctx,
59736079
__nft_set_elem_expr_destroy(ctx, expr);
59746080
}
59756081

6082+
/* Drop references and destroy. Called from gc, dynset and abort path. */
59766083
void nft_set_elem_destroy(const struct nft_set *set, void *elem,
59776084
bool destroy_expr)
59786085
{
@@ -5994,11 +6101,11 @@ void nft_set_elem_destroy(const struct nft_set *set, void *elem,
59946101
}
59956102
EXPORT_SYMBOL_GPL(nft_set_elem_destroy);
59966103

5997-
/* Only called from commit path, nft_setelem_data_deactivate() already deals
5998-
* with the refcounting from the preparation phase.
6104+
/* Destroy element. References have been already dropped in the preparation
6105+
* path via nft_setelem_data_deactivate().
59996106
*/
6000-
static void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
6001-
const struct nft_set *set, void *elem)
6107+
void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
6108+
const struct nft_set *set, void *elem)
60026109
{
60036110
struct nft_set_ext *ext = nft_set_elem_ext(set, elem);
60046111

@@ -6631,7 +6738,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
66316738
if (obj)
66326739
obj->use--;
66336740
err_elem_userdata:
6634-
nf_tables_set_elem_destroy(ctx, set, elem.priv);
6741+
nft_set_elem_destroy(set, elem.priv, true);
66356742
err_parse_data:
66366743
if (nla[NFTA_SET_ELEM_DATA] != NULL)
66376744
nft_data_release(&elem.data.val, desc.type);
@@ -9799,6 +9906,9 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
97999906
case NFT_MSG_DESTROYSET:
98009907
trans->ctx.table->use++;
98019908
nft_clear(trans->ctx.net, nft_trans_set(trans));
9909+
if (nft_trans_set(trans)->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
9910+
nft_map_activate(&trans->ctx, nft_trans_set(trans));
9911+
98029912
nft_trans_destroy(trans);
98039913
break;
98049914
case NFT_MSG_NEWSETELEM:
@@ -10568,6 +10678,9 @@ static void __nft_release_table(struct net *net, struct nft_table *table)
1056810678
list_for_each_entry_safe(set, ns, &table->sets, list) {
1056910679
list_del(&set->list);
1057010680
table->use--;
10681+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
10682+
nft_map_deactivate(&ctx, set);
10683+
1057110684
nft_set_destroy(&ctx, set);
1057210685
}
1057310686
list_for_each_entry_safe(obj, ne, &table->objects, list) {

‎net/netfilter/nft_set_bitmap.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,13 +271,14 @@ static int nft_bitmap_init(const struct nft_set *set,
271271
return 0;
272272
}
273273

274-
static void nft_bitmap_destroy(const struct nft_set *set)
274+
static void nft_bitmap_destroy(const struct nft_ctx *ctx,
275+
const struct nft_set *set)
275276
{
276277
struct nft_bitmap *priv = nft_set_priv(set);
277278
struct nft_bitmap_elem *be, *n;
278279

279280
list_for_each_entry_safe(be, n, &priv->list, head)
280-
nft_set_elem_destroy(set, be, true);
281+
nf_tables_set_elem_destroy(ctx, set, be);
281282
}
282283

283284
static bool nft_bitmap_estimate(const struct nft_set_desc *desc, u32 features,

‎net/netfilter/nft_set_hash.c‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -400,19 +400,31 @@ static int nft_rhash_init(const struct nft_set *set,
400400
return 0;
401401
}
402402

403+
struct nft_rhash_ctx {
404+
const struct nft_ctx ctx;
405+
const struct nft_set *set;
406+
};
407+
403408
static void nft_rhash_elem_destroy(void *ptr, void *arg)
404409
{
405-
nft_set_elem_destroy(arg, ptr, true);
410+
struct nft_rhash_ctx *rhash_ctx = arg;
411+
412+
nf_tables_set_elem_destroy(&rhash_ctx->ctx, rhash_ctx->set, ptr);
406413
}
407414

408-
static void nft_rhash_destroy(const struct nft_set *set)
415+
static void nft_rhash_destroy(const struct nft_ctx *ctx,
416+
const struct nft_set *set)
409417
{
410418
struct nft_rhash *priv = nft_set_priv(set);
419+
struct nft_rhash_ctx rhash_ctx = {
420+
.ctx = *ctx,
421+
.set = set,
422+
};
411423

412424
cancel_delayed_work_sync(&priv->gc_work);
413425
rcu_barrier();
414426
rhashtable_free_and_destroy(&priv->ht, nft_rhash_elem_destroy,
415-
(void *)set);
427+
(void *)&rhash_ctx);
416428
}
417429

418430
/* Number of buckets is stored in u32, so cap our result to 1U<<31 */
@@ -643,7 +655,8 @@ static int nft_hash_init(const struct nft_set *set,
643655
return 0;
644656
}
645657

646-
static void nft_hash_destroy(const struct nft_set *set)
658+
static void nft_hash_destroy(const struct nft_ctx *ctx,
659+
const struct nft_set *set)
647660
{
648661
struct nft_hash *priv = nft_set_priv(set);
649662
struct nft_hash_elem *he;
@@ -653,7 +666,7 @@ static void nft_hash_destroy(const struct nft_set *set)
653666
for (i = 0; i < priv->buckets; i++) {
654667
hlist_for_each_entry_safe(he, next, &priv->table[i], node) {
655668
hlist_del_rcu(&he->node);
656-
nft_set_elem_destroy(set, he, true);
669+
nf_tables_set_elem_destroy(ctx, set, he);
657670
}
658671
}
659672
}

‎net/netfilter/nft_set_pipapo.c‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2148,10 +2148,12 @@ static int nft_pipapo_init(const struct nft_set *set,
21482148

21492149
/**
21502150
* nft_set_pipapo_match_destroy() - Destroy elements from key mapping array
2151+
* @ctx: context
21512152
* @set: nftables API set representation
21522153
* @m: matching data pointing to key mapping array
21532154
*/
2154-
static void nft_set_pipapo_match_destroy(const struct nft_set *set,
2155+
static void nft_set_pipapo_match_destroy(const struct nft_ctx *ctx,
2156+
const struct nft_set *set,
21552157
struct nft_pipapo_match *m)
21562158
{
21572159
struct nft_pipapo_field *f;
@@ -2168,15 +2170,17 @@ static void nft_set_pipapo_match_destroy(const struct nft_set *set,
21682170

21692171
e = f->mt[r].e;
21702172

2171-
nft_set_elem_destroy(set, e, true);
2173+
nf_tables_set_elem_destroy(ctx, set, e);
21722174
}
21732175
}
21742176

21752177
/**
21762178
* nft_pipapo_destroy() - Free private data for set and all committed elements
2179+
* @ctx: context
21772180
* @set: nftables API set representation
21782181
*/
2179-
static void nft_pipapo_destroy(const struct nft_set *set)
2182+
static void nft_pipapo_destroy(const struct nft_ctx *ctx,
2183+
const struct nft_set *set)
21802184
{
21812185
struct nft_pipapo *priv = nft_set_priv(set);
21822186
struct nft_pipapo_match *m;
@@ -2186,7 +2190,7 @@ static void nft_pipapo_destroy(const struct nft_set *set)
21862190
if (m) {
21872191
rcu_barrier();
21882192

2189-
nft_set_pipapo_match_destroy(set, m);
2193+
nft_set_pipapo_match_destroy(ctx, set, m);
21902194

21912195
#ifdef NFT_PIPAPO_ALIGN
21922196
free_percpu(m->scratch_aligned);
@@ -2203,7 +2207,7 @@ static void nft_pipapo_destroy(const struct nft_set *set)
22032207
m = priv->clone;
22042208

22052209
if (priv->dirty)
2206-
nft_set_pipapo_match_destroy(set, m);
2210+
nft_set_pipapo_match_destroy(ctx, set, m);
22072211

22082212
#ifdef NFT_PIPAPO_ALIGN
22092213
free_percpu(priv->clone->scratch_aligned);

0 commit comments

Comments
 (0)