Netfilter-Devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/6] Netfilter fixes for net
@ 2024-04-04 10:43 Pablo Neira Ayuso
  2024-04-04 10:43 ` [PATCH net 1/6] netfilter: nf_tables: release batch on table validation from abort path Pablo Neira Ayuso
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-04 10:43 UTC (permalink / raw
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet

Hi,

The following patchset contains Netfilter fixes for net:

Patch #1 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.

Patch #2 release mutex after nft_gc_seq_end() in commit path, otherwise
         async GC worker could collect expired objects.

Patch #3 flush pending destroy work in module removal path, otherwise UaF
         is possible.

Patch #4 and #6 restrict the table dormant flag with basechain updates
	 to fix state inconsistency in the hook registration.

Patch #5 adds missing RCU read side lock to flowtable type to avoid races
	 with module removal.

Please, pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-24-04-04

Thanks.

----------------------------------------------------------------

The following changes since commit 72076fc9fe60b9143cd971fd8737718719bc512e:

  Revert "tg3: Remove residual error handling in tg3_suspend" (2024-04-04 10:51:01 +0200)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git tags/nf-24-04-04

for you to fetch changes up to 1bc83a019bbe268be3526406245ec28c2458a518:

  netfilter: nf_tables: discard table flag update with pending basechain deletion (2024-04-04 11:38:35 +0200)

----------------------------------------------------------------
netfilter pull request 24-04-04

----------------------------------------------------------------
Pablo Neira Ayuso (5):
      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: flush pending destroy work before exit_net release
      netfilter: nf_tables: reject new basechain after table flag update
      netfilter: nf_tables: discard table flag update with pending basechain deletion

Ziyang Xuan (1):
      netfilter: nf_tables: Fix potential data-race in __nft_flowtable_type_get()

 net/netfilter/nf_tables_api.c | 50 +++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 16 deletions(-)

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

* [PATCH net 1/6] netfilter: nf_tables: release batch on table validation from abort path
  2024-04-04 10:43 [PATCH net 0/6] Netfilter fixes for net Pablo Neira Ayuso
@ 2024-04-04 10:43 ` Pablo Neira Ayuso
  2024-04-04 16:50   ` patchwork-bot+netdevbpf
  2024-04-04 10:43 ` [PATCH net 2/6] netfilter: nf_tables: release mutex after nft_gc_seq_end " Pablo Neira Ayuso
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-04 10:43 UTC (permalink / raw
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet

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 | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index fd86f2720c9e..ffcd3213c335 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -10455,10 +10455,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) {
@@ -10655,7 +10656,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,
@@ -10668,6 +10669,9 @@ static int nf_tables_abort(struct net *net, struct sk_buff *skb,
 	gc_seq = nft_gc_seq_begin(nft_net);
 	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;
@@ -11473,9 +11477,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] 9+ messages in thread

* [PATCH net 2/6] netfilter: nf_tables: release mutex after nft_gc_seq_end from abort path
  2024-04-04 10:43 [PATCH net 0/6] Netfilter fixes for net Pablo Neira Ayuso
  2024-04-04 10:43 ` [PATCH net 1/6] netfilter: nf_tables: release batch on table validation from abort path Pablo Neira Ayuso
@ 2024-04-04 10:43 ` Pablo Neira Ayuso
  2024-04-04 10:43 ` [PATCH net 3/6] netfilter: nf_tables: flush pending destroy work before exit_net release Pablo Neira Ayuso
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-04 10:43 UTC (permalink / raw
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet

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 ffcd3213c335..0d432d0674e1 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -10651,11 +10651,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;
 }
 
@@ -10672,6 +10667,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] 9+ messages in thread

* [PATCH net 3/6] netfilter: nf_tables: flush pending destroy work before exit_net release
  2024-04-04 10:43 [PATCH net 0/6] Netfilter fixes for net Pablo Neira Ayuso
  2024-04-04 10:43 ` [PATCH net 1/6] netfilter: nf_tables: release batch on table validation from abort path Pablo Neira Ayuso
  2024-04-04 10:43 ` [PATCH net 2/6] netfilter: nf_tables: release mutex after nft_gc_seq_end " Pablo Neira Ayuso
@ 2024-04-04 10:43 ` Pablo Neira Ayuso
  2024-04-04 10:43 ` [PATCH net 4/6] netfilter: nf_tables: reject new basechain after table flag update Pablo Neira Ayuso
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-04 10:43 UTC (permalink / raw
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet

Similar to 2c9f0293280e ("netfilter: nf_tables: flush pending destroy
work before netlink notifier") to address a race between exit_net and
the destroy workqueue.

The trace below shows an element to be released via destroy workqueue
while exit_net path (triggered via module removal) has already released
the set that is used in such transaction.

[ 1360.547789] BUG: KASAN: slab-use-after-free in nf_tables_trans_destroy_work+0x3f5/0x590 [nf_tables]
[ 1360.547861] Read of size 8 at addr ffff888140500cc0 by task kworker/4:1/152465
[ 1360.547870] CPU: 4 PID: 152465 Comm: kworker/4:1 Not tainted 6.8.0+ #359
[ 1360.547882] Workqueue: events nf_tables_trans_destroy_work [nf_tables]
[ 1360.547984] Call Trace:
[ 1360.547991]  <TASK>
[ 1360.547998]  dump_stack_lvl+0x53/0x70
[ 1360.548014]  print_report+0xc4/0x610
[ 1360.548026]  ? __virt_addr_valid+0xba/0x160
[ 1360.548040]  ? __pfx__raw_spin_lock_irqsave+0x10/0x10
[ 1360.548054]  ? nf_tables_trans_destroy_work+0x3f5/0x590 [nf_tables]
[ 1360.548176]  kasan_report+0xae/0xe0
[ 1360.548189]  ? nf_tables_trans_destroy_work+0x3f5/0x590 [nf_tables]
[ 1360.548312]  nf_tables_trans_destroy_work+0x3f5/0x590 [nf_tables]
[ 1360.548447]  ? __pfx_nf_tables_trans_destroy_work+0x10/0x10 [nf_tables]
[ 1360.548577]  ? _raw_spin_unlock_irq+0x18/0x30
[ 1360.548591]  process_one_work+0x2f1/0x670
[ 1360.548610]  worker_thread+0x4d3/0x760
[ 1360.548627]  ? __pfx_worker_thread+0x10/0x10
[ 1360.548640]  kthread+0x16b/0x1b0
[ 1360.548653]  ? __pfx_kthread+0x10/0x10
[ 1360.548665]  ret_from_fork+0x2f/0x50
[ 1360.548679]  ? __pfx_kthread+0x10/0x10
[ 1360.548690]  ret_from_fork_asm+0x1a/0x30
[ 1360.548707]  </TASK>

[ 1360.548719] Allocated by task 192061:
[ 1360.548726]  kasan_save_stack+0x20/0x40
[ 1360.548739]  kasan_save_track+0x14/0x30
[ 1360.548750]  __kasan_kmalloc+0x8f/0xa0
[ 1360.548760]  __kmalloc_node+0x1f1/0x450
[ 1360.548771]  nf_tables_newset+0x10c7/0x1b50 [nf_tables]
[ 1360.548883]  nfnetlink_rcv_batch+0xbc4/0xdc0 [nfnetlink]
[ 1360.548909]  nfnetlink_rcv+0x1a8/0x1e0 [nfnetlink]
[ 1360.548927]  netlink_unicast+0x367/0x4f0
[ 1360.548935]  netlink_sendmsg+0x34b/0x610
[ 1360.548944]  ____sys_sendmsg+0x4d4/0x510
[ 1360.548953]  ___sys_sendmsg+0xc9/0x120
[ 1360.548961]  __sys_sendmsg+0xbe/0x140
[ 1360.548971]  do_syscall_64+0x55/0x120
[ 1360.548982]  entry_SYSCALL_64_after_hwframe+0x55/0x5d

[ 1360.548994] Freed by task 192222:
[ 1360.548999]  kasan_save_stack+0x20/0x40
[ 1360.549009]  kasan_save_track+0x14/0x30
[ 1360.549019]  kasan_save_free_info+0x3b/0x60
[ 1360.549028]  poison_slab_object+0x100/0x180
[ 1360.549036]  __kasan_slab_free+0x14/0x30
[ 1360.549042]  kfree+0xb6/0x260
[ 1360.549049]  __nft_release_table+0x473/0x6a0 [nf_tables]
[ 1360.549131]  nf_tables_exit_net+0x170/0x240 [nf_tables]
[ 1360.549221]  ops_exit_list+0x50/0xa0
[ 1360.549229]  free_exit_list+0x101/0x140
[ 1360.549236]  unregister_pernet_operations+0x107/0x160
[ 1360.549245]  unregister_pernet_subsys+0x1c/0x30
[ 1360.549254]  nf_tables_module_exit+0x43/0x80 [nf_tables]
[ 1360.549345]  __do_sys_delete_module+0x253/0x370
[ 1360.549352]  do_syscall_64+0x55/0x120
[ 1360.549360]  entry_SYSCALL_64_after_hwframe+0x55/0x5d

(gdb) list *__nft_release_table+0x473
0x1e033 is in __nft_release_table (net/netfilter/nf_tables_api.c:11354).
11349           list_for_each_entry_safe(flowtable, nf, &table->flowtables, list) {
11350                   list_del(&flowtable->list);
11351                   nft_use_dec(&table->use);
11352                   nf_tables_flowtable_destroy(flowtable);
11353           }
11354           list_for_each_entry_safe(set, ns, &table->sets, list) {
11355                   list_del(&set->list);
11356                   nft_use_dec(&table->use);
11357                   if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
11358                           nft_map_deactivate(&ctx, set);
(gdb)

[ 1360.549372] Last potentially related work creation:
[ 1360.549376]  kasan_save_stack+0x20/0x40
[ 1360.549384]  __kasan_record_aux_stack+0x9b/0xb0
[ 1360.549392]  __queue_work+0x3fb/0x780
[ 1360.549399]  queue_work_on+0x4f/0x60
[ 1360.549407]  nft_rhash_remove+0x33b/0x340 [nf_tables]
[ 1360.549516]  nf_tables_commit+0x1c6a/0x2620 [nf_tables]
[ 1360.549625]  nfnetlink_rcv_batch+0x728/0xdc0 [nfnetlink]
[ 1360.549647]  nfnetlink_rcv+0x1a8/0x1e0 [nfnetlink]
[ 1360.549671]  netlink_unicast+0x367/0x4f0
[ 1360.549680]  netlink_sendmsg+0x34b/0x610
[ 1360.549690]  ____sys_sendmsg+0x4d4/0x510
[ 1360.549697]  ___sys_sendmsg+0xc9/0x120
[ 1360.549706]  __sys_sendmsg+0xbe/0x140
[ 1360.549715]  do_syscall_64+0x55/0x120
[ 1360.549725]  entry_SYSCALL_64_after_hwframe+0x55/0x5d

Fixes: 0935d5588400 ("netfilter: nf_tables: asynchronous release")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 0d432d0674e1..17bf53cd0e84 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -11575,6 +11575,7 @@ static void __exit nf_tables_module_exit(void)
 	unregister_netdevice_notifier(&nf_tables_flowtable_notifier);
 	nft_chain_filter_fini();
 	nft_chain_route_fini();
+	nf_tables_trans_destroy_flush_work();
 	unregister_pernet_subsys(&nf_tables_net_ops);
 	cancel_work_sync(&trans_gc_work);
 	cancel_work_sync(&trans_destroy_work);
-- 
2.30.2


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

* [PATCH net 4/6] netfilter: nf_tables: reject new basechain after table flag update
  2024-04-04 10:43 [PATCH net 0/6] Netfilter fixes for net Pablo Neira Ayuso
                   ` (2 preceding siblings ...)
  2024-04-04 10:43 ` [PATCH net 3/6] netfilter: nf_tables: flush pending destroy work before exit_net release Pablo Neira Ayuso
@ 2024-04-04 10:43 ` Pablo Neira Ayuso
  2024-04-04 10:43 ` [PATCH net 5/6] netfilter: nf_tables: Fix potential data-race in __nft_flowtable_type_get() Pablo Neira Ayuso
  2024-04-04 10:43 ` [PATCH net 6/6] netfilter: nf_tables: discard table flag update with pending basechain deletion Pablo Neira Ayuso
  5 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-04 10:43 UTC (permalink / raw
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet

When dormant flag is toggled, hooks are disabled in the commit phase by
iterating over current chains in table (existing and new).

The following configuration allows for an inconsistent state:

  add table x
  add chain x y { type filter hook input priority 0; }
  add table x { flags dormant; }
  add chain x w { type filter hook input priority 1; }

which triggers the following warning when trying to unregister chain w
which is already unregistered.

[  127.322252] WARNING: CPU: 7 PID: 1211 at net/netfilter/core.c:50                                                                     1 __nf_unregister_net_hook+0x21a/0x260
[...]
[  127.322519] Call Trace:
[  127.322521]  <TASK>
[  127.322524]  ? __warn+0x9f/0x1a0
[  127.322531]  ? __nf_unregister_net_hook+0x21a/0x260
[  127.322537]  ? report_bug+0x1b1/0x1e0
[  127.322545]  ? handle_bug+0x3c/0x70
[  127.322552]  ? exc_invalid_op+0x17/0x40
[  127.322556]  ? asm_exc_invalid_op+0x1a/0x20
[  127.322563]  ? kasan_save_free_info+0x3b/0x60
[  127.322570]  ? __nf_unregister_net_hook+0x6a/0x260
[  127.322577]  ? __nf_unregister_net_hook+0x21a/0x260
[  127.322583]  ? __nf_unregister_net_hook+0x6a/0x260
[  127.322590]  ? __nf_tables_unregister_hook+0x8a/0xe0 [nf_tables]
[  127.322655]  nft_table_disable+0x75/0xf0 [nf_tables]
[  127.322717]  nf_tables_commit+0x2571/0x2620 [nf_tables]

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

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 17bf53cd0e84..1eb51bf24fc2 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2449,6 +2449,9 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
 		struct nft_stats __percpu *stats = NULL;
 		struct nft_chain_hook hook = {};
 
+		if (table->flags & __NFT_TABLE_F_UPDATE)
+			return -EINVAL;
+
 		if (flags & NFT_CHAIN_BINDING)
 			return -EOPNOTSUPP;
 
-- 
2.30.2


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

* [PATCH net 5/6] netfilter: nf_tables: Fix potential data-race in __nft_flowtable_type_get()
  2024-04-04 10:43 [PATCH net 0/6] Netfilter fixes for net Pablo Neira Ayuso
                   ` (3 preceding siblings ...)
  2024-04-04 10:43 ` [PATCH net 4/6] netfilter: nf_tables: reject new basechain after table flag update Pablo Neira Ayuso
@ 2024-04-04 10:43 ` Pablo Neira Ayuso
  2024-04-11 10:03   ` Pablo Neira Ayuso
  2024-04-04 10:43 ` [PATCH net 6/6] netfilter: nf_tables: discard table flag update with pending basechain deletion Pablo Neira Ayuso
  5 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-04 10:43 UTC (permalink / raw
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet

From: Ziyang Xuan <william.xuanziyang@huawei.com>

nft_unregister_flowtable_type() within nf_flow_inet_module_exit() can
concurrent with __nft_flowtable_type_get() within nf_tables_newflowtable().
And thhere is not any protection when iterate over nf_tables_flowtables
list in __nft_flowtable_type_get(). Therefore, there is pertential
data-race of nf_tables_flowtables list entry.

Use list_for_each_entry_rcu() to iterate over nf_tables_flowtables list
in __nft_flowtable_type_get(), and use rcu_read_lock() in the caller
nft_flowtable_type_get() to protect the entire type query process.

Fixes: 3b49e2e94e6e ("netfilter: nf_tables: add flow table netlink frontend")
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 1eb51bf24fc2..e02d0ae4f436 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -8296,11 +8296,12 @@ static int nft_flowtable_parse_hook(const struct nft_ctx *ctx,
 	return err;
 }
 
+/* call under rcu_read_lock */
 static const struct nf_flowtable_type *__nft_flowtable_type_get(u8 family)
 {
 	const struct nf_flowtable_type *type;
 
-	list_for_each_entry(type, &nf_tables_flowtables, list) {
+	list_for_each_entry_rcu(type, &nf_tables_flowtables, list) {
 		if (family == type->family)
 			return type;
 	}
@@ -8312,9 +8313,13 @@ nft_flowtable_type_get(struct net *net, u8 family)
 {
 	const struct nf_flowtable_type *type;
 
+	rcu_read_lock();
 	type = __nft_flowtable_type_get(family);
-	if (type != NULL && try_module_get(type->owner))
+	if (type != NULL && try_module_get(type->owner)) {
+		rcu_read_unlock();
 		return type;
+	}
+	rcu_read_unlock();
 
 	lockdep_nfnl_nft_mutex_not_held();
 #ifdef CONFIG_MODULES
-- 
2.30.2


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

* [PATCH net 6/6] netfilter: nf_tables: discard table flag update with pending basechain deletion
  2024-04-04 10:43 [PATCH net 0/6] Netfilter fixes for net Pablo Neira Ayuso
                   ` (4 preceding siblings ...)
  2024-04-04 10:43 ` [PATCH net 5/6] netfilter: nf_tables: Fix potential data-race in __nft_flowtable_type_get() Pablo Neira Ayuso
@ 2024-04-04 10:43 ` Pablo Neira Ayuso
  5 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-04 10:43 UTC (permalink / raw
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet

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 | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index e02d0ae4f436..d89d77946719 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1209,10 +1209,11 @@ static bool nft_table_pending_update(const struct nft_ctx *ctx)
 		return true;
 
 	list_for_each_entry(trans, &nft_net->commit_list, list) {
-		if ((trans->msg_type == NFT_MSG_NEWCHAIN ||
-		     trans->msg_type == NFT_MSG_DELCHAIN) &&
-		    trans->ctx.table == ctx->table &&
-		    nft_trans_chain_update(trans))
+		if (trans->ctx.table == ctx->table &&
+		    ((trans->msg_type == NFT_MSG_NEWCHAIN &&
+		      nft_trans_chain_update(trans)) ||
+		     (trans->msg_type == NFT_MSG_DELCHAIN &&
+		      nft_is_base_chain(trans->ctx.chain))))
 			return true;
 	}
 
-- 
2.30.2


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

* Re: [PATCH net 1/6] netfilter: nf_tables: release batch on table validation from abort path
  2024-04-04 10:43 ` [PATCH net 1/6] netfilter: nf_tables: release batch on table validation from abort path Pablo Neira Ayuso
@ 2024-04-04 16:50   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-04-04 16:50 UTC (permalink / raw
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, davem, netdev, kuba, pabeni, edumazet

Hello:

This series was applied to netdev/net.git (main)
by Pablo Neira Ayuso <pablo@netfilter.org>:

On Thu,  4 Apr 2024 12:43:29 +0200 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [net,1/6] netfilter: nf_tables: release batch on table validation from abort path
    https://git.kernel.org/netdev/net/c/a45e6889575c
  - [net,2/6] netfilter: nf_tables: release mutex after nft_gc_seq_end from abort path
    https://git.kernel.org/netdev/net/c/0d459e2ffb54
  - [net,3/6] netfilter: nf_tables: flush pending destroy work before exit_net release
    https://git.kernel.org/netdev/net/c/24cea9677025
  - [net,4/6] netfilter: nf_tables: reject new basechain after table flag update
    https://git.kernel.org/netdev/net/c/994209ddf4f4
  - [net,5/6] netfilter: nf_tables: Fix potential data-race in __nft_flowtable_type_get()
    https://git.kernel.org/netdev/net/c/24225011d81b
  - [net,6/6] netfilter: nf_tables: discard table flag update with pending basechain deletion
    https://git.kernel.org/netdev/net/c/1bc83a019bbe

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH net 5/6] netfilter: nf_tables: Fix potential data-race in __nft_flowtable_type_get()
  2024-04-04 10:43 ` [PATCH net 5/6] netfilter: nf_tables: Fix potential data-race in __nft_flowtable_type_get() Pablo Neira Ayuso
@ 2024-04-11 10:03   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 9+ messages in thread
From: Pablo Neira Ayuso @ 2024-04-11 10:03 UTC (permalink / raw
  To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet

On Thu, Apr 04, 2024 at 12:43:33PM +0200, Pablo Neira Ayuso wrote:
> From: Ziyang Xuan <william.xuanziyang@huawei.com>
> 
> nft_unregister_flowtable_type() within nf_flow_inet_module_exit() can
> concurrent with __nft_flowtable_type_get() within nf_tables_newflowtable().
> And thhere is not any protection when iterate over nf_tables_flowtables
> list in __nft_flowtable_type_get(). Therefore, there is pertential
> data-race of nf_tables_flowtables list entry.
> 
> Use list_for_each_entry_rcu() to iterate over nf_tables_flowtables list
> in __nft_flowtable_type_get(), and use rcu_read_lock() in the caller
> nft_flowtable_type_get() to protect the entire type query process.

Applied to nf, thanks

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

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

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-04 10:43 [PATCH net 0/6] Netfilter fixes for net Pablo Neira Ayuso
2024-04-04 10:43 ` [PATCH net 1/6] netfilter: nf_tables: release batch on table validation from abort path Pablo Neira Ayuso
2024-04-04 16:50   ` patchwork-bot+netdevbpf
2024-04-04 10:43 ` [PATCH net 2/6] netfilter: nf_tables: release mutex after nft_gc_seq_end " Pablo Neira Ayuso
2024-04-04 10:43 ` [PATCH net 3/6] netfilter: nf_tables: flush pending destroy work before exit_net release Pablo Neira Ayuso
2024-04-04 10:43 ` [PATCH net 4/6] netfilter: nf_tables: reject new basechain after table flag update Pablo Neira Ayuso
2024-04-04 10:43 ` [PATCH net 5/6] netfilter: nf_tables: Fix potential data-race in __nft_flowtable_type_get() Pablo Neira Ayuso
2024-04-11 10:03   ` Pablo Neira Ayuso
2024-04-04 10:43 ` [PATCH net 6/6] netfilter: nf_tables: discard table flag update with pending basechain deletion Pablo Neira Ayuso

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).