Skip to content

Commit e564400

Browse files
fw-strlenfw-strlen
committed
netfilter: nf_tables: drop map element references from preparation phase
JIRA: https://issues.redhat.com/browse/RHEL-1720 JIRA: https://issues.redhat.com/browse/RHEL-1721 Upstream Status: commit 628bd3e commit 628bd3e Author: Pablo Neira Ayuso <pablo@netfilter.org> Date: Fri Jun 16 14:51:49 2023 +0200 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> Signed-off-by: Florian Westphal <fwestpha@redhat.com>
1 parent f74ccdb commit e564400

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
@@ -435,7 +435,8 @@ struct nft_set_ops {
435435
int (*init)(const struct nft_set *set,
436436
const struct nft_set_desc *desc,
437437
const struct nlattr * const nla[]);
438-
void (*destroy)(const struct nft_set *set);
438+
void (*destroy)(const struct nft_ctx *ctx,
439+
const struct nft_set *set);
439440
void (*gc_init)(const struct nft_set *set);
440441

441442
unsigned int elemsize;
@@ -772,6 +773,8 @@ int nft_set_elem_expr_clone(const struct nft_ctx *ctx, struct nft_set *set,
772773
struct nft_expr *expr_array[]);
773774
void nft_set_elem_destroy(const struct nft_set *set, void *elem,
774775
bool destroy_expr);
776+
void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
777+
const struct nft_set *set, void *elem);
775778

