Netfilter-Devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH -stable 6.1.x 0/3] Netfilter fixes for -stable
@ 2024-04-08 21:18 Pablo Neira Ayuso
  2024-04-08 21:18 ` [PATCH -stable 6.1.x 1/3] netfilter: nf_tables: release batch on table validation from abort path Pablo Neira Ayuso
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-08 21:18 UTC (permalink / raw
  To: netfilter-devel; +Cc: stable, gregkh, sashal

Hi Greg, Sasha,

This batch contains a backport for recent fixes already upstream for 6.1.x,
to add them on top of enqueued patches:

a45e6889575c ("netfilter: nf_tables: release batch on table validation from abort path")
0d459e2ffb54 ("netfilter: nf_tables: release mutex after nft_gc_seq_end from abort path")
1bc83a019bbe ("netfilter: nf_tables: discard table flag update with pending basechain deletion")

Please, apply, thanks.

Pablo Neira Ayuso (3):
  netfilter: nf_tables: release batch on table validation from abort path
  netfilter: nf_tables: release mutex after nft_gc_seq_end from abort path
  netfilter: nf_tables: discard table flag update with pending basechain deletion

 net/netfilter/nf_tables_api.c | 47 +++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 11 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH -stable 6.1.x 1/3] netfilter: nf_tables: release batch on table validation from abort path
  2024-04-08 21:18 [PATCH -stable 6.1.x 0/3] Netfilter fixes for -stable Pablo Neira Ayuso
@ 2024-04-08 21:18 ` Pablo Neira Ayuso
  2024-04-08 21:18 ` [PATCH -stable 6.1.x 2/3] netfilter: nf_tables: release mutex after nft_gc_seq_end " Pablo Neira Ayuso
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-08 21:18 UTC (permalink / raw
  To: netfilter-devel; +Cc: stable, gregkh, sashal

commit a45e6889575c2067d3c0212b6bc1022891e65b91 upstream.

Unlike early commit path stage which triggers a call to abort, an
explicit release of the batch is required on abort, otherwise mutex is
released and commit_list remains in place.

Add WARN_ON_ONCE to ensure commit_list is empty from the abort path
before releasing the mutex.

After this patch, commit_list is always assumed to be empty before
grabbing the mutex, therefore

  03c1f1ef1584 ("netfilter: Cleanup nft_net->module_list from nf_tables_exit_net()")

only needs to release the pending modules for registration.

Cc: stable@vger.kernel.org
Fixes: c0391b6ab810 ("netfilter: nf_tables: missing validation from the abort path")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8d38cd504769..6b032a90e2b1 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -9902,10 +9902,11 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 	struct nft_trans *trans, *next;
 	LIST_HEAD(set_update_list);
 	struct nft_trans_elem *te;
+	int err = 0;
 
 	if (action == NFNL_ABORT_VALIDATE &&
 	    nf_tables_validate(net) < 0)
-		return -EAGAIN;
+		err = -EAGAIN;
 
 	list_for_each_entry_safe_reverse(trans, next, &nft_net->commit_list,
 					 list) {
@@ -10081,7 +10082,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 	else
 		nf_tables_module_autoload_cleanup(net);
 
-	return 0;
+	return err;
 }
 
 static int nf_tables_abort(struct net *net, struct sk_buff *skb,
@@ -10095,6 +10096,8 @@ static int nf_tables_abort(struct net *net, struct sk_buff *skb,
 	ret = __nf_tables_abort(net, action);
 	nft_gc_seq_end(nft_net, gc_seq);
 
+	WARN_ON_ONCE(!list_empty(&nft_net->commit_list));
+
 	mutex_unlock(&nft_net->commit_mutex);
 
 	return ret;
@@ -10892,9 +10895,10 @@ static void __net_exit nf_tables_exit_net(struct net *net)
 
 	gc_seq = nft_gc_seq_begin(nft_net);
 
-	if (!list_empty(&nft_net->commit_list) ||
-	    !list_empty(&nft_net->module_list))
-		__nf_tables_abort(net, NFNL_ABORT_NONE);
+	WARN_ON_ONCE(!list_empty(&nft_net->commit_list));
+
+	if (!list_empty(&nft_net->module_list))
+		nf_tables_module_autoload_cleanup(net);
 
 	__nft_release_tables(net);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH -stable 6.1.x 2/3] netfilter: nf_tables: release mutex after nft_gc_seq_end from abort path
  2024-04-08 21:18 [PATCH -stable 6.1.x 0/3] Netfilter fixes for -stable Pablo Neira Ayuso
  2024-04-08 21:18 ` [PATCH -stable 6.1.x 1/3] netfilter: nf_tables: release batch on table validation from abort path Pablo Neira Ayuso
