stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: netfilter-devel@vger.kernel.org
Cc: stable@vger.kernel.org, gregkh@linuxfoundation.org, sashal@kernel.org
Subject: [-stable,6.1 17/17] netfilter: nf_tables: fix memleak when more than 255 elements expired
Date: Fri, 22 Sep 2023 18:30:29 +0200	[thread overview]
Message-ID: <20230922163029.150988-18-pablo@netfilter.org> (raw)
In-Reply-To: <20230922163029.150988-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

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>
---
 include/net/netfilter/nf_tables.h |  2 +-
 net/netfilter/nf_tables_api.c     | 10 ++++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index eb2103a9a7dd..05d7a60a0e1f 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1657,7 +1657,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 6e67fb999a25..b22f2d9ee4af 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -9190,12 +9190,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)
@@ -9210,15 +9213,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)
-- 
2.30.2


      parent reply	other threads:[~2023-09-22 16:31 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-22 16:30 [PATCH -stable,6.1 00/17] Netfilter stable fixes for 6.1 Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 01/17] netfilter: nf_tables: don't skip expired elements during walk Pablo Neira Ayuso
2023-09-22 16:31   ` kernel test robot
2023-09-22 16:30 ` [-stable,6.1 02/17] netfilter: nf_tables: GC transaction API to avoid race with control plane Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 03/17] netfilter: nf_tables: adapt set backend to use GC transaction API Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 04/17] netfilter: nft_set_hash: mark set element as dead when deleting from packet path Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 05/17] netfilter: nf_tables: remove busy mark and gc batch API Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 06/17] netfilter: nf_tables: don't fail inserts if duplicate has expired Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 07/17] netfilter: nf_tables: fix GC transaction races with netns and netlink event exit path Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 08/17] netfilter: nf_tables: GC transaction race with netns dismantle Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 09/17] netfilter: nf_tables: GC transaction race with abort path Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 10/17] netfilter: nf_tables: use correct lock to protect gc_list Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 11/17] netfilter: nf_tables: defer gc run if previous batch is still pending Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 12/17] netfilter: nft_set_rbtree: skip sync GC for new elements in this transaction Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 13/17] netfilter: nft_set_rbtree: use read spinlock to avoid datapath contention Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 14/17] netfilter: nft_set_pipapo: call nft_trans_gc_queue_sync() in catchall GC Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 15/17] netfilter: nft_set_pipapo: stop GC iteration if GC transaction allocation fails Pablo Neira Ayuso
2023-09-22 16:30 ` [-stable,6.1 16/17] netfilter: nft_set_hash: try later when GC hits EAGAIN on iteration Pablo Neira Ayuso
2023-09-22 16:30 ` Pablo Neira Ayuso [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230922163029.150988-18-pablo@netfilter.org \
    --to=pablo@netfilter.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).