776779
/**
777780
* 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
@@ -561,6 +561,58 @@ static int nft_trans_set_add(const struct nft_ctx *ctx, int msg_type,
561561
return __nft_trans_set_add(ctx, msg_type, set, NULL);
562562
}
563563

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

624+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
625+
nft_map_deactivate(ctx, set);
626+
572627
nft_deactivate_next(ctx->net, set);
573628
ctx->table->use--;
574629

@@ -3413,12 +3468,6 @@ int nft_setelem_validate(const struct nft_ctx *ctx, struct nft_set *set,
34133468
return 0;
34143469
}
34153470

3416-
struct nft_set_elem_catchall {
3417-
struct list_head list;
3418-
struct rcu_head rcu;
3419-
void *elem;
3420-
};
3421-
34223471
int nft_set_catchall_validate(const struct nft_ctx *ctx, struct nft_set *set)
34233472
{
34243473
u8 genmask = nft_genmask_next(ctx->net);
@@ -4747,7 +4796,7 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info,
47474796
for (i = 0; i < set->num_exprs; i++)
47484797
nft_expr_destroy(&ctx, set->exprs[i]);
47494798
err_set_destroy:
4750-
ops->destroy(set);
4799+
ops->destroy(&ctx, set);
47514800
err_set_init:
47524801
kfree(set->name);
47534802
err_set_name:
@@ -4762,7 +4811,7 @@ static void nft_set_catchall_destroy(const struct nft_ctx *ctx,
47624811

47634812
list_for_each_entry_safe(catchall, next, &set->catchall_list, list) {
47644813
list_del_rcu(&catchall->list);
4765-
nft_set_elem_destroy(set, catchall->elem, true);
4814+
nf_tables_set_elem_destroy(ctx, set, catchall->elem);
47664815
kfree_rcu(catchall, rcu);
47674816
}
47684817
}
@@ -4777,7 +4826,7 @@ static void nft_set_destroy(const struct nft_ctx *ctx, struct nft_set *set)
47774826
for (i = 0; i < set->num_exprs; i++)
47784827
nft_expr_destroy(ctx, set->exprs[i]);
47794828

4780-
set->ops->destroy(set);
4829+
set->ops->destroy(ctx, set);
47814830
nft_set_catchall_destroy(ctx, set);
47824831
kfree(set->name);
47834832
kvfree(set);
@@ -4942,10 +4991,60 @@ static void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
49424991
}
49434992
}
49444993

4994+
static void nft_setelem_data_activate(const struct net *net,
4995+
const struct nft_set *set,
4996+
struct nft_set_elem *elem);
4997+
4998+
static int nft_mapelem_activate(const struct nft_ctx *ctx,
4999+
struct nft_set *set,
5000+
const struct nft_set_iter *iter,
5001+
struct nft_set_elem *elem)
5002+
{
5003+
nft_setelem_data_activate(ctx->net, set, elem);
5004+
5005+
return 0;
5006+
}
5007+
5008+
static void nft_map_catchall_activate(const struct nft_ctx *ctx,
5009+
struct nft_set *set)
5010+
{
5011+
u8 genmask = nft_genmask_next(ctx->net);
5012+
struct nft_set_elem_catchall *catchall;
5013+
struct nft_set_elem elem;
5014+
struct nft_set_ext *ext;
5015+
5016+
list_for_each_entry(catchall, &set->catchall_list, list) {
5017+
ext = nft_set_elem_ext(set, catchall->elem);
5018+
if (!nft_set_elem_active(ext, genmask))
5019+
continue;
5020+
5021+
elem.priv = catchall->elem;
5022+
nft_setelem_data_activate(ctx->net, set, &elem);
5023+
break;
5024+
}
5025+
}
5026+
5027+
static void nft_map_activate(const struct nft_ctx *ctx, struct nft_set *set)
5028+
{
5029+
struct nft_set_iter iter = {
5030+
.genmask = nft_genmask_next(ctx->net),
5031+
.fn = nft_mapelem_activate,
5032+
};
5033+
5034+
set->ops->walk(ctx, set, &iter);
5035+
WARN_ON_ONCE(iter.err);
5036+
5037+
nft_map_catchall_activate(ctx, set);
5038+
}
5039+
49455040
void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set)
49465041
{
4947-
if (nft_set_is_anonymous(set))
5042+
if (nft_set_is_anonymous(set)) {
5043+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5044+
nft_map_activate(ctx, set);
5045+
49485046
nft_clear(ctx->net, set);
5047+
}
49495048

49505049
set->use++;
49515050
}
@@ -4966,13 +5065,20 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
49665065
set->use--;
49675066
break;
49685067
case NFT_TRANS_PREPARE:
4969-
if (nft_set_is_anonymous(set))
4970-
nft_deactivate_next(ctx->net, set);
5068+
if (nft_set_is_anonymous(set)) {
5069+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5070+
nft_map_deactivate(ctx, set);
49715071

5072+
nft_deactivate_next(ctx->net, set);
5073+
}
49725074
set->use--;
49735075
return;
49745076
case NFT_TRANS_ABORT:
49755077
case NFT_TRANS_RELEASE:
5078+
if (nft_set_is_anonymous(set) &&
5079+
set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5080+
nft_map_deactivate(ctx, set);
5081+
49765082
set->use--;
49775083
fallthrough;
49785084
default:
@@ -5747,6 +5853,7 @@ static void nft_set_elem_expr_destroy(const struct nft_ctx *ctx,
57475853
__nft_set_elem_expr_destroy(ctx, expr);
57485854
}
57495855

5856+
/* Drop references and destroy. Called from gc, dynset and abort path. */
57505857
void nft_set_elem_destroy(const struct nft_set *set, void *elem,
57515858
bool destroy_expr)
57525859
{
@@ -5768,11 +5875,11 @@ void nft_set_elem_destroy(const struct nft_set *set, void *elem,
57685875
}
57695876
EXPORT_SYMBOL_GPL(nft_set_elem_destroy);
57705877

5771-
/* Only called from commit path, nft_setelem_data_deactivate() already deals
5772-
* with the refcounting from the preparation phase.
5878+
/* Destroy element. References have been already dropped in the preparation
5879+
* path via nft_setelem_data_deactivate().
57735880
*/
5774-
static void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
5775-
const struct nft_set *set, void *elem)
5881+
void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
5882+
const struct nft_set *set, void *elem)
57765883
{
57775884
struct nft_set_ext *ext = nft_set_elem_ext(set, elem);
57785885

@@ -6405,7 +6512,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
64056512
if (obj)
64066513
obj->use--;
64076514
err_elem_userdata:
6408-
nf_tables_set_elem_destroy(ctx, set, elem.priv);
6515+
nft_set_elem_destroy(set, elem.priv, true);
64096516
err_parse_data:
64106517
if (nla[NFTA_SET_ELEM_DATA] != NULL)
64116518
nft_data_release(&elem.data.val, desc.type);
@@ -9481,6 +9588,9 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
94819588
case NFT_MSG_DESTROYSET:
94829589
trans->ctx.table->use++;
94839590
nft_clear(trans->ctx.net, nft_trans_set(trans));
9591+
if (nft_trans_set(trans)->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
9592+
nft_map_activate(&trans->ctx, nft_trans_set(trans));
9593+
94849594
nft_trans_destroy(trans);
94859595
break;
94869596
case NFT_MSG_NEWSETELEM:
@@ -10251,6 +10361,9 @@ static void __nft_release_table(struct net *net, struct nft_table *table)
1025110361
list_for_each_entry_safe(set, ns, &table->sets, list) {
1025210362
list_del(&set->list);
1025310363
table->use--;
10364+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
10365+
nft_map_deactivate(&ctx, set);
10366+
1025410367
nft_set_destroy(&ctx, set);
1025510368
}
1025610369
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
@@ -2156,10 +2156,12 @@ static int nft_pipapo_init(const struct nft_set *set,
21562156

21572157
/**
21582158
* nft_set_pipapo_match_destroy() - Destroy elements from key mapping array
2159+
* @ctx: context
21592160
* @set: nftables API set representation
21602161
* @m: matching data pointing to key mapping array
21612162
*/
2162-
static void nft_set_pipapo_match_destroy(const struct nft_set *set,
2163+
static void nft_set_pipapo_match_destroy(const struct nft_ctx *ctx,
2164+
const struct nft_set *set,
21632165
struct nft_pipapo_match *m)
21642166
{
21652167
struct nft_pipapo_field *f;
@@ -2176,15 +2178,17 @@ static void nft_set_pipapo_match_destroy(const struct nft_set *set,
21762178

21772179
e = f->mt[r].e;
21782180

2179-
nft_set_elem_destroy(set, e, true);
2181+
nf_tables_set_elem_destroy(ctx, set, e);
21802182
}
21812183
}
21822184

21832185
/**
21842186
* nft_pipapo_destroy() - Free private data for set and all committed elements
2187+
* @ctx: context
21852188
* @set: nftables API set representation
21862189
*/
2187-
static void nft_pipapo_destroy(const struct nft_set *set)
2190+
static void nft_pipapo_destroy(const struct nft_ctx *ctx,
2191+
const struct nft_set *set)
21882192
{
21892193
struct nft_pipapo *priv = nft_set_priv(set);
21902194
struct nft_pipapo_match *m;
@@ -2194,7 +2198,7 @@ static void nft_pipapo_destroy(const struct nft_set *set)
21942198
if (m) {
21952199
rcu_barrier();
21962200

2197-
nft_set_pipapo_match_destroy(set, m);
2201+
nft_set_pipapo_match_destroy(ctx, set, m);
21982202

21992203
#ifdef NFT_PIPAPO_ALIGN
22002204
free_percpu(m->scratch_aligned);
@@ -2211,7 +2215,7 @@ static void nft_pipapo_destroy(const struct nft_set *set)
22112215
m = priv->clone;
22122216

22132217
if (priv->dirty)
2214-
nft_set_pipapo_match_destroy(set, m);
2218+
nft_set_pipapo_match_destroy(ctx, set, m);
22152219

22162220
#ifdef NFT_PIPAPO_ALIGN
22172221
free_percpu(priv->clone->scratch_aligned);

0 commit comments

Comments
 (0)