Skip to content

Commit ea628a4

Browse files
committed
netfilter: nf_tables: adapt set backend to use GC transaction API
jira VULN-429 cve CVE-2023-4244 commit-author Pablo Neira Ayuso <pablo@netfilter.org>' commit f6c383b upstream-diff - Upstream fuzz and conflicts. Resolved by pointing to the rocky8_10 branch as the source of truth. Use the GC transaction API to replace the old and buggy gc API and the busy mark approach. No set elements are removed from async garbage collection anymore, instead the _DEAD bit is set on so the set element is not visible from lookup path anymore. Async GC enqueues transaction work that might be aborted and retried later. rbtree and pipapo set backends does not set on the _DEAD bit from the sync GC path since this runs in control plane path where mutex is held. In this case, set elements are deactivated, removed and then released via RCU callback, sync GC never fails. Fixes: 3c4287f ("nf_tables: Add set type for arbitrary concatenation of ranges") Fixes: 8d8540c ("netfilter: nft_set_rbtree: add timeout support") Fixes: 9d09829 ("netfilter: nft_hash: add support for timeouts") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> (cherry picked from commit f6c383b) Signed-off-by: Greg Rose <g.v.rose@ciq.com>
1 parent 9cff1f7 commit ea628a4

File tree

4 files changed

+150
-69
lines changed

4 files changed

+150
-69
lines changed

net/netfilter/nf_tables_api.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5778,6 +5778,7 @@ void nft_setelem_data_deactivate(const struct net *net,
57785778
if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
57795779
nft_use_dec(&(*nft_set_ext_obj(ext))->use);
57805780
}
5781+
EXPORT_SYMBOL_GPL(nft_setelem_data_deactivate);
57815782

57825783
static int nft_del_setelem(struct nft_ctx *ctx, struct nft_set *set,
57835784
const struct nlattr *attr)
@@ -7846,6 +7847,7 @@ void nft_trans_gc_destroy(struct nft_trans_gc *trans)
78467847
put_net(trans->net);
78477848
kfree(trans);
78487849
}
7850+
EXPORT_SYMBOL_GPL(nft_trans_gc_destroy);
78497851

