Commit cf5000a7 authored by Florian Westphal's avatar Florian Westphal
Browse files

netfilter: nf_tables: fix memleak when more than 255 elements expired 



When more than 255 elements expired we're supposed to switch to a new gc
container structure.

This never happens: u8 type will wrap before reaching the boundary
and nft_trans_gc_space() always returns true.

This means we recycle the initial gc container structure and
lose track of the elements that came before.

While at it, don't deref 'gc' after we've passed it to call_rcu.

Fixes: 5f68718b ("netfilter: nf_tables: GC transaction API to avoid race with control plane")
Reported-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
parent c9bd2651

include/net/netfilter/nf_tables.h

+1 −1
Original line number Diff line number Diff line
@@ -1682,7 +1682,7 @@ struct nft_trans_gc { 
	struct net		*net;

	struct nft_set		*set;

	u32			seq;

	u8			count;

	u16			count;

	void			*priv[NFT_TRANS_GC_BATCHCOUNT];

	struct rcu_head		rcu;

};

net/netfilter/nf_tables_api.c

+8 −2
Original line number Diff line number Diff line
@@ -9579,12 +9579,15 @@ static int nft_trans_gc_space(struct nft_trans_gc *trans) 
struct nft_trans_gc *nft_trans_gc_queue_async(struct nft_trans_gc *gc,

					      unsigned int gc_seq, gfp_t gfp)

{

	struct nft_set *set;



	if (nft_trans_gc_space(gc))

		return gc;



	set = gc->set;

	nft_trans_gc_queue_work(gc);



	return nft_trans_gc_alloc(gc->set, gc_seq, gfp);

	return nft_trans_gc_alloc(set, gc_seq, gfp);

}



void nft_trans_gc_queue_async_done(struct nft_trans_gc *trans)
@@ -9599,15 +9602,18 @@ void nft_trans_gc_queue_async_done(struct nft_trans_gc *trans) 


struct nft_trans_gc *nft_trans_gc_queue_sync(struct nft_trans_gc *gc, gfp_t gfp)

{

	struct nft_set *set;



	if (WARN_ON_ONCE(!lockdep_commit_lock_is_held(gc->net)))

		return NULL;



	if (nft_trans_gc_space(gc))

		return gc;



	set = gc->set;

	call_rcu(&gc->rcu, nft_trans_gc_trans_free);



	return nft_trans_gc_alloc(gc->set, 0, gfp);

	return nft_trans_gc_alloc(set, 0, gfp);

}



void nft_trans_gc_queue_sync_done(struct nft_trans_gc *trans)