Skip to content

Commit 5f68718

Browse files
committed
netfilter: nf_tables: GC transaction API to avoid race with control plane
The set types rhashtable and rbtree use a GC worker to reclaim memory. From system work queue, in periodic intervals, a scan of the table is done. The major caveat here is that the nft transaction mutex is not held. This causes a race between control plane and GC when they attempt to delete the same element. We cannot grab the netlink mutex from the work queue, because the control plane has to wait for the GC work queue in case the set is to be removed, so we get following deadlock: cpu 1 cpu2 GC work transaction comes in , lock nft mutex `acquire nft mutex // BLOCKS transaction asks to remove the set set destruction calls cancel_work_sync() cancel_work_sync will now block forever, because it is waiting for the mutex the caller already owns. This patch adds a new API that deals with garbage collection in two steps: 1) Lockless GC of expired elements sets on the NFT_SET_ELEM_DEAD_BIT so they are not visible via lookup. Annotate current GC sequence in the GC transaction. Enqueue GC transaction work as soon as it is full. If ruleset is updated, then GC transaction is aborted and retried later. 2) GC work grabs the mutex. If GC sequence has changed then this GC transaction lost race with control plane, abort it as it contains stale references to objects and let GC try again later. If the ruleset is intact, then this GC transaction deactivates and removes the elements and it uses call_rcu() to destroy elements. Note that no elements are removed from GC lockless path, the _DEAD bit is set and pointers are collected. GC catchall does not remove the elements anymore too. There is a new set->dead flag that is set on to abort the GC transaction to deal with set->ops->destroy() path which removes the remaining elements in the set from commit_release, where no mutex is held. To deal with GC when mutex is held, which allows safe deactivate and removal, add sync GC API which releases the set element object via call_rcu(). This is used by rbtree and pipapo backends which also perform garbage collection from control plane path. Since element removal from sets can happen from control plane and element garbage collection/timeout, it is necessary to keep the set structure alive until all elements have been deactivated and destroyed. We cannot do a cancel_work_sync or flush_work in nft_set_destroy because its called with the transaction mutex held, but the aforementioned async work queue might be blocked on the very mutex that nft_set_destroy() callchain is sitting on. This gives us the choice of ABBA deadlock or UaF. To avoid both, add set->refs refcount_t member. The GC API can then increment the set refcount and release it once the elements have been free'd. Set backends are adapted to use the GC transaction API in a follow up patch entitled: ("netfilter: nf_tables: use gc transaction API in set backends") This is joint work with Florian Westphal. Fixes: cfed7e1 ("netfilter: nf_tables: add set garbage collection helpers") Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent 2413893 commit 5f68718

File tree

2 files changed

+300
-12
lines changed

2 files changed

+300
-12
lines changed

include/net/netfilter/nf_tables.h

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,7 @@ struct nft_set_elem_expr {
512512
*
513513
* @list: table set list node
514514
* @bindings: list of set bindings
515+
* @refs: internal refcounting for async set destruction
515516
* @table: table this set belongs to
516517
* @net: netnamespace this set belongs to
517518
* @name: name of the set
@@ -541,6 +542,7 @@ struct nft_set_elem_expr {
541542
struct nft_set {
542543
struct list_head list;
543544
struct list_head bindings;
545+
refcount_t refs;
544546
struct nft_table *table;
545547
possible_net_t net;
546548
char *name;
@@ -562,7 +564,8 @@ struct nft_set {
562564
struct list_head pending_update;
563565
/* runtime data below here */
564566
const struct nft_set_ops *ops ____cacheline_aligned;
565-
u16 flags:14,
567+
u16 flags:13,
568+
dead:1,
566569
genmask:2;
567570
u8 klen;
568571
u8 dlen;
@@ -1592,6 +1595,32 @@ static inline void nft_set_elem_clear_busy(struct nft_set_ext *ext)
15921595
clear_bit(NFT_SET_ELEM_BUSY_BIT, word);
15931596
}
15941597

1598+
#define NFT_SET_ELEM_DEAD_MASK (1 << 3)
1599+
1600+
#if defined(__LITTLE_ENDIAN_BITFIELD)
1601+
#define NFT_SET_ELEM_DEAD_BIT 3
1602+
#elif defined(__BIG_ENDIAN_BITFIELD)
1603+
#define NFT_SET_ELEM_DEAD_BIT (BITS_PER_LONG - BITS_PER_BYTE + 3)
1604+
#else
1605+
#error
1606+
#endif
1607+
1608+
static inline void nft_set_elem_dead(struct nft_set_ext *ext)
1609+
{
1610+
unsigned long *word = (unsigned long *)ext;
1611+
1612+
BUILD_BUG_ON(offsetof(struct nft_set_ext, genmask) != 0);
1613+
set_bit(NFT_SET_ELEM_DEAD_BIT, word);
1614+
}
1615+
1616+
static inline int nft_set_elem_is_dead(const struct nft_set_ext *ext)
1617+
{
1618+
unsigned long *word = (unsigned long *)ext;
1619+
1620+
BUILD_BUG_ON(offsetof(struct nft_set_ext, genmask) != 0);
1621+
return test_bit(NFT_SET_ELEM_DEAD_BIT, word);
1622+
}
1623+
15951624
/**
15961625
* struct nft_trans - nf_tables object update in transaction
15971626
*
@@ -1732,6 +1761,38 @@ struct nft_trans_flowtable {
17321761
#define nft_trans_flowtable_flags(trans) \
17331762
(((struct nft_trans_flowtable *)trans->data)->flags)
17341763

1764+
#define NFT_TRANS_GC_BATCHCOUNT 256
1765+
1766+
struct nft_trans_gc {
1767+
struct list_head list;
1768+
struct net *net;
1769+
struct nft_set *set;
1770+
u32 seq;
1771+
u8 count;
1772+
void *priv[NFT_TRANS_GC_BATCHCOUNT];
1773+
struct rcu_head rcu;
1774+
};
1775+
1776+
struct nft_trans_gc *nft_trans_gc_alloc(struct nft_set *set,
1777+
unsigned int gc_seq, gfp_t gfp);
1778+
void nft_trans_gc_destroy(struct nft_trans_gc *trans);
1779+
1780+
struct nft_trans_gc *nft_trans_gc_queue_async(struct nft_trans_gc *gc,
1781+
unsigned int gc_seq, gfp_t gfp);
1782+
void nft_trans_gc_queue_async_done(struct nft_trans_gc *gc);
1783+
1784+
struct nft_trans_gc *nft_trans_gc_queue_sync(struct nft_trans_gc *gc, gfp_t gfp);
1785+
void nft_trans_gc_queue_sync_done(struct nft_trans_gc *trans);
1786+
1787+
void nft_trans_gc_elem_add(struct nft_trans_gc *gc, void *priv);
1788+
1789+
struct nft_trans_gc *nft_trans_gc_catchall(struct nft_trans_gc *gc,
1790+
unsigned int gc_seq);
1791+
1792+
void nft_setelem_data_deactivate(const struct net *net,
1793+
const struct nft_set *set,
1794+
struct nft_set_elem *elem);
1795+
17351796
int __init nft_chain_filter_init(void);
17361797
void nft_chain_filter_fini(void);
17371798

@@ -1758,6 +1819,7 @@ struct nftables_pernet {
17581819
struct mutex commit_mutex;
17591820
u64 table_handle;
17601821
unsigned int base_seq;
1822+
unsigned int gc_seq;
17611823
};
17621824

17631825
extern unsigned int nf_tables_net_id;

0 commit comments

Comments
 (0)