Skip to content

Commit 233bfe0

Browse files
pvts-matpvts-mat
committed
netfilter: nf_tables: restore set elements when delete set fails
jira VULN-37622 cve CVE-2024-27012 commit-author Pablo Neira Ayuso <pablo@netfilter.org> commit e79b47a upstream-diff Accounted for the missing commit 0e1ea65. From abort path, nft_mapelem_activate() needs to restore refcounters to the original state. Currently, it uses the set->ops->walk() to iterate over these set elements. The existing set iterator skips inactive elements in the next generation, this does not work from the abort path to restore the original state since it has to skip active elements instead (not inactive ones). This patch moves the check for inactive elements to the set iterator callback, then it reverses the logic for the .activate case which needs to skip active elements. Toggle next generation bit for elements when delete set command is invoked and call nft_clear() from .activate (abort) path to restore the next generation bit. The splat below shows an object in mappings memleak: [43929.457523] ------------[ cut here ]------------ [43929.457532] WARNING: CPU: 0 PID: 1139 at include/net/netfilter/nf_tables.h:1237 nft_setelem_data_deactivate+0xe4/0xf0 [nf_tables] [...] [43929.458014] RIP: 0010:nft_setelem_data_deactivate+0xe4/0xf0 [nf_tables] [43929.458076] Code: 83 f8 01 77 ab 49 8d 7c 24 08 e8 37 5e d0 de 49 8b 6c 24 08 48 8d 7d 50 e8 e9 5c d0 de 8b 45 50 8d 50 ff 89 55 50 85 c0 75 86 <0f> 0b eb 82 0f 0b eb b3 0f 1f 40 00 90 90 90 90 90 90 90 90 90 90 [43929.458081] RSP: 0018:ffff888140f9f4b0 EFLAGS: 00010246 [43929.458086] RAX: 0000000000000000 RBX: ffff8881434f5288 RCX: dffffc0000000000 [43929.458090] RDX: 00000000ffffffff RSI: ffffffffa26d28a7 RDI: ffff88810ecc9550 [43929.458093] RBP: ffff88810ecc9500 R08: 0000000000000001 R09: ffffed10281f3e8f [43929.458096] R10: 0000000000000003 R11: ffff0000ffff0000 R12: ffff8881434f52a0 [43929.458100] R13: ffff888140f9f5f4 R14: ffff888151c7a800 R15: 0000000000000002 [43929.458103] FS: 00007f0c687c4740(0000) GS:ffff888390800000(0000) knlGS:0000000000000000 [43929.458107] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [43929.458111] CR2: 00007f58dbe5b008 CR3: 0000000123602005 CR4: 00000000001706f0 [43929.458114] Call Trace: [43929.458118] <TASK> [43929.458121] ? __warn+0x9f/0x1a0 [43929.458127] ? nft_setelem_data_deactivate+0xe4/0xf0 [nf_tables] [43929.458188] ? report_bug+0x1b1/0x1e0 [43929.458196] ? handle_bug+0x3c/0x70 [43929.458200] ? exc_invalid_op+0x17/0x40 [43929.458211] ? nft_setelem_data_deactivate+0xd7/0xf0 [nf_tables] [43929.458271] ? nft_setelem_data_deactivate+0xe4/0xf0 [nf_tables] [43929.458332] nft_mapelem_deactivate+0x24/0x30 [nf_tables] [43929.458392] nft_rhash_walk+0xdd/0x180 [nf_tables] [43929.458453] ? __pfx_nft_rhash_walk+0x10/0x10 [nf_tables] [43929.458512] ? rb_insert_color+0x2e/0x280 [43929.458520] nft_map_deactivate+0xdc/0x1e0 [nf_tables] [43929.458582] ? __pfx_nft_map_deactivate+0x10/0x10 [nf_tables] [43929.458642] ? __pfx_nft_mapelem_deactivate+0x10/0x10 [nf_tables] [43929.458701] ? __rcu_read_unlock+0x46/0x70 [43929.458709] nft_delset+0xff/0x110 [nf_tables] [43929.458769] nft_flush_table+0x16f/0x460 [nf_tables] [43929.458830] nf_tables_deltable+0x501/0x580 [nf_tables] Fixes: 628bd3e ("netfilter: nf_tables: drop map element references from preparation phase") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> (cherry picked from commit e79b47a) Signed-off-by: Marcin Wcisło <marcin.wcislo@conclusive.pl>
1 parent bfe15da commit 233bfe0

5 files changed

Lines changed: 44 additions & 20 deletions

File tree

