Skip to content

Commit f2005ca

Browse files
ummakynessmb49
ummakynes
authored and
smb49
committed
netfilter: nf_tables: drop map element references from preparation phase
BugLink: https://bugs.launchpad.net/bugs/2032689 commit 628bd3e upstream. 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: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Kamal Mostafa <kamal@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
1 parent 295bad8 commit f2005ca

File tree

6 files changed

+166
-31
lines changed

6 files changed

+166
-31
lines changed

‎include/net/netfilter/nf_tables.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,8 @@ struct nft_set_ops {
437437
int (*init)(const struct nft_set *set,
438438
const struct nft_set_desc *desc,
439439
const struct nlattr * const nla[]);
440-
void (*destroy)(const struct nft_set *set);
440+
void (*destroy)(const struct nft_ctx *ctx,
441+
const struct nft_set *set);
441442
void (*gc_init)(const struct nft_set *set);
442443

443444
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: 129 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,58 @@ static int nft_trans_set_add(const struct nft_ctx *ctx, int msg_type,
581581
return __nft_trans_set_add(ctx, msg_type, set, NULL);
582582
}
583583

584+
static void nft_setelem_data_deactivate(const struct net *net,
585+
const struct nft_set *set,
586+
struct nft_set_elem *elem);
587+
588+
static int nft_mapelem_deactivate(const struct nft_ctx *ctx,
589+
struct nft_set *set,
590+
const struct nft_set_iter *iter,
591+
struct nft_set_elem *elem)
592+
{
593+
nft_setelem_data_deactivate(ctx->net, set, elem);
594+
595+
return 0;
596+
}
597+
598+
struct nft_set_elem_catchall {
599+
struct list_head list;
600+
struct rcu_head rcu;
601+
void *elem;
602+
};
603+
604+
static void nft_map_catchall_deactivate(const struct nft_ctx *ctx,
605+
struct nft_set *set)
606+
{
607+
u8 genmask = nft_genmask_next(ctx->net);
608+
struct nft_set_elem_catchall *catchall;
609+
struct nft_set_elem elem;
610+
struct nft_set_ext *ext;
611+
612+
list_for_each_entry(catchall, &set->catchall_list, list) {
613+
ext = nft_set_elem_ext(set, catchall->elem);
614+
if (!nft_set_elem_active(ext, genmask))
615+
continue;
616+
617+
elem.priv = catchall->elem;
618+
nft_setelem_data_deactivate(ctx->net, set, &elem);
619+
break;
620+
}
621+
}
622+
623+
static void nft_map_deactivate(const struct nft_ctx *ctx, struct nft_set *set)
624+
{
625+
struct nft_set_iter iter = {
626+
.genmask = nft_genmask_next(ctx->net),
627+
.fn = nft_mapelem_deactivate,
628+
};
629+
630+
set->ops->walk(ctx, set, &iter);
631+
WARN_ON_ONCE(iter.err);
632+
633+
nft_map_catchall_deactivate(ctx, set);
634+
}
635+
584636
static int nft_delset(const struct nft_ctx *ctx, struct nft_set *set)
585637
{
586638
int err;
@@ -589,6 +641,9 @@ static int nft_delset(const struct nft_ctx *ctx, struct nft_set *set)
589641
if (err < 0)
590642
return err;
591643

644+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
645+
nft_map_deactivate(ctx, set);
646+
592647
nft_deactivate_next(ctx->net, set);
593648
ctx->table->use--;
594649

@@ -3408,12 +3463,6 @@ int nft_setelem_validate(const struct nft_ctx *ctx, struct nft_set *set,
34083463
return 0;
34093464
}
34103465

3411-
struct nft_set_elem_catchall {
3412-
struct list_head list;
3413-
struct rcu_head rcu;
3414-
void *elem;
3415-
};
3416-
34173466
int nft_set_catchall_validate(const struct nft_ctx *ctx, struct nft_set *set)
34183467
{
34193468
u8 genmask = nft_genmask_next(ctx->net);
@@ -4739,7 +4788,7 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info,
47394788
for (i = 0; i < set->num_exprs; i++)
47404789
nft_expr_destroy(&ctx, set->exprs[i]);
47414790
err_set_destroy:
4742-
ops->destroy(set);
4791+
ops->destroy(&ctx, set);
47434792
err_set_init:
47444793
kfree(set->name);
47454794
err_set_name:
@@ -4754,7 +4803,7 @@ static void nft_set_catchall_destroy(const struct nft_ctx *ctx,
47544803

47554804
list_for_each_entry_safe(catchall, next, &set->catchall_list, list) {
47564805
list_del_rcu(&catchall->list);
4757-
nft_set_elem_destroy(set, catchall->elem, true);
4806+
nf_tables_set_elem_destroy(ctx, set, catchall->elem);
47584807
kfree_rcu(catchall, rcu);
47594808
}
47604809
}
@@ -4769,7 +4818,7 @@ static void nft_set_destroy(const struct nft_ctx *ctx, struct nft_set *set)
47694818
for (i = 0; i < set->num_exprs; i++)
47704819
nft_expr_destroy(ctx, set->exprs[i]);
47714820

4772-
set->ops->destroy(set);
4821+
set->ops->destroy(ctx, set);
47734822
nft_set_catchall_destroy(ctx, set);
47744823
kfree(set->name);
47754824
kvfree(set);
@@ -4930,10 +4979,60 @@ static void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
49304979
}
49314980
}
49324981

4982+
static void nft_setelem_data_activate(const struct net *net,
4983+
const struct nft_set *set,
4984+
struct nft_set_elem *elem);
4985+
4986+
static int nft_mapelem_activate(const struct nft_ctx *ctx,
4987+
struct nft_set *set,
4988+
const struct nft_set_iter *iter,
4989+
struct nft_set_elem *elem)
4990+
{
4991+
nft_setelem_data_activate(ctx->net, set, elem);
4992+
4993+
return 0;
4994+
}
4995+
4996+
static void nft_map_catchall_activate(const struct nft_ctx *ctx,
4997+
struct nft_set *set)
4998+
{
4999+
u8 genmask = nft_genmask_next(ctx->net);
5000+
struct nft_set_elem_catchall *catchall;
5001+
struct nft_set_elem elem;
5002+
struct nft_set_ext *ext;
5003+
5004+
list_for_each_entry(catchall, &set->catchall_list, list) {
5005+
ext = nft_set_elem_ext(set, catchall->elem);
5006+
if (!nft_set_elem_active(ext, genmask))
5007+
continue;
5008+
5009+
elem.priv = catchall->elem;
5010+
nft_setelem_data_activate(ctx->net, set, &elem);
5011+
break;
5012+
}
5013+
}
5014+
5015+
static void nft_map_activate(const struct nft_ctx *ctx, struct nft_set *set)
5016+
{
5017+
struct nft_set_iter iter = {
5018+
.genmask = nft_genmask_next(ctx->net),
5019+
.fn = nft_mapelem_activate,
5020+
};
5021+
5022+
set->ops->walk(ctx, set, &iter);
5023+
WARN_ON_ONCE(iter.err);
5024+
5025+
nft_map_catchall_activate(ctx, set);
5026+
}
5027+
49335028
void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set)
49345029
{
4935-
if (nft_set_is_anonymous(set))
5030+
if (nft_set_is_anonymous(set)) {
5031+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5032+
nft_map_activate(ctx, set);
5033+
49365034
nft_clear(ctx->net, set);
5035+
}
49375036

49385037
set->use++;
49395038
}
@@ -4954,13 +5053,20 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
49545053
set->use--;
49555054
break;
49565055
case NFT_TRANS_PREPARE:
4957-
if (nft_set_is_anonymous(set))
4958-
nft_deactivate_next(ctx->net, set);
5056+
if (nft_set_is_anonymous(set)) {
5057+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5058+
nft_map_deactivate(ctx, set);
49595059

5060+
nft_deactivate_next(ctx->net, set);
5061+
}
49605062
set->use--;
49615063
return;
49625064
case NFT_TRANS_ABORT:
49635065
case NFT_TRANS_RELEASE:
5066+
if (nft_set_is_anonymous(set) &&
5067+
set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5068+
nft_map_deactivate(ctx, set);
5069+
49645070
set->use--;
49655071
fallthrough;
49665072
default:
@@ -5676,6 +5782,7 @@ static void nft_set_elem_expr_destroy(const struct nft_ctx *ctx,
56765782
__nft_set_elem_expr_destroy(ctx, expr);
56775783
}
56785784

5785+
/* Drop references and destroy. Called from gc, dynset and abort path. */
56795786
void nft_set_elem_destroy(const struct nft_set *set, void *elem,
56805787
bool destroy_expr)
56815788
{
@@ -5697,11 +5804,11 @@ void nft_set_elem_destroy(const struct nft_set *set, void *elem,
56975804
}
56985805
EXPORT_SYMBOL_GPL(nft_set_elem_destroy);
56995806

5700-
/* Only called from commit path, nft_setelem_data_deactivate() already deals
5701-
* with the refcounting from the preparation phase.
5807+
/* Destroy element. References have been already dropped in the preparation
5808+
* path via nft_setelem_data_deactivate().
57025809
*/
5703-
static void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
5704-
const struct nft_set *set, void *elem)
5810+
void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
5811+
const struct nft_set *set, void *elem)
57055812
{
57065813
struct nft_set_ext *ext = nft_set_elem_ext(set, elem);
57075814

@@ -9330,6 +9437,9 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
93309437
case NFT_MSG_DELSET:
93319438
trans->ctx.table->use++;
93329439
nft_clear(trans->ctx.net, nft_trans_set(trans));
9440+
if (nft_trans_set(trans)->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
9441+
nft_map_activate(&trans->ctx, nft_trans_set(trans));
9442+
93339443
nft_trans_destroy(trans);
93349444
break;
93359445
case NFT_MSG_NEWSETELEM:
@@ -10097,6 +10207,9 @@ static void __nft_release_table(struct net *net, struct nft_table *table)
1009710207
list_for_each_entry_safe(set, ns, &table->sets, list) {
1009810208
list_del(&set->list);
1009910209
table->use--;
10210+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
10211+
nft_map_deactivate(&ctx, set);
10212+
1010010213
nft_set_destroy(&ctx, set);
1010110214
}
1010210215
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);

‎net/netfilter/nft_set_rbtree.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,8 @@ static int nft_rbtree_init(const struct nft_set *set,
664664
return 0;
665665
}
666666

667-
static void nft_rbtree_destroy(const struct nft_set *set)
667+
static void nft_rbtree_destroy(const struct nft_ctx *ctx,
668+
const struct nft_set *set)
668669
{
669670
struct nft_rbtree *priv = nft_set_priv(set);
670671
struct nft_rbtree_elem *rbe;
@@ -675,7 +676,7 @@ static void nft_rbtree_destroy(const struct nft_set *set)
675676
while ((node = priv->root.rb_node) != NULL) {
676677
rb_erase(node, &priv->root);
677678
rbe = rb_entry(node, struct nft_rbtree_elem, node);
678-
nft_set_elem_destroy(set, rbe, true);
679+
nf_tables_set_elem_destroy(ctx, set, rbe);
679680
}
680681
}
681682

0 commit comments

Comments
 (0)