Skip to content

Commit bb3c6b5

Browse files
fw-strlenfw-strlen
committed
netfilter: nf_tables: adapt set backend to use GC transaction API
JIRA: https://issues.redhat.com/browse/RHEL-1720 JIRA: https://issues.redhat.com/browse/RHEL-1721 Upstream Status: commit f6c383b CVE: CVE-2023-4244 commit f6c383b Author: Pablo Neira Ayuso <pablo@netfilter.org> Date: Wed Aug 9 14:54:23 2023 +0200 netfilter: nf_tables: adapt set backend to use GC transaction API 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> Signed-off-by: Florian Westphal <fwestpha@redhat.com>
1 parent 7e1fc03 commit bb3c6b5

File tree

4 files changed

+173
-103
lines changed

4 files changed

+173
-103
lines changed

‎net/netfilter/nf_tables_api.c‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6122,7 +6122,6 @@ static void nft_setelem_activate(struct net *net, struct nft_set *set,
61226122

61236123
if (nft_setelem_is_catchall(set, elem)) {
61246124
nft_set_elem_change_active(net, set, ext);
6125-
nft_set_elem_clear_busy(ext);
61266125
} else {
61276126
set->ops->activate(net, set, elem);
61286127
}
@@ -6137,8 +6136,7 @@ static int nft_setelem_catchall_deactivate(const struct net *net,
61376136

61386137
list_for_each_entry(catchall, &set->catchall_list, list) {
61396138
ext = nft_set_elem_ext(set, catchall->elem);
6140-
if (!nft_is_active(net, ext) ||
6141-
nft_set_elem_mark_busy(ext))
6139+
if (!nft_is_active(net, ext))
61426140
continue;
61436141

61446142
kfree(elem->priv);
@@ -6849,8 +6847,7 @@ static int nft_set_catchall_flush(const struct nft_ctx *ctx,
68496847

68506848
list_for_each_entry_rcu(catchall, &set->catchall_list, list) {
68516849
ext = nft_set_elem_ext(set, catchall->elem);
6852-
if (!nft_set_elem_active(ext, genmask) ||
6853-
nft_set_elem_mark_busy(ext))
6850+
if (!nft_set_elem_active(ext, genmask))
68546851
continue;
68556852

68566853
elem.priv = catchall->elem;

‎net/netfilter/nft_set_hash.c‎

Lines changed: 48 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ static inline int nft_rhash_cmp(struct rhashtable_compare_arg *arg,
5959

6060
if (memcmp(nft_set_ext_key(&he->ext), x->key, x->set->klen))
6161
return 1;
62+
if (nft_set_elem_is_dead(&he->ext))
63+
return 1;
6264
if (nft_set_elem_expired(&he->ext))
6365
return 1;
6466
if (!nft_set_elem_active(&he->ext, x->genmask))
@@ -188,20 +190,16 @@ static void nft_rhash_activate(const struct net *net, const struct nft_set *set,
188190
struct nft_rhash_elem *he = elem->priv;
189191

190192
nft_set_elem_change_active(net, set, &he->ext);
191-
nft_set_elem_clear_busy(&he->ext);
192193
}
193194

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

199-
if (!nft_set_elem_mark_busy(&he->ext) ||
200-
!nft_is_active(net, &he->ext)) {
201-
nft_set_elem_change_active(net, set, &he->ext);
202-
return true;
203-
}
204-
return false;
200+
nft_set_elem_change_active(net, set, &he->ext);
201+
202+
return true;
205203
}
206204

207205
static void *nft_rhash_deactivate(const struct net *net,
@@ -218,9 +216,8 @@ static void *nft_rhash_deactivate(const struct net *net,
218216

219217
rcu_read_lock();
220218
he = rhashtable_lookup(&priv->ht, &arg, nft_rhash_params);
221-
if (he != NULL &&
222-
!nft_rhash_flush(net, set, he))
223-
he = NULL;
219+
if (he)
220+
nft_set_elem_change_active(net, set, &he->ext);
224221

225222
rcu_read_unlock();
226223

@@ -312,52 +309,75 @@ static bool nft_rhash_expr_needs_gc_run(const struct nft_set *set,
312309

313310
static void nft_rhash_gc(struct work_struct *work)
314311
{
312+
struct nftables_pernet *nft_net;
315313
struct nft_set *set;
316314
struct nft_rhash_elem *he;
317315
struct nft_rhash *priv;
318-
struct nft_set_gc_batch *gcb = NULL;
319316
struct rhashtable_iter hti;
317+
struct nft_trans_gc *gc;
318+
struct net *net;
319+
u32 gc_seq;
320320

321321
priv = container_of(work, struct nft_rhash, gc_work.work);
322322
set = nft_set_container_of(priv);
323+
net = read_pnet(&set->net);
324+
nft_net = nft_pernet(net);
325+
gc_seq = READ_ONCE(nft_net->gc_seq);
326+
327+
gc = nft_trans_gc_alloc(set, gc_seq, GFP_KERNEL);
328+
if (!gc)
329+
goto done;
323330

324331
rhashtable_walk_enter(&priv->ht, &hti);
325332
rhashtable_walk_start(&hti);
326333

327334
while ((he = rhashtable_walk_next(&hti))) {
328335
if (IS_ERR(he)) {
329-
if (PTR_ERR(he) != -EAGAIN)
330-
break;
336+
if (PTR_ERR(he) != -EAGAIN) {
337+
nft_trans_gc_destroy(gc);
338+
gc = NULL;
339+
goto try_later;
340+
}
331341
continue;
332342
}
333343

344+
/* Ruleset has been updated, try later. */
345+
if (READ_ONCE(nft_net->gc_seq) != gc_seq) {
346+
nft_trans_gc_destroy(gc);
347+
gc = NULL;
348+
goto try_later;
349+
}
350+
351+
if (nft_set_elem_is_dead(&he->ext))
352+
goto dead_elem;
353+
334354
if (nft_set_ext_exists(&he->ext, NFT_SET_EXT_EXPRESSIONS) &&
335355
nft_rhash_expr_needs_gc_run(set, &he->ext))
336356
goto needs_gc_run;
337357

338358
if (!nft_set_elem_expired(&he->ext))
339359
continue;
340360
needs_gc_run:
341-
if (nft_set_elem_mark_busy(&he->ext))
342-
continue;
361+
nft_set_elem_dead(&he->ext);
362+
dead_elem:
363+
gc = nft_trans_gc_queue_async(gc, gc_seq, GFP_ATOMIC);
364+
if (!gc)
365+
goto try_later;
343366

344-
gcb = nft_set_gc_batch_check(set, gcb, GFP_ATOMIC);
345-
if (gcb == NULL)
346-
break;
347-
rhashtable_remove_fast(&priv->ht, &he->node, nft_rhash_params);
348-
atomic_dec(&set->nelems);
349-
nft_set_gc_batch_add(gcb, he);
367+
nft_trans_gc_elem_add(gc, he);
350368
}
369+
370+
gc = nft_trans_gc_catchall(gc, gc_seq);
371+
372+
try_later:
373+
/* catchall list iteration requires rcu read side lock. */
351374
rhashtable_walk_stop(&hti);
352375
rhashtable_walk_exit(&hti);
353376

354-
he = nft_set_catchall_gc(set);
355-
if (he) {
356-
gcb = nft_set_gc_batch_check(set, gcb, GFP_ATOMIC);
357-
if (gcb)
358-
nft_set_gc_batch_add(gcb, he);
359-
}
360-
nft_set_gc_batch_complete(gcb);
377+
if (gc)
378+
nft_trans_gc_queue_async_done(gc);
379+
380+
done:
361381
queue_delayed_work(system_power_efficient_wq, &priv->gc_work,
362382
nft_set_gc_interval(set));
363383
}
@@ -420,7 +440,6 @@ static void nft_rhash_destroy(const struct nft_ctx *ctx,
420440
};
421441

422442
cancel_delayed_work_sync(&priv->gc_work);
423-
rcu_barrier();
424443
rhashtable_free_and_destroy(&priv->ht, nft_rhash_elem_destroy,
425444
(void *)&rhash_ctx);
426445
}

‎net/netfilter/nft_set_pipapo.c‎

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,16 +1537,34 @@ static void pipapo_drop(struct nft_pipapo_match *m,
15371537
}
15381538
}
15391539

1540+
static void nft_pipapo_gc_deactivate(struct net *net, struct nft_set *set,
1541+
struct nft_pipapo_elem *e)
1542+
1543+
{
1544+
struct nft_set_elem elem = {
1545+
.priv = e,
1546+
};
1547+
1548+
nft_setelem_data_deactivate(net, set, &elem);
1549+
}
1550+
15401551
/**
15411552
* pipapo_gc() - Drop expired entries from set, destroy start and end elements
15421553
* @set: nftables API set representation
15431554
* @m: Matching data
15441555
*/
1545-
static void pipapo_gc(const struct nft_set *set, struct nft_pipapo_match *m)
1556+
static void pipapo_gc(const struct nft_set *_set, struct nft_pipapo_match *m)
15461557
{
1558+
struct nft_set *set = (struct nft_set *) _set;
15471559
struct nft_pipapo *priv = nft_set_priv(set);
1560+
struct net *net = read_pnet(&set->net);
15481561
int rules_f0, first_rule = 0;
15491562
struct nft_pipapo_elem *e;
1563+
struct nft_trans_gc *gc;
1564+
1565+
gc = nft_trans_gc_alloc(set, 0, GFP_KERNEL);
1566+
if (!gc)
1567+
return;
15501568

15511569
while ((rules_f0 = pipapo_rules_same_key(m->f, first_rule))) {
15521570
union nft_pipapo_map_bucket rulemap[NFT_PIPAPO_MAX_FIELDS];
@@ -1570,13 +1588,20 @@ static void pipapo_gc(const struct nft_set *set, struct nft_pipapo_match *m)
15701588
f--;
15711589
i--;
15721590
e = f->mt[rulemap[i].to].e;
1573-
if (nft_set_elem_expired(&e->ext) &&
1574-
!nft_set_elem_mark_busy(&e->ext)) {
1591+
1592+
/* synchronous gc never fails, there is no need to set on
1593+
* NFT_SET_ELEM_DEAD_BIT.
1594+
*/
1595+
if (nft_set_elem_expired(&e->ext)) {
15751596
priv->dirty = true;
1576-
pipapo_drop(m, rulemap);
15771597

1578-
rcu_barrier();
1579-
nft_set_elem_destroy(set, e, true);
1598+
gc = nft_trans_gc_queue_sync(gc, GFP_ATOMIC);
1599+
if (!gc)
1600+
break;
1601+
1602+
nft_pipapo_gc_deactivate(net, set, e);
1603+
pipapo_drop(m, rulemap);
1604+
nft_trans_gc_elem_add(gc, e);
15801605

15811606
/* And check again current first rule, which is now the
15821607
* first we haven't checked.
@@ -1586,11 +1611,11 @@ static void pipapo_gc(const struct nft_set *set, struct nft_pipapo_match *m)
15861611
}
15871612
}
15881613

1589-
e = nft_set_catchall_gc(set);
1590-
if (e)
1591-
nft_set_elem_destroy(set, e, true);
1592-
1593-
priv->last_gc = jiffies;
1614+
gc = nft_trans_gc_catchall(gc, 0);
1615+
if (gc) {
1616+
nft_trans_gc_queue_sync_done(gc);
1617+
priv->last_gc = jiffies;
1618+
}
15941619
}
15951620

15961621
/**
@@ -1715,7 +1740,6 @@ static void nft_pipapo_activate(const struct net *net,
17151740
return;
17161741

17171742
nft_set_elem_change_active(net, set, &e->ext);
1718-
nft_set_elem_clear_busy(&e->ext);
17191743
}
17201744

17211745
/**

0 commit comments

Comments
 (0)