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

commit cf5000a7787cbc10341091d37245a42c119d26c5 upstream.

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: 5f68718b34a5 ("netfilter: nf_tables: GC transaction API to avoid race with control plane")
Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Gbp-Pq: Topic bugfix/all
Gbp-Pq: Name netfilter-nf_tables-fix-memleak-when-more-than-255-e.patch

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index bbe472c07d07eaea46b0ae2213e445481da6517c..5619642b9ad471d7f97eac862b88c5dd70223c90 100644 (file)
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1525,7 +1525,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;
 };

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 9fc302a6836ba3623bd5f320f595280a16fe214c..32c97cc87ddc211ba79d542b4ead70782f649632 100644 (file)
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -8124,12 +8124,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)
@@ -8144,15 +8147,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)