‎net/netfilter/nf_tables_api.c‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,12 @@ static int nft_mapelem_deactivate(const struct nft_ctx *ctx,
593593
const struct nft_set_iter *iter,
594594
struct nft_set_elem *elem)
595595
{
596+
struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
597+
598+
if (!nft_set_elem_active(ext, iter->genmask))
599+
return 0;
600+
601+
nft_set_elem_change_active(ctx->net, set, ext);
596602
nft_setelem_data_deactivate(ctx->net, set, elem);
597603

598604
return 0;
@@ -617,6 +623,7 @@ static void nft_map_catchall_deactivate(const struct nft_ctx *ctx,
617623
if (!nft_set_elem_active(ext, genmask))
618624
continue;
619625

626+
nft_set_elem_change_active(ctx->net, set, ext);
620627
elem.priv = catchall->elem;
621628
nft_setelem_data_deactivate(ctx->net, set, &elem);
622629
break;
@@ -3486,6 +3493,9 @@ int nft_setelem_validate(const struct nft_ctx *ctx, struct nft_set *set,
34863493
const struct nft_data *data;
34873494
int err;
34883495

3496+
if (!nft_set_elem_active(ext, iter->genmask))
3497+
return 0;
3498+
34893499
if (nft_set_ext_exists(ext, NFT_SET_EXT_FLAGS) &&
34903500
*nft_set_ext_flags(ext) & NFT_SET_ELEM_INTERVAL_END)
34913501
return 0;
@@ -3509,19 +3519,21 @@ int nft_setelem_validate(const struct nft_ctx *ctx, struct nft_set *set,
35093519

35103520
int nft_set_catchall_validate(const struct nft_ctx *ctx, struct nft_set *set)
35113521
{
3512-
u8 genmask = nft_genmask_next(ctx->net);
3522+
struct nft_set_iter dummy_iter = {
3523+
.genmask = nft_genmask_next(ctx->net),
3524+
};
35133525
struct nft_set_elem_catchall *catchall;
35143526
struct nft_set_elem elem;
35153527
struct nft_set_ext *ext;
35163528
int ret = 0;
35173529

35183530
list_for_each_entry_rcu(catchall, &set->catchall_list, list) {
35193531
ext = nft_set_elem_ext(set, catchall->elem);
3520-
if (!nft_set_elem_active(ext, genmask))
3532+
if (!nft_set_elem_active(ext, dummy_iter.genmask))
35213533
continue;
35223534

35233535
elem.priv = catchall->elem;
3524-
ret = nft_setelem_validate(ctx, set, NULL, &elem);
3536+
ret = nft_setelem_validate(ctx, set, &dummy_iter, &elem);
35253537
if (ret < 0)
35263538
return ret;
35273539
}
@@ -4998,6 +5010,11 @@ static int nf_tables_bind_check_setelem(const struct nft_ctx *ctx,
49985010
const struct nft_set_iter *iter,
49995011
struct nft_set_elem *elem)
50005012
{
5013+
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
5014+
5015+
if (!nft_set_elem_active(ext, iter->genmask))
5016+
return 0;
5017+
50015018
return nft_setelem_data_validate(ctx, set, elem);
50025019
}
50035020

@@ -5092,6 +5109,13 @@ static int nft_mapelem_activate(const struct nft_ctx *ctx,
50925109
const struct nft_set_iter *iter,
50935110
struct nft_set_elem *elem)
50945111
{
5112+
struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
5113+
5114+
/* called from abort path, reverse check to undo changes. */
5115+
if (nft_set_elem_active(ext, iter->genmask))
5116+
return 0;
5117+
5118+
nft_clear(ctx->net, ext);
50955119
nft_setelem_data_activate(ctx->net, set, elem);
50965120

50975121
return 0;
@@ -5110,6 +5134,7 @@ static void nft_map_catchall_activate(const struct nft_ctx *ctx,
51105134
if (!nft_set_elem_active(ext, genmask))
51115135
continue;
51125136

5137+
nft_clear(ctx->net, ext);
51135138
elem.priv = catchall->elem;
51145139
nft_setelem_data_activate(ctx->net, set, &elem);
51155140
break;
@@ -5382,6 +5407,9 @@ static int nf_tables_dump_setelem(const struct nft_ctx *ctx,
53825407
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
53835408
struct nft_set_dump_args *args;
53845409

5410+
if (!nft_set_elem_active(ext, iter->genmask))
5411+
return 0;
5412+
53855413
if (nft_set_elem_expired(ext))
53865414
return 0;
53875415

@@ -6115,7 +6143,7 @@ static void nft_setelem_activate(struct net *net, struct nft_set *set,
61156143
struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
61166144

61176145
if (nft_setelem_is_catchall(set, elem)) {
6118-
nft_set_elem_change_active(net, set, ext);
6146+
nft_clear(net, ext);
61196147
} else {
61206148
set->ops->activate(net, set, elem);
61216149
}
@@ -6789,8 +6817,12 @@ static int nft_setelem_flush(const struct nft_ctx *ctx,
67896817
const struct nft_set_iter *iter,
67906818
struct nft_set_elem *elem)
67916819
{
6820+
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
67926821
struct nft_trans *trans;
67936822

6823+
if (!nft_set_elem_active(ext, iter->genmask))
6824+
return 0;
6825+
67946826
trans = nft_trans_alloc_gfp(ctx, NFT_MSG_DELSETELEM,
67956827
sizeof(struct nft_trans_elem), GFP_ATOMIC);
67966828
if (!trans)
@@ -10054,6 +10086,9 @@ static int nf_tables_loop_check_setelem(const struct nft_ctx *ctx,
1005410086
{
1005510087
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
1005610088

10089+
if (!nft_set_elem_active(ext, iter->genmask))
10090+
return 0;
10091+
1005710092
if (nft_set_ext_exists(ext, NFT_SET_EXT_FLAGS) &&
1005810093
*nft_set_ext_flags(ext) & NFT_SET_ELEM_INTERVAL_END)
1005910094
return 0;

‎net/netfilter/nft_set_bitmap.c‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ static void nft_bitmap_activate(const struct net *net,
173173
nft_bitmap_location(set, nft_set_ext_key(&be->ext), &idx, &off);
174174
/* Enter 11 state. */
175175
priv->bitmap[idx] |= (genmask << off);
176-
nft_set_elem_change_active(net, set, &be->ext);
176+
nft_clear(net, &be->ext);
177177
}
178178

179179
static void nft_bitmap_flush(const struct net *net,
@@ -224,8 +224,6 @@ static void nft_bitmap_walk(const struct nft_ctx *ctx,
224224
list_for_each_entry_rcu(be, &priv->list, head) {
225225
if (iter->count < iter->skip)
226226
goto cont;
227-
if (!nft_set_elem_active(&be->ext, iter->genmask))
228-
goto cont;
229227

230228
elem.priv = &be->priv;
231229

‎net/netfilter/nft_set_hash.c‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ static void nft_rhash_activate(const struct net *net, const struct nft_set *set,
194194
{
195195
struct nft_rhash_elem *he = nft_elem_priv_cast(elem->priv);
196196

197-
nft_set_elem_change_active(net, set, &he->ext);
197+
nft_clear(net, &he->ext);
198198
}
199199

200200
static void nft_rhash_flush(const struct net *net,
@@ -281,8 +281,6 @@ static void nft_rhash_walk(const struct nft_ctx *ctx, struct nft_set *set,
281281

282282
if (iter->count < iter->skip)
283283
goto cont;
284-
if (!nft_set_elem_active(&he->ext, iter->genmask))
285-
goto cont;
286284

287285
elem.priv = &he->priv;
288286

@@ -596,7 +594,7 @@ static void nft_hash_activate(const struct net *net, const struct nft_set *set,
596594
{
597595
struct nft_hash_elem *he = nft_elem_priv_cast(elem->priv);
598596

599-
nft_set_elem_change_active(net, set, &he->ext);
597+
nft_clear(net, &he->ext);
600598
}
601599

602600
static void nft_hash_flush(const struct net *net,
@@ -650,8 +648,6 @@ static void nft_hash_walk(const struct nft_ctx *ctx, struct nft_set *set,
650648
hlist_for_each_entry_rcu(he, &priv->table[i], node) {
651649
if (iter->count < iter->skip)
652650
goto cont;
653-
if (!nft_set_elem_active(&he->ext, iter->genmask))
654-
goto cont;
655651

656652
elem.priv = &he->priv;
657653

‎net/netfilter/nft_set_pipapo.c‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1754,7 +1754,7 @@ static void nft_pipapo_activate(const struct net *net,
17541754
{
17551755
struct nft_pipapo_elem *e = nft_elem_priv_cast(elem->priv);
17561756

1757-
nft_set_elem_change_active(net, set, &e->ext);
1757+
nft_clear(net, &e->ext);
17581758
}
17591759

17601760
/**
@@ -2052,9 +2052,6 @@ static void nft_pipapo_walk(const struct nft_ctx *ctx, struct nft_set *set,
20522052

20532053
e = f->mt[r].e;
20542054

2055-
if (!nft_set_elem_active(&e->ext, iter->genmask))
2056-
goto cont;
2057-
20582055
elem.priv = &e->priv;
20592056

20602057
iter->err = iter->fn(ctx, set, iter, &elem);

‎net/netfilter/nft_set_rbtree.c‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ static void nft_rbtree_activate(const struct net *net,
530530
{
531531
struct nft_rbtree_elem *rbe = nft_elem_priv_cast(elem->priv);
532532

533-
nft_set_elem_change_active(net, set, &rbe->ext);
533+
nft_clear(net, &rbe->ext);
534534
}
535535

536536
static void nft_rbtree_flush(const struct net *net,
@@ -596,8 +596,6 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx,
596596

597597
if (iter->count < iter->skip)
598598
goto cont;
599-
if (!nft_set_elem_active(&rbe->ext, iter->genmask))
600-
goto cont;
601599

602600
elem.priv = &rbe->priv;
603601

0 commit comments

Comments
 (0)