78507852
static void nft_trans_gc_trans_free(struct rcu_head *rcu)
78517853
{
@@ -7933,11 +7935,13 @@ struct nft_trans_gc *nft_trans_gc_alloc(struct nft_set *set,
79337935

79347936
return trans;
79357937
}
7938+
EXPORT_SYMBOL_GPL(nft_trans_gc_alloc);
79367939

79377940
void nft_trans_gc_elem_add(struct nft_trans_gc *trans, void *priv)
79387941
{
79397942
trans->priv[trans->count++] = priv;
79407943
}
7944+
EXPORT_SYMBOL_GPL(nft_trans_gc_elem_add);
79417945

79427946
static void nft_trans_gc_queue_work(struct nft_trans_gc *trans)
79437947
{
@@ -7963,6 +7967,7 @@ struct nft_trans_gc *nft_trans_gc_queue_async(struct nft_trans_gc *gc,
79637967

79647968
return nft_trans_gc_alloc(gc->set, gc_seq, gfp);
79657969
}
7970+
EXPORT_SYMBOL_GPL(nft_trans_gc_queue_async);
79667971

79677972
void nft_trans_gc_queue_async_done(struct nft_trans_gc *trans)
79687973
{
@@ -7973,6 +7978,7 @@ void nft_trans_gc_queue_async_done(struct nft_trans_gc *trans)
79737978

79747979
nft_trans_gc_queue_work(trans);
79757980
}
7981+
EXPORT_SYMBOL_GPL(nft_trans_gc_queue_async_done);
79767982

79777983
struct nft_trans_gc *nft_trans_gc_queue_sync(struct nft_trans_gc *gc, gfp_t gfp)
79787984
{
@@ -7986,6 +7992,7 @@ struct nft_trans_gc *nft_trans_gc_queue_sync(struct nft_trans_gc *gc, gfp_t gfp)
79867992

79877993
return nft_trans_gc_alloc(gc->set, 0, gfp);
79887994
}
7995+
EXPORT_SYMBOL_GPL(nft_trans_gc_queue_sync);
79897996

79907997
void nft_trans_gc_queue_sync_done(struct nft_trans_gc *trans)
79917998
{
@@ -7998,6 +8005,7 @@ void nft_trans_gc_queue_sync_done(struct nft_trans_gc *trans)
79988005

79998006
call_rcu(&trans->rcu, nft_trans_gc_trans_free);
80008007
}
8008+
EXPORT_SYMBOL_GPL(nft_trans_gc_queue_sync_done);
80018009

80028010
static void nf_tables_module_autoload_cleanup(struct net *net)
80038011
{

net/netfilter/nft_set_hash.c

Lines changed: 45 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ static inline int nft_rhash_cmp(struct rhashtable_compare_arg *arg,
6262

6363
if (memcmp(nft_set_ext_key(&he->ext), x->key, x->set->klen))
6464
return 1;
65+
if (nft_set_elem_is_dead(&he->ext))
66+
return 1;
6567
if (nft_set_elem_expired(&he->ext))
6668
return 1;
6769
if (!nft_set_elem_active(&he->ext, x->genmask))
@@ -190,20 +192,16 @@ static void nft_rhash_activate(const struct net *net, const struct nft_set *set,
190192
struct nft_rhash_elem *he = elem->priv;
191193

192194
nft_set_elem_change_active(net, set, &he->ext);
193-
nft_set_elem_clear_busy(&he->ext);
194195
}
195196

196197
static bool nft_rhash_flush(const struct net *net,
197198
const struct nft_set *set, void *priv)
198199
{
199200
struct nft_rhash_elem *he = priv;
200201

201-
if (!nft_set_elem_mark_busy(&he->ext) ||
202-
!nft_is_active(net, &he->ext)) {
203-
nft_set_elem_change_active(net, set, &he->ext);
204-
return true;
205-
}
206-
return false;
202+
nft_set_elem_change_active(net, set, &he->ext);
203+
204+
return true;
207205
}
208206

209207
static void *nft_rhash_deactivate(const struct net *net,
@@ -220,9 +218,8 @@ static void *nft_rhash_deactivate(const struct net *net,
220218

221219
rcu_read_lock();
222220
he = rhashtable_lookup(&priv->ht, &arg, nft_rhash_params);
223-
if (he != NULL &&
224-
!nft_rhash_flush(net, set, he))
225-
he = NULL;
221+
if (he)
222+
nft_set_elem_change_active(net, set, &he->ext);
226223

227224
rcu_read_unlock();
228225

@@ -296,46 +293,72 @@ static bool nft_rhash_expr_needs_gc_run(const struct nft_set *set,
296293

297294
static void nft_rhash_gc(struct work_struct *work)
298295
{
296+
struct nftables_pernet *nft_net;
299297
struct nft_set *set;
300298
struct nft_rhash_elem *he;
301299
struct nft_rhash *priv;
302-
struct nft_set_gc_batch *gcb = NULL;
303300
struct rhashtable_iter hti;
301+
struct nft_trans_gc *gc;
302+
struct net *net;
303+
u32 gc_seq;
304304

305305
priv = container_of(work, struct nft_rhash, gc_work.work);
306306
set = nft_set_container_of(priv);
307+
net = read_pnet(&set->net);
308+
nft_net = nft_pernet(net);
309+
gc_seq = READ_ONCE(nft_net->gc_seq);
310+
311+
gc = nft_trans_gc_alloc(set, gc_seq, GFP_KERNEL);
312+
if (!gc)
313+
goto done;
307314

308315
rhashtable_walk_enter(&priv->ht, &hti);
309316
rhashtable_walk_start(&hti);
310317

311318
while ((he = rhashtable_walk_next(&hti))) {
312319
if (IS_ERR(he)) {
313-
if (PTR_ERR(he) != -EAGAIN)
314-
break;
320+
if (PTR_ERR(he) != -EAGAIN) {
321+
nft_trans_gc_destroy(gc);
322+
gc = NULL;
323+
goto try_later;
324+
}
315325
continue;
316326
}
317327

328+
/* Ruleset has been updated, try later. */
329+
if (READ_ONCE(nft_net->gc_seq) != gc_seq) {
330+
nft_trans_gc_destroy(gc);
331+
gc = NULL;
332+
goto try_later;
333+
}
334+
335+
if (nft_set_elem_is_dead(&he->ext))
336+
goto dead_elem;
337+
318338
if (nft_set_ext_exists(&he->ext, NFT_SET_EXT_EXPRESSIONS) &&
319339
nft_rhash_expr_needs_gc_run(set, &he->ext))
320340
goto needs_gc_run;
321341

322342
if (!nft_set_elem_expired(&he->ext))
323343
continue;
324344
needs_gc_run:
325-
if (nft_set_elem_mark_busy(&he->ext))
326-
continue;
345+
nft_set_elem_dead(&he->ext);
346+
dead_elem:
347+
gc = nft_trans_gc_queue_async(gc, gc_seq, GFP_ATOMIC);
348+
if (!gc)
349+
goto try_later;
327350

328-
gcb = nft_set_gc_batch_check(set, gcb, GFP_ATOMIC);
329-
if (gcb == NULL)
330-
break;
331-
rhashtable_remove_fast(&priv->ht, &he->node, nft_rhash_params);
332-
atomic_dec(&set->nelems);
333-
nft_set_gc_batch_add(gcb, he);
351+
nft_trans_gc_elem_add(gc, he);
334352
}
353+
354+
try_later:
335355
rhashtable_walk_stop(&hti);
336356
rhashtable_walk_exit(&hti);
337357

338-
nft_set_gc_batch_complete(gcb);
358+
if (gc)
359+
nft_trans_gc_queue_async_done(gc);
360+
361+
done:
339362
queue_delayed_work(system_power_efficient_wq, &priv->gc_work,
340363
nft_set_gc_interval(set));
341364
}
@@ -386,7 +409,6 @@ static void nft_rhash_destroy(const struct nft_set *set)
386409
struct nft_rhash *priv = nft_set_priv(set);
387410

388411
cancel_delayed_work_sync(&priv->gc_work);
389-
rcu_barrier();
390412
rhashtable_free_and_destroy(&priv->ht, nft_rhash_elem_destroy,
391413
(void *)set);
392414
}

net/netfilter/nft_set_pipapo.c

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,20 +1436,38 @@ static void pipapo_drop(struct nft_pipapo_match *m,
14361436
}
14371437
}
14381438

1439+
static void nft_pipapo_gc_deactivate(struct net *net, struct nft_set *set,
1440+
struct nft_pipapo_elem *e)
1441+
1442+
{
1443+
struct nft_set_elem elem = {
1444+
.priv = e,
1445+
};
1446+
1447+
nft_setelem_data_deactivate(net, set, &elem);
1448+
}
1449+
14391450
/**
14401451
* pipapo_gc() - Drop expired entries from set, destroy start and end elements
14411452
* @set: nftables API set representation
14421453
* @m: Matching data
14431454
*/
1444-
static void pipapo_gc(const struct nft_set *set, struct nft_pipapo_match *m)
1455+
static void pipapo_gc(const struct nft_set *_set, struct nft_pipapo_match *m)
14451456
{
1457+
struct nft_set *set = (struct nft_set *) _set;
14461458
struct nft_pipapo *priv = nft_set_priv(set);
1459+
struct net *net = read_pnet(&set->net);
14471460
int rules_f0, first_rule = 0;
1461+
struct nft_pipapo_elem *e;
1462+
struct nft_trans_gc *gc;
1463+
1464+
gc = nft_trans_gc_alloc(set, 0, GFP_KERNEL);
1465+
if (!gc)
1466+
return;
14481467

14491468
while ((rules_f0 = pipapo_rules_same_key(m->f, first_rule))) {
14501469
union nft_pipapo_map_bucket rulemap[NFT_PIPAPO_MAX_FIELDS];
14511470
struct nft_pipapo_field *f;
1452-
struct nft_pipapo_elem *e;
14531471
int i, start, rules_fx;
14541472

14551473
start = first_rule;
@@ -1469,13 +1487,20 @@ static void pipapo_gc(const struct nft_set *set, struct nft_pipapo_match *m)
14691487
f--;
14701488
i--;
14711489
e = f->mt[rulemap[i].to].e;
1472-
if (nft_set_elem_expired(&e->ext) &&
1473-
!nft_set_elem_mark_busy(&e->ext)) {
1490+
1491+
/* synchronous gc never fails, there is no need to set on
1492+
* NFT_SET_ELEM_DEAD_BIT.
1493+
*/
1494+
if (nft_set_elem_expired(&e->ext)) {
14741495
priv->dirty = true;
1475-
pipapo_drop(m, rulemap);
14761496

1477-
rcu_barrier();
1478-
nft_set_elem_destroy(set, e, true);
1497+
gc = nft_trans_gc_queue_sync(gc, GFP_ATOMIC);
1498+
if (!gc)
1499+
break;
1500+
1501+
nft_pipapo_gc_deactivate(net, set, e);
1502+
pipapo_drop(m, rulemap);
1503+
nft_trans_gc_elem_add(gc, e);
14791504

14801505
/* And check again current first rule, which is now the
14811506
* first we haven't checked.
@@ -1485,7 +1510,10 @@ static void pipapo_gc(const struct nft_set *set, struct nft_pipapo_match *m)
14851510
}
14861511
}
14871512

1488-
priv->last_gc = jiffies;
1513+
if (gc) {
1514+
nft_trans_gc_queue_sync_done(gc);
1515+
priv->last_gc = jiffies;
1516+
}
14891517
}
14901518

14911519
/**
@@ -1607,7 +1635,6 @@ static void nft_pipapo_activate(const struct net *net,
16071635
return;
16081636

16091637
nft_set_elem_change_active(net, set, &e->ext);
1610-
nft_set_elem_clear_busy(&e->ext);
16111638
}
16121639

16131640
/**

0 commit comments

Comments
 (0)