@ 2024-04-08 21:18 ` Pablo Neira Ayuso
  2024-04-08 21:18 ` [PATCH -stable 6.1.x 3/3] netfilter: nf_tables: discard table flag update with pending basechain deletion Pablo Neira Ayuso
  2024-04-10 15:58 ` [PATCH -stable 6.1.x 0/3] Netfilter fixes for -stable Sasha Levin
  3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-08 21:18 UTC (permalink / raw
  To: netfilter-devel; +Cc: stable, gregkh, sashal

commit 0d459e2ffb541841714839e8228b845458ed3b27 upstream.

The commit mutex should not be released during the critical section
between nft_gc_seq_begin() and nft_gc_seq_end(), otherwise, async GC
worker could collect expired objects and get the released commit lock
within the same GC sequence.

nf_tables_module_autoload() temporarily releases the mutex to load
module dependencies, then it goes back to replay the transaction again.
Move it at the end of the abort phase after nft_gc_seq_end() is called.

Cc: stable@vger.kernel.org
Fixes: 720344340fb9 ("netfilter: nf_tables: GC transaction race with abort path")
Reported-by: Kuan-Ting Chen <hexrabbit@devco.re>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 6b032a90e2b1..e7b31c2c92df 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -10077,11 +10077,6 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 		nf_tables_abort_release(trans);
 	}
 
-	if (action == NFNL_ABORT_AUTOLOAD)
-		nf_tables_module_autoload(net);
-	else
-		nf_tables_module_autoload_cleanup(net);
-
 	return err;
 }
 
@@ -10098,6 +10093,14 @@ static int nf_tables_abort(struct net *net, struct sk_buff *skb,
 
 	WARN_ON_ONCE(!list_empty(&nft_net->commit_list));
 
+	/* module autoload needs to happen after GC sequence update because it
+	 * temporarily releases and grabs mutex again.
+	 */
+	if (action == NFNL_ABORT_AUTOLOAD)
+		nf_tables_module_autoload(net);
+	else
+		nf_tables_module_autoload_cleanup(net);
+
 	mutex_unlock(&nft_net->commit_mutex);
 
 	return ret;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH -stable 6.1.x 3/3] netfilter: nf_tables: discard table flag update with pending basechain deletion
  2024-04-08 21:18 [PATCH -stable 6.1.x 0/3] Netfilter fixes for -stable Pablo Neira Ayuso
  2024-04-08 21:18 ` [PATCH -stable 6.1.x 1/3] netfilter: nf_tables: release batch on table validation from abort path Pablo Neira Ayuso
  2024-04-08 21:18 ` [PATCH -stable 6.1.x 2/3] netfilter: nf_tables: release mutex after nft_gc_seq_end " Pablo Neira Ayuso
@ 2024-04-08 21:18 ` Pablo Neira Ayuso
  2024-04-10 15:58 ` [PATCH -stable 6.1.x 0/3] Netfilter fixes for -stable Sasha Levin
  3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-08 21:18 UTC (permalink / raw
  To: netfilter-devel; +Cc: stable, gregkh, sashal

commit 1bc83a019bbe268be3526406245ec28c2458a518 upstream.

Hook unregistration is deferred to the commit phase, same occurs with
hook updates triggered by the table dormant flag. When both commands are
combined, this results in deleting a basechain while leaving its hook
still registered in the core.

Fixes: 179d9ba5559a ("netfilter: nf_tables: fix table flag updates")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index e7b31c2c92df..8152a69d8268 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1192,6 +1192,24 @@ static void nf_tables_table_disable(struct net *net, struct nft_table *table)
 #define __NFT_TABLE_F_UPDATE		(__NFT_TABLE_F_WAS_DORMANT | \
 					 __NFT_TABLE_F_WAS_AWAKEN)
 
+static bool nft_table_pending_update(const struct nft_ctx *ctx)
+{
+	struct nftables_pernet *nft_net = nft_pernet(ctx->net);
+	struct nft_trans *trans;
+
+	if (ctx->table->flags & __NFT_TABLE_F_UPDATE)
+		return true;
+
+	list_for_each_entry(trans, &nft_net->commit_list, list) {
+		if (trans->ctx.table == ctx->table &&
+		    trans->msg_type == NFT_MSG_DELCHAIN &&
+		    nft_is_base_chain(trans->ctx.chain))
+			return true;
+	}
+
+	return false;
+}
+
 static int nf_tables_updtable(struct nft_ctx *ctx)
 {
 	struct nft_trans *trans;
@@ -1215,7 +1233,7 @@ static int nf_tables_updtable(struct nft_ctx *ctx)
 		return -EOPNOTSUPP;
 
 	/* No dormant off/on/off/on games in single transaction */
-	if (ctx->table->flags & __NFT_TABLE_F_UPDATE)
+	if (nft_table_pending_update(ctx))
 		return -EINVAL;
 
 	trans = nft_trans_alloc(ctx, NFT_MSG_NEWTABLE,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH -stable 6.1.x 0/3] Netfilter fixes for -stable
  2024-04-08 21:18 [PATCH -stable 6.1.x 0/3] Netfilter fixes for -stable Pablo Neira Ayuso
                   ` (2 preceding siblings ...)
  2024-04-08 21:18 ` [PATCH -stable 6.1.x 3/3] netfilter: nf_tables: discard table flag update with pending basechain deletion Pablo Neira Ayuso
@ 2024-04-10 15:58 ` Sasha Levin
  3 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2024-04-10 15:58 UTC (permalink / raw
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, stable, gregkh

On Mon, Apr 08, 2024 at 11:18:31PM +0200, Pablo Neira Ayuso wrote:
>Hi Greg, Sasha,
>
>This batch contains a backport for recent fixes already upstream for 6.1.x,
>to add them on top of enqueued patches:
>
>a45e6889575c ("netfilter: nf_tables: release batch on table validation from abort path")
>0d459e2ffb54 ("netfilter: nf_tables: release mutex after nft_gc_seq_end from abort path")
>1bc83a019bbe ("netfilter: nf_tables: discard table flag update with pending basechain deletion")
>
>Please, apply, thanks.

Queued up (this and for other trees), thanks!

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-04-10 15:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-08 21:18 [PATCH -stable 6.1.x 0/3] Netfilter fixes for -stable Pablo Neira Ayuso
2024-04-08 21:18 ` [PATCH -stable 6.1.x 1/3] netfilter: nf_tables: release batch on table validation from abort path Pablo Neira Ayuso
2024-04-08 21:18 ` [PATCH -stable 6.1.x 2/3] netfilter: nf_tables: release mutex after nft_gc_seq_end " Pablo Neira Ayuso
2024-04-08 21:18 ` [PATCH -stable 6.1.x 3/3] netfilter: nf_tables: discard table flag update with pending basechain deletion Pablo Neira Ayuso
2024-04-10 15:58 ` [PATCH -stable 6.1.x 0/3] Netfilter fixes for -stable Sasha Levin

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 read-only IMAP folder(s) and NNTP newsgroup(s).