Skip to contentSkip to conversation

5.4.262-rc1 review

PATCH In review Greg Kroah-Hartman 3 participants 166 messages Nov 24, 2023 – Nov 25, 2023 Page 2 of 2

Subsystems (3)
Stable branch netfilter Arm/Amlogic Meson SoC support
lore
Share & tools
lore
author maintainer
#153 lore raw

PATCH 149/159 netfilter: nf_tables: defer gc run if previous batch is still pending

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Florian Westphal <[email protected]>

commit 8e51830e29e12670b4c10df070a4ea4c9593e961 upstream.

Don't queue more gc work, else we may queue the same elements multiple times.

If an element is flagged as dead, this can mean that either the previous gc request was invalidated/discarded by a transaction or that the previous request is still pending in the system work queue.

The latter will happen if the gc interval is set to a very low value, e.g. 1ms, and system work queue is backlogged.

The sets refcount is 1 if no previous gc requeusts are queued, so add a helper for this and skip gc run if old requests are pending.

Add a helper for this and skip the gc run in this case.

Fixes f6c383b8c31a ("netfilter: nf_tables: adapt set backend to use GC transaction API")
Signed-off-by Florian Westphal <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 include/net/netfilter/nf_tables.h |    5 +++++
 net/netfilter/nft_set_hash.c      |    3 +++
 net/netfilter/nft_set_rbtree.c    |    3 +++
 3 files changed, 11 insertions(+)
include/net/netfilter/nf_tables.h +5 -0
@@ -467,6 +467,11 @@ static inline void *nft_set_priv(const s
 	return (void *)set->data;
 }
 
