Skip to content

Commit 70c31ad

Browse files
pvts-matpvts-mat
committed
netfilter: nf_tables: drop map element references from preparation phase
jira VULN-430 cve-pre CVE-2023-4244 commit-author Pablo Neira Ayuso <pablo@netfilter.org> commit 628bd3e 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> (cherry picked from commit 628bd3e) Signed-off-by: Marcin Wcisło <marcin.wcislo@conclusive.pl>
1 parent cde4746 commit 70c31ad

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

@@ -3438,12 +3493,6 @@ int nft_setelem_validate(const struct nft_ctx *ctx, struct nft_set *set,
34383493
return 0;
34393494
}
34403495

3441-
struct nft_set_elem_catchall {
3442-
struct list_head list;
3443-
struct rcu_head rcu;
3444-
void *elem;
3445-
};
3446-
34473496
int nft_set_catchall_validate(const struct nft_ctx *ctx, struct nft_set *set)
34483497
{
34493498
u8 genmask = nft_genmask_next(ctx->net);
@@ -4774,7 +4823,7 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info,
47744823
for (i = 0; i < set->num_exprs; i++)
47754824
nft_expr_destroy(&ctx, set->exprs[i]);
47764825
err_set_destroy:
4777-
ops->destroy(set);
4826+
ops->destroy(&ctx, set);
47784827
err_set_init:
47794828
kfree(set->name);
47804829
err_set_name:
@@ -4789,7 +4838,7 @@ static void nft_set_catchall_destroy(const struct nft_ctx *ctx,
47894838

47904839
list_for_each_entry_safe(catchall, next, &set->catchall_list, list) {
47914840
list_del_rcu(&catchall->list);
4792-
nft_set_elem_destroy(set, catchall->elem, true);
4841+
nf_tables_set_elem_destroy(ctx, set, catchall->elem);
47934842
kfree_rcu(catchall, rcu);
47944843
}
47954844
}
@@ -4804,7 +4853,7 @@ static void nft_set_destroy(const struct nft_ctx *ctx, struct nft_set *set)
48044853
for (i = 0; i < set->num_exprs; i++)
48054854
nft_expr_destroy(ctx, set->exprs[i]);
48064855

4807-
set->ops->destroy(set);
4856+
set->ops->destroy(ctx, set);
48084857
nft_set_catchall_destroy(ctx, set);
48094858
kfree(set->name);
48104859
kvfree(set);
@@ -4969,10 +5018,60 @@ static void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
49695018
}
49705019
}
49715020

5021+
static void nft_setelem_data_activate(const struct net *net,
5022+
const struct nft_set *set,
5023+
struct nft_set_elem *elem);
5024+
5025+
static int nft_mapelem_activate(const struct nft_ctx *ctx,
5026+
struct nft_set *set,
5027+
const struct nft_set_iter *iter,
5028+
struct nft_set_elem *elem)
5029+
{
5030+
nft_setelem_data_activate(ctx->net, set, elem);
5031+
5032+
return 0;
5033+
}
5034+
5035+
static void nft_map_catchall_activate(const struct nft_ctx *ctx,
5036+
struct nft_set *set)
5037+
{
5038+
u8 genmask = nft_genmask_next(ctx->net);
5039+
struct nft_set_elem_catchall *catchall;
5040+
struct nft_set_elem elem;
5041+
struct nft_set_ext *ext;
5042+
5043+
list_for_each_entry(catchall, &set->catchall_list, list) {
5044+
ext = nft_set_elem_ext(set, catchall->elem);
5045+
if (!nft_set_elem_active(ext, genmask))
5046+
continue;
5047+
5048+
elem.priv = catchall->elem;
5049+
nft_setelem_data_activate(ctx->net, set, &elem);
5050+
break;
5051+
}
5052+
}
5053+
5054+
static void nft_map_activate(const struct nft_ctx *ctx, struct nft_set *set)
5055+
{
5056+
struct nft_set_iter iter = {
5057+
.genmask = nft_genmask_next(ctx->net),
5058+
.fn = nft_mapelem_activate,
5059+
};
5060+
5061+
set->ops->walk(ctx, set, &iter);
5062+
WARN_ON_ONCE(iter.err);
5063+
5064+
nft_map_catchall_activate(ctx, set);
5065+
}
5066+
49725067
void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set)
49735068
{
4974-
if (nft_set_is_anonymous(set))
5069+
if (nft_set_is_anonymous(set)) {
5070+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5071+
nft_map_activate(ctx, set);
5072+
49755073
nft_clear(ctx->net, set);
5074+
}
49765075

49775076
set->use++;
49785077
}
@@ -4993,13 +5092,20 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
49935092
set->use--;
49945093
break;
49955094
case NFT_TRANS_PREPARE:
4996-
if (nft_set_is_anonymous(set))
4997-
nft_deactivate_next(ctx->net, set);
5095+
if (nft_set_is_anonymous(set)) {
5096+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5097+
nft_map_deactivate(ctx, set);
49985098

5099+
nft_deactivate_next(ctx->net, set);
5100+
}
49995101
set->use--;
50005102
return;
50015103
case NFT_TRANS_ABORT:
50025104
case NFT_TRANS_RELEASE:
5105+
if (nft_set_is_anonymous(set) &&
5106+
set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
5107+
nft_map_deactivate(ctx, set);
5108+
50035109
set->use--;
50045110
fallthrough;
50055111
default:
@@ -5774,6 +5880,7 @@ static void nft_set_elem_expr_destroy(const struct nft_ctx *ctx,
57745880
__nft_set_elem_expr_destroy(ctx, expr);
57755881
}
57765882

5883+
/* Drop references and destroy. Called from gc, dynset and abort path. */
57775884
void nft_set_elem_destroy(const struct nft_set *set, void *elem,
57785885
bool destroy_expr)
57795886
{
@@ -5795,11 +5902,11 @@ void nft_set_elem_destroy(const struct nft_set *set, void *elem,
57955902
}
57965903
EXPORT_SYMBOL_GPL(nft_set_elem_destroy);
57975904

5798-
/* Only called from commit path, nft_setelem_data_deactivate() already deals
5799-
* with the refcounting from the preparation phase.
5905+
/* Destroy element. References have been already dropped in the preparation
5906+
* path via nft_setelem_data_deactivate().
58005907
*/
5801-
static void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
5802-
const struct nft_set *set, void *elem)
5908+
void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
5909+
const struct nft_set *set, void *elem)
58035910
{
58045911
struct nft_set_ext *ext = nft_set_elem_ext(set, elem);
58055912

@@ -6432,7 +6539,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
64326539
if (obj)
64336540
obj->use--;
64346541
err_elem_userdata:
6435-
nf_tables_set_elem_destroy(ctx, set, elem.priv);
6542+
nft_set_elem_destroy(set, elem.priv, true);
64366543
err_parse_data:
64376544
if (nla[NFTA_SET_ELEM_DATA] != NULL)
64386545
nft_data_release(&elem.data.val, desc.type);
@@ -9508,6 +9615,9 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
95089615
case NFT_MSG_DESTROYSET:
95099616
trans->ctx.table->use++;
95109617
nft_clear(trans->ctx.net, nft_trans_set(trans));
9618+
if (nft_trans_set(trans)->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
9619+
nft_map_activate(&trans->ctx, nft_trans_set(trans));
9620+
95119621
nft_trans_destroy(trans);
95129622
break;
95139623
case NFT_MSG_NEWSETELEM:
@@ -10274,6 +10384,9 @@ static void __nft_release_table(struct net *net, struct nft_table *table)
1027410384
list_for_each_entry_safe(set, ns, &table->sets, list) {
1027510385
list_del(&set->list);
1027610386
table->use--;
10387+
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
10388+
nft_map_deactivate(&ctx, set);
10389+
1027710390
nft_set_destroy(&ctx, set);
1027810391
}
1027910392
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
@@ -2160,10 +2160,12 @@ static int nft_pipapo_init(const struct nft_set *set,
21602160

21612161
/**
21622162
* nft_set_pipapo_match_destroy() - Destroy elements from key mapping array
2163+
* @ctx: context
21632164
* @set: nftables API set representation
21642165
* @m: matching data pointing to key mapping array
21652166
*/
2166-
static void nft_set_pipapo_match_destroy(const struct nft_set *set,
2167+
static void nft_set_pipapo_match_destroy(const struct nft_ctx *ctx,
2168+
const struct nft_set *set,
21672169
struct nft_pipapo_match *m)
21682170
{
21692171
struct nft_pipapo_field *f;
@@ -2180,15 +2182,17 @@ static void nft_set_pipapo_match_destroy(const struct nft_set *set,
21802182

21812183
e = f->mt[r].e;
21822184

2183-
nft_set_elem_destroy(set, e, true);
2185+
nf_tables_set_elem_destroy(ctx, set, e);
21842186
}
21852187
}
21862188

21872189
/**
21882190
* nft_pipapo_destroy() - Free private data for set and all committed elements
2191+
* @ctx: context
21892192
* @set: nftables API set representation
21902193
*/
2191-
static void nft_pipapo_destroy(const struct nft_set *set)
2194+
static void nft_pipapo_destroy(const struct nft_ctx *ctx,
2195+
const struct nft_set *set)
21922196
{
21932197
struct nft_pipapo *priv = nft_set_priv(set);
21942198
struct nft_pipapo_match *m;
@@ -2198,7 +2202,7 @@ static void nft_pipapo_destroy(const struct nft_set *set)
21982202
if (m) {
21992203
rcu_barrier();
22002204

2201-
nft_set_pipapo_match_destroy(set, m);
2205+
nft_set_pipapo_match_destroy(ctx, set, m);
22022206

22032207
#ifdef NFT_PIPAPO_ALIGN
22042208
free_percpu(m->scratch_aligned);
@@ -2215,7 +2219,7 @@ static void nft_pipapo_destroy(const struct nft_set *set)
22152219
m = priv->clone;
22162220

22172221
if (priv->dirty)
2218-
nft_set_pipapo_match_destroy(set, m);
2222+
nft_set_pipapo_match_destroy(ctx, set, m);
22192223

22202224
#ifdef NFT_PIPAPO_ALIGN
22212225
free_percpu(priv->clone->scratch_aligned);

0 commit comments

Comments
 (0)