+static inline bool nft_set_gc_is_pending(const struct nft_set *s)
+{
+	return refcount_read(&s->refs) != 1;
+}
+
 static inline struct nft_set *nft_set_container_of(const void *priv)
 {
 	return (void *)priv - offsetof(struct nft_set, data);
net/netfilter/nft_set_hash.c +3 -0
@@ -312,6 +312,9 @@ static void nft_rhash_gc(struct work_str
 	nft_net = net_generic(net, nf_tables_net_id);
 	gc_seq = READ_ONCE(nft_net->gc_seq);
 
+	if (nft_set_gc_is_pending(set))
+		goto done;
+
 	gc = nft_trans_gc_alloc(set, gc_seq, GFP_KERNEL);
 	if (!gc)
 		goto done;
net/netfilter/nft_set_rbtree.c +3 -0
@@ -615,6 +615,9 @@ static void nft_rbtree_gc(struct work_st
 	nft_net = net_generic(net, nf_tables_net_id);
 	gc_seq  = READ_ONCE(nft_net->gc_seq);
 
+	if (nft_set_gc_is_pending(set))
+		goto done;
+
 	gc = nft_trans_gc_alloc(set, gc_seq, GFP_KERNEL);
 	if (!gc)
 		goto done;
author maintainer
#154 lore raw

PATCH 150/159 netfilter: nft_set_rbtree: skip sync GC for new elements in this transaction

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Pablo Neira Ayuso <[email protected]>

commit 2ee52ae94baabf7ee09cf2a8d854b990dac5d0e4 upstream.

New elements in this transaction might expired before such transaction ends. Skip sync GC for such elements otherwise commit path might walk over an already released object. Once transaction is finished, async GC will collect such expired element.

Fixes f6c383b8c31a ("netfilter: nf_tables: adapt set backend to use GC transaction API")
Signed-off-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Florian Westphal <[email protected]>
Signed-off-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 net/netfilter/nft_set_rbtree.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
net/netfilter/nft_set_rbtree.c +6 -2
@@ -314,6 +314,7 @@ static int __nft_rbtree_insert(const str
 	struct nft_rbtree_elem *rbe, *rbe_le = NULL, *rbe_ge = NULL;
 	struct rb_node *node, *next, *parent, **p, *first = NULL;
 	struct nft_rbtree *priv = nft_set_priv(set);
+	u8 cur_genmask = nft_genmask_cur(net);
 	u8 genmask = nft_genmask_next(net);
 	int d, err;
 

@@ -359,8 +360,11 @@ static int __nft_rbtree_insert(const str
 		if (!nft_set_elem_active(&rbe->ext, genmask))
 			continue;
 
-		/* perform garbage collection to avoid bogus overlap reports. */
-		if (nft_set_elem_expired(&rbe->ext)) {
+		/* perform garbage collection to avoid bogus overlap reports
+		 * but skip new elements in this transaction.
+		 */
+		if (nft_set_elem_expired(&rbe->ext) &&
+		    nft_set_elem_active(&rbe->ext, cur_genmask)) {
 			err = nft_rbtree_gc_elem(set, priv, rbe, genmask);
 			if (err < 0)
 				return err;
author maintainer
#155 lore raw

PATCH 151/159 netfilter: nft_set_rbtree: use read spinlock to avoid datapath contention

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Pablo Neira Ayuso <[email protected]>

commit 96b33300fba880ec0eafcf3d82486f3463b4b6da upstream.

rbtree GC does not modify the datastructure, instead it collects expired elements and it enqueues a GC transaction. Use a read spinlock instead to avoid data contention while GC worker is running.

Fixes f6c383b8c31a ("netfilter: nf_tables: adapt set backend to use GC transaction API")
Signed-off-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 net/netfilter/nft_set_rbtree.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
net/netfilter/nft_set_rbtree.c +2 -4
@@ -626,8 +626,7 @@ static void nft_rbtree_gc(struct work_st
 	if (!gc)
 		goto done;
 
-	write_lock_bh(&priv->lock);
-	write_seqcount_begin(&priv->count);
+	read_lock_bh(&priv->lock);
 	for (node = rb_first(&priv->root); node != NULL; node = rb_next(node)) {
 
 		/* Ruleset has been updated, try later. */

@@ -676,8 +675,7 @@ dead_elem:
 	}
 
 try_later:
-	write_seqcount_end(&priv->count);
-	write_unlock_bh(&priv->lock);
+	read_unlock_bh(&priv->lock);
 
 	if (gc)
 		nft_trans_gc_queue_async_done(gc);
author maintainer
#156 lore raw

PATCH 152/159 netfilter: nft_set_hash: try later when GC hits EAGAIN on iteration

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Pablo Neira Ayuso <[email protected]>

commit b079155faae94e9b3ab9337e82100a914ebb4e8d upstream.

Skip GC run if iterator rewinds to the beginning with EAGAIN, otherwise GC might collect the same element more than once.

Fixes f6c383b8c31a ("netfilter: nf_tables: adapt set backend to use GC transaction API")
Signed-off-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 net/netfilter/nft_set_hash.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
net/netfilter/nft_set_hash.c +3 -6
@@ -324,12 +324,9 @@ static void nft_rhash_gc(struct work_str
 
 	while ((he = rhashtable_walk_next(&hti))) {
 		if (IS_ERR(he)) {
-			if (PTR_ERR(he) != -EAGAIN) {
-				nft_trans_gc_destroy(gc);
-				gc = NULL;
-				goto try_later;
-			}
-			continue;
+			nft_trans_gc_destroy(gc);
+			gc = NULL;
+			goto try_later;
 		}
 
 		/* Ruleset has been updated, try later. */
author maintainer
#157 lore raw

PATCH 153/159 netfilter: nf_tables: fix memleak when more than 255 elements expired

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Pablo Neira Ayuso <[email protected]>

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 <[email protected]>
Signed-off-by Florian Westphal <[email protected]>
Signed-off-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 include/net/netfilter/nf_tables.h |    2 +-
 net/netfilter/nf_tables_api.c     |   10 ++++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)
include/net/netfilter/nf_tables.h +1 -1
@@ -1440,7 +1440,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
@@ -7089,12 +7089,15 @@ static int nft_trans_gc_space(struct nft
 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);
 }
 EXPORT_SYMBOL_GPL(nft_trans_gc_queue_async);
 

@@ -7111,15 +7114,18 @@ EXPORT_SYMBOL_GPL(nft_trans_gc_queue_asy
 
 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);
 }
 EXPORT_SYMBOL_GPL(nft_trans_gc_queue_sync);
author maintainer
#158 lore raw

PATCH 154/159 netfilter: nf_tables: unregister flowtable hooks on netns exit

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Pablo Neira Ayuso <[email protected]>

commit 6069da443bf65f513bb507bb21e2f87cfb1ad0b6 upstream.

Unregister flowtable hooks before they are releases via nf_tables_flowtable_destroy() otherwise hook core reports UAF.

BUG: KASAN: use-after-free in nf_hook_entries_grow+0x5a7/0x700 net/netfilter/core.c:142 net/netfilter/core.c:142
Read of size 4 at addr ffff8880736f7438 by task syz-executor579/3666

CPU: 0 PID: 3666 Comm: syz-executor579 Not tainted 5.16.0-rc5-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 __dump_stack lib/dump_stack.c:88 [inline] lib/dump_stack.c:106
 dump_stack_lvl+0x1dc/0x2d8 lib/dump_stack.c:106 lib/dump_stack.c:106
 print_address_description+0x65/0x380 mm/kasan/report.c:247 mm/kasan/report.c:247
 __kasan_report mm/kasan/report.c:433 [inline]
 __kasan_report mm/kasan/report.c:433 [inline] mm/kasan/report.c:450
 kasan_report+0x19a/0x1f0 mm/kasan/report.c:450 mm/kasan/report.c:450
 nf_hook_entries_grow+0x5a7/0x700 net/netfilter/core.c:142 net/netfilter/core.c:142
 __nf_register_net_hook+0x27e/0x8d0 net/netfilter/core.c:429 net/netfilter/core.c:429
 nf_register_net_hook+0xaa/0x180 net/netfilter/core.c:571 net/netfilter/core.c:571
 nft_register_flowtable_net_hooks+0x3c5/0x730 net/netfilter/nf_tables_api.c:7232 net/netfilter/nf_tables_api.c:7232
 nf_tables_newflowtable+0x2022/0x2cf0 net/netfilter/nf_tables_api.c:7430 net/netfilter/nf_tables_api.c:7430
 nfnetlink_rcv_batch net/netfilter/nfnetlink.c:513 [inline]
 nfnetlink_rcv_skb_batch net/netfilter/nfnetlink.c:634 [inline]
 nfnetlink_rcv_batch net/netfilter/nfnetlink.c:513 [inline] net/netfilter/nfnetlink.c:652
 nfnetlink_rcv_skb_batch net/netfilter/nfnetlink.c:634 [inline] net/netfilter/nfnetlink.c:652
 nfnetlink_rcv+0x10e6/0x2550 net/netfilter/nfnetlink.c:652 net/netfilter/nfnetlink.c:652

__nft_release_hook() calls nft_unregister_flowtable_net_hooks() which only unregisters the hooks, then after RCU grace period, it is guaranteed that no packets add new entries to the flowtable (no flow offload rules and flowtable hooks are reachable from packet path), so it is safe to call nf_flow_table_free() which cleans up the remaining entries from the flowtable (both software and hardware) and it unbinds the flow_block.

Fixes ff4bf2f42a40 ("netfilter: nf_tables: add nft_unregister_flowtable_hook()")
Reported-by [email protected]
Signed-off-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 net/netfilter/nf_tables_api.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
net/netfilter/nf_tables_api.c +13 -5
@@ -8186,16 +8186,24 @@ int __nft_release_basechain(struct nft_c
 }
 EXPORT_SYMBOL_GPL(__nft_release_basechain);
 
+static void __nft_release_hook(struct net *net, struct nft_table *table)
+{
+	struct nft_flowtable *flowtable;
+	struct nft_chain *chain;
+
+	list_for_each_entry(chain, &table->chains, list)
+		nf_tables_unregister_hook(net, table, chain);
+	list_for_each_entry(flowtable, &table->flowtables, list)
+		nft_unregister_flowtable_net_hooks(net, flowtable);
+}
+
 static void __nft_release_hooks(struct net *net)
 {
 	struct nftables_pernet *nft_net = net_generic(net, nf_tables_net_id);
 	struct nft_table *table;
-	struct nft_chain *chain;
 
-	list_for_each_entry(table, &nft_net->tables, list) {
-		list_for_each_entry(chain, &table->chains, list)
-			nf_tables_unregister_hook(net, table, chain);
-	}
+	list_for_each_entry(table, &nft_net->tables, list)
+		__nft_release_hook(net, table);
 }
 
 static void __nft_release_table(struct net *net, struct nft_table *table)
author maintainer
#159 lore raw

PATCH 155/159 netfilter: nf_tables: double hook unregistration in netns path

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Pablo Neira Ayuso <[email protected]>

commit f9a43007d3f7ba76d5e7f9421094f00f2ef202f8 upstream.

__nft_release_hooks() is called from pre_netns exit path which unregisters the hooks, then the NETDEV_UNREGISTER event is triggered which unregisters the hooks again.

[  565.221461] WARNING: CPU: 18 PID: 193 at net/netfilter/core.c:495 __nf_unregister_net_hook+0x247/0x270
[...]
[  565.246890] CPU: 18 PID: 193 Comm: kworker/u64:1 Tainted: G            E     5.18.0-rc7+ #27
[  565.253682] Workqueue: netns cleanup_net
[  565.257059] RIP: 0010:__nf_unregister_net_hook+0x247/0x270
[...]
[  565.297120] Call Trace:
[  565.300900]  <TASK>
[  565.304683]  nf_tables_flowtable_event+0x16a/0x220 [nf_tables]
[  565.308518]  raw_notifier_call_chain+0x63/0x80
[  565.312386]  unregister_netdevice_many+0x54f/0xb50

Unregister and destroy netdev hook from netns pre_exit via kfree_rcu so the NETDEV_UNREGISTER path see unregistered hooks.

Fixes 767d1216bff8 ("netfilter: nftables: fix possible UAF over chains from packet path in netns")
Signed-off-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 net/netfilter/nf_tables_api.c    |   34 +++++++++++++++++++++++++++-------
 net/netfilter/nft_chain_filter.c |    3 +++
 2 files changed, 30 insertions(+), 7 deletions(-)
net/netfilter/nf_tables_api.c +27 -7
@@ -219,9 +219,10 @@ static int nf_tables_register_hook(struc
 	return nf_register_net_hook(net, ops);
 }
 
-static void nf_tables_unregister_hook(struct net *net,
-				      const struct nft_table *table,
-				      struct nft_chain *chain)
+static void __nf_tables_unregister_hook(struct net *net,
+					const struct nft_table *table,
+					struct nft_chain *chain,
+					bool release_netdev)
 {
 	const struct nft_base_chain *basechain;
 	const struct nf_hook_ops *ops;

@@ -236,6 +237,16 @@ static void nf_tables_unregister_hook(st
 		return basechain->type->ops_unregister(net, ops);
 
 	nf_unregister_net_hook(net, ops);
+	if (release_netdev &&
+	    table->family == NFPROTO_NETDEV)
+		nft_base_chain(chain)->ops.dev = NULL;
+}
+
+static void nf_tables_unregister_hook(struct net *net,
+				      const struct nft_table *table,
+				      struct nft_chain *chain)
+{
+	__nf_tables_unregister_hook(net, table, chain, false);
 }
 
 static int nft_trans_table_add(struct nft_ctx *ctx, int msg_type)

@@ -5997,8 +6008,9 @@ nft_flowtable_type_get(struct net *net,
 	return ERR_PTR(-ENOENT);
 }
 
-static void nft_unregister_flowtable_net_hooks(struct net *net,
-					       struct nft_flowtable *flowtable)
+static void __nft_unregister_flowtable_net_hooks(struct net *net,
+						 struct nft_flowtable *flowtable,
+						 bool release_netdev)
 {
 	int i;
 

@@ -6007,9 +6019,17 @@ static void nft_unregister_flowtable_net
 			continue;
 
 		nf_unregister_net_hook(net, &flowtable->ops[i]);
+		if (release_netdev)
+			flowtable->ops[i].dev = NULL;
 	}
 }
 
+static void nft_unregister_flowtable_net_hooks(struct net *net,
+					       struct nft_flowtable *flowtable)
+{
+	__nft_unregister_flowtable_net_hooks(net, flowtable, false);
+}
+
 static int nf_tables_newflowtable(struct net *net, struct sock *nlsk,
 				  struct sk_buff *skb,
 				  const struct nlmsghdr *nlh,

@@ -8192,9 +8212,9 @@ static void __nft_release_hook(struct ne
 	struct nft_chain *chain;
 
 	list_for_each_entry(chain, &table->chains, list)
-		nf_tables_unregister_hook(net, table, chain);
+		__nf_tables_unregister_hook(net, table, chain, true);
 	list_for_each_entry(flowtable, &table->flowtables, list)
-		nft_unregister_flowtable_net_hooks(net, flowtable);
+		__nft_unregister_flowtable_net_hooks(net, flowtable, true);
 }
 
 static void __nft_release_hooks(struct net *net)
net/netfilter/nft_chain_filter.c +3 -0
@@ -296,6 +296,9 @@ static void nft_netdev_event(unsigned lo
 		if (strcmp(basechain->dev_name, dev->name) != 0)
 			return;
 
+		if (!basechain->ops.dev)
+			return;
+
 		/* UNREGISTER events are also happpening on netns exit.
 		 *
 		 * Altough nf_tables core releases all tables/chains, only
author maintainer
#160 lore raw

PATCH 156/159 netfilter: nftables: update table flags from the commit phase

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Pablo Neira Ayuso <[email protected]>

commit 0ce7cf4127f14078ca598ba9700d813178a59409 upstream.

Do not update table flags from the preparation phase. Store the flags update into the transaction, then update the flags from the commit phase.

Signed-off-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 include/net/netfilter/nf_tables.h |    9 ++++++---
 net/netfilter/nf_tables_api.c     |   31 ++++++++++++++++---------------
 2 files changed, 22 insertions(+), 18 deletions(-)
include/net/netfilter/nf_tables.h +6 -3
@@ -1392,13 +1392,16 @@ struct nft_trans_chain {
 
 struct nft_trans_table {
 	bool				update;
-	bool				enable;
+	u8				state;
+	u32				flags;
 };
 
 #define nft_trans_table_update(trans)	\
 	(((struct nft_trans_table *)trans->data)->update)
-#define nft_trans_table_enable(trans)	\
-	(((struct nft_trans_table *)trans->data)->enable)
+#define nft_trans_table_state(trans)	\
+	(((struct nft_trans_table *)trans->data)->state)
+#define nft_trans_table_flags(trans)	\
+	(((struct nft_trans_table *)trans->data)->flags)
 
 struct nft_trans_elem {
 	struct nft_set			*set;
net/netfilter/nf_tables_api.c +16 -15
@@ -893,6 +893,12 @@ static void nf_tables_table_disable(stru
 	nft_table_disable(net, table, 0);
 }
 
+enum {
+	NFT_TABLE_STATE_UNCHANGED	= 0,
+	NFT_TABLE_STATE_DORMANT,
+	NFT_TABLE_STATE_WAKEUP
+};
+
 static int nf_tables_updtable(struct nft_ctx *ctx)
 {
 	struct nft_trans *trans;

@@ -916,19 +922,17 @@ static int nf_tables_updtable(struct nft
 
 	if ((flags & NFT_TABLE_F_DORMANT) &&
 	    !(ctx->table->flags & NFT_TABLE_F_DORMANT)) {
-		nft_trans_table_enable(trans) = false;
+		nft_trans_table_state(trans) = NFT_TABLE_STATE_DORMANT;
 	} else if (!(flags & NFT_TABLE_F_DORMANT) &&
 		   ctx->table->flags & NFT_TABLE_F_DORMANT) {
-		ctx->table->flags &= ~NFT_TABLE_F_DORMANT;
 		ret = nf_tables_table_enable(ctx->net, ctx->table);
 		if (ret >= 0)
-			nft_trans_table_enable(trans) = true;
-		else
-			ctx->table->flags |= NFT_TABLE_F_DORMANT;
+			nft_trans_table_state(trans) = NFT_TABLE_STATE_WAKEUP;
 	}
 	if (ret < 0)
 		goto err;
 
+	nft_trans_table_flags(trans) = flags;
 	nft_trans_table_update(trans) = true;
 	nft_trans_commit_list_add_tail(ctx->net, trans);
 	return 0;

@@ -7298,11 +7302,10 @@ static int nf_tables_commit(struct net *
 		switch (trans->msg_type) {
 		case NFT_MSG_NEWTABLE:
 			if (nft_trans_table_update(trans)) {
-				if (!nft_trans_table_enable(trans)) {
-					nf_tables_table_disable(net,
-								trans->ctx.table);
-					trans->ctx.table->flags |= NFT_TABLE_F_DORMANT;
-				}
+				if (nft_trans_table_state(trans) == NFT_TABLE_STATE_DORMANT)
+					nf_tables_table_disable(net, trans->ctx.table);
+
+				trans->ctx.table->flags = nft_trans_table_flags(trans);
 			} else {
 				nft_clear(net, trans->ctx.table);
 			}

@@ -7497,11 +7500,9 @@ static int __nf_tables_abort(struct net
 		switch (trans->msg_type) {
 		case NFT_MSG_NEWTABLE:
 			if (nft_trans_table_update(trans)) {
-				if (nft_trans_table_enable(trans)) {
-					nf_tables_table_disable(net,
-								trans->ctx.table);
-					trans->ctx.table->flags |= NFT_TABLE_F_DORMANT;
-				}
+				if (nft_trans_table_state(trans) == NFT_TABLE_STATE_WAKEUP)
+					nf_tables_table_disable(net, trans->ctx.table);
+
 				nft_trans_destroy(trans);
 			} else {
 				list_del_rcu(&trans->ctx.table->list);
author maintainer
#161 lore raw

PATCH 157/159 netfilter: nf_tables: fix table flag updates

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Pablo Neira Ayuso <[email protected]>

commit 179d9ba5559a756f4322583388b3213fe4e391b0 upstream.

The dormant flag need to be updated from the preparation phase, otherwise, two consecutive requests to dorm a table in the same batch might try to remove the same hooks twice, resulting in the following warning:

hook not found, pf 3 num 0

 WARNING: CPU: 0 PID: 334 at net/netfilter/core.c:480 __nf_unregister_net_hook+0x1eb/0x610 net/netfilter/core.c:480
 Modules linked in:
 CPU: 0 PID: 334 Comm: kworker/u4:5 Not tainted 5.12.0-syzkaller #0
 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
 Workqueue: netns cleanup_net
 RIP: 0010:__nf_unregister_net_hook+0x1eb/0x610 net/netfilter/core.c:480

This patch is a partial revert of 0ce7cf4127f1 ("netfilter: nftables:
update table flags from the commit phase") to restore the previous behaviour.

However, there is still another problem: A batch containing a series of dorm-wakeup-dorm table and vice-versa also trigger the warning above since hook unregistration happens from the preparation phase, while hook registration occurs from the commit phase.

To fix this problem, this patch adds two internal flags to annotate the original dormant flag status which are __NFT_TABLE_F_WAS_DORMANT and __NFT_TABLE_F_WAS_AWAKEN, to restore it from the abort path.

The __NFT_TABLE_F_UPDATE bitmask allows to handle the dormant flag update with one single transaction.

Reported-by [email protected]
Fixes 0ce7cf4127f1 ("netfilter: nftables: update table flags from the commit phase")
Signed-off-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 include/net/netfilter/nf_tables.h        |    6 ---
 include/uapi/linux/netfilter/nf_tables.h |    1 
 net/netfilter/nf_tables_api.c            |   59 +++++++++++++++++++++----------
 3 files changed, 41 insertions(+), 25 deletions(-)
include/net/netfilter/nf_tables.h +0 -6
@@ -1392,16 +1392,10 @@ struct nft_trans_chain {
 
 struct nft_trans_table {
 	bool				update;
-	u8				state;
-	u32				flags;
 };
 
 #define nft_trans_table_update(trans)	\
 	(((struct nft_trans_table *)trans->data)->update)
-#define nft_trans_table_state(trans)	\
-	(((struct nft_trans_table *)trans->data)->state)
-#define nft_trans_table_flags(trans)	\
-	(((struct nft_trans_table *)trans->data)->flags)
 
 struct nft_trans_elem {
 	struct nft_set			*set;
include/uapi/linux/netfilter/nf_tables.h +1 -0
@@ -162,6 +162,7 @@ enum nft_hook_attributes {
 enum nft_table_flags {
 	NFT_TABLE_F_DORMANT	= 0x1,
 };
+#define NFT_TABLE_F_MASK       (NFT_TABLE_F_DORMANT)
 
 /**
  * enum nft_table_attributes - nf_tables table netlink attributes
net/netfilter/nf_tables_api.c +40 -19

107 lines not on this page · raw message

author maintainer
#162 lore raw

PATCH 158/159 netfilter: nf_tables: disable toggling dormant table state more than once

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Pablo Neira Ayuso <[email protected]>

commit c9bd26513b3a11b3adb3c2ed8a31a01a87173ff1 upstream.

nft -f -<<EOF
add table ip t
add table ip t { flags dormant; }
add chain ip t c { type filter hook input priority 0; } add table ip t
EOF

Triggers a splat from nf core on next table delete because we lose track of right hook register state:

WARNING: CPU: 2 PID: 1597 at net/netfilter/core.c:501 __nf_unregister_net_hook
RIP: 0010:__nf_unregister_net_hook+0x41b/0x570
 nf_unregister_net_hook+0xb4/0xf0
 __nf_tables_unregister_hook+0x160/0x1d0

[..]

The above should have table in *active* state, but in fact no hooks were registered.

Reject on/off/on games rather than attempting to fix this.

Fixes 179d9ba5559a ("netfilter: nf_tables: fix table flag updates")
Reported-by "Lee, Cherie-Anne" <[email protected]>
Cc Bing-Jhong Billy Jheng <[email protected]>
Cc [email protected]
Signed-off-by Florian Westphal <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 net/netfilter/nf_tables_api.c |    4 ++++
 1 file changed, 4 insertions(+)
net/netfilter/nf_tables_api.c +4 -0
@@ -918,6 +918,10 @@ static int nf_tables_updtable(struct nft
 	if (flags == ctx->table->flags)
 		return 0;
 
+	/* No dormant off/on/off/on games in single transaction */
+	if (ctx->table->flags & __NFT_TABLE_F_UPDATE)
+		return -EINVAL;
+
 	trans = nft_trans_alloc(ctx, NFT_MSG_NEWTABLE,
 				sizeof(struct nft_trans_table));
 	if (trans == NULL)
author maintainer
#163 lore raw

PATCH 159/159 netfilter: nf_tables: bogus EBUSY when deleting flowtable after flush (for 5.4)

In reply to Greg Kroah-Hartman

5.4-stable review patch. If anyone has any objections, please let me know.

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

From: Pablo Neira Ayuso <[email protected]>

3f0465a9ef02 ("netfilter: nf_tables: dynamically allocate hooks per net_device in flowtables") reworks flowtable support to allow for dynamic allocation of hooks, which implicitly fixes the following bogus EBUSY in transaction:

delete flowtable
add flowtable # same flowtable with same devices, it hits EBUSY

This patch does not exist in any tree, but it fixes this issue for -stable Linux kernel 5.4

Signed-off-by Pablo Neira Ayuso <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 net/netfilter/nf_tables_api.c |    3 +++
 1 file changed, 3 insertions(+)
net/netfilter/nf_tables_api.c +3 -0
@@ -6132,6 +6132,9 @@ static int nf_tables_newflowtable(struct
 			continue;
 
 		list_for_each_entry(ft, &table->flowtables, list) {
+			if (!nft_is_active_next(net, ft))
+				continue;
+
 			for (k = 0; k < ft->ops_len; k++) {
 				if (!ft->ops[k].dev)
 					continue;
#164 lore raw
In reply to Greg Kroah-Hartman
Quoted 10 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 5.4.262 release. There are 159 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Sun, 26 Nov 2023 17:19:17 +0000. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

Quoted 8 lines
        https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.4.262-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.4.y
and the diffstat can be found below.

thanks,

greg k-h

As Daniel replied on 4.19 build failures,
Following build warning / errors occurred on arm and arm64 on the stable-rc linux.5.4.y and linux-4.19.y.

tty/serial: Migrate meson_uart to use has_sysrq
[ Upstream commit dca3ac8d3bc9436eb5fd35b80cdcad762fbfa518 ]

drivers/tty/serial/meson_uart.c: In function 'meson_uart_probe':
drivers/tty/serial/meson_uart.c:728:13: error: 'struct uart_port' has no member named 'has_sysrq'
728 | port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_MESON_CONSOLE);
| ^~

Reported-by Linux Kernel Functional Testing <[email protected]>

--
Linaro LKFT
https://lkft.linaro.org

author maintainer
#165 lore raw
Quoted 29 lines · Naresh Kamboju
Quoted 10 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 5.4.262 release. There are 159 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Sun, 26 Nov 2023 17:19:17 +0000. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

Quoted 8 lines
        https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.4.262-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.4.y
and the diffstat can be found below.

thanks,

greg k-h

As Daniel replied on 4.19 build failures,
Following build warning / errors occurred on arm and arm64 on the stable-rc linux.5.4.y and linux-4.19.y.

tty/serial: Migrate meson_uart to use has_sysrq
[ Upstream commit dca3ac8d3bc9436eb5fd35b80cdcad762fbfa518 ]

Quoted 7 lines

drivers/tty/serial/meson_uart.c: In function 'meson_uart_probe':
drivers/tty/serial/meson_uart.c:728:13: error: 'struct uart_port' has no member named 'has_sysrq'
728 | port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_MESON_CONSOLE);
| ^~

Reported-by Linux Kernel Functional Testing <[email protected]>

Now fixed, thanks.

greg k-h