[PATCH net 00/10] Netfilter fixes for net

[PATCH net 00/10] Netfilter fixes for net

[Pablo Neira Ayuso]

Hi,
 
The following patchset contains Netfilter fixes for net:
 
1) Use DEBUG_NET_WARN_ON_ONCE() instead of WARN_ON() from the tproxy
   datapath, a recent bug found a way to reach WARN_ON from datapath
   due to insufficient validation of xt_TPROTO checkentry.
   From Fernando F. Mancera.
 
2) Similar to previous patch to replace WARN_ON_ONCE by
   DEBUG_NET_WARN_ON_ONCE() for connlimit. Not known issue, but
   since this patch has been around for a while, let's merge it.
   Also from Fernando.

3) Move nf_tables harware offload commit path after chain blob
   and audit to reduce chances of leaving the hardware in
   inconsistent state.

4) Add missing vzeroupper to nf_tables pipapo AVX2 to address
   performace degradation to later user of SSE code,
   from Eric Biggers.

5) Remove pr_debug() in x_tables extensions, a recent bogus found a
   way to print a unsanitized string in xt_IDLETIMER, many of these
   pr_debug() calls are there for historical reasons.

6) Use pr_info_ratelimited() in x_tables .checkentry.

7) Fix an imbalance in module refcount due to incorrect override
   expression logic with sets. Remove unnecessary clone in control
   plane, use the existing expressions provided by set or dynset
   expression. Release override expressions only.

9) Tigthen nf_tables device name removal, it is possible to remove
   prefix strings with exact device name. From Fernando F. Mancera.

9) Set on the set dead bit earlier, otherwise it is possible to
   call .commit on deleted sets. This also addresses the
   re-introduction of a bug.

10) Remove leftover definition of the local set_update_list.
    A recent patch made it per-netns but did not remove this which
    is now unused.

Please, pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-26-08-21

Thanks.

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

The following changes since commit 746fc0787f616da418ffc04a110296fe95d53491:

  net: usb: cdc_ncm: add Apple MacBook Pro USB product ID 0x1902 (2026-08-20 14:47:29 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git tags/nf-26-08-21

for you to fetch changes up to 878ca6f9739ab460a185fd1f0e64c8ef026057e4:

  netfilter: nf_tables: remove leftover set_update_list (2026-08-21 17:12:59 +0200)

----------------------------------------------------------------
netfilter pull request 26-08-21

----------------------------------------------------------------
Eric Biggers (1):
      netfilter: nft_set_pipapo_avx2: add missing vzeroupper

Fernando Fernandez Mancera (3):
      netfilter: tproxy: use DEBUG_NET_WARN_ON_ONCE for protocol fallbacks
      netfilter: conncount: use DEBUG_NET_WARN_ON_ONCE on reaching count limit
      netfilter: nf_tables: fix device name and prefix match in hook lookup

Pablo Neira Ayuso (6):
      netfilter: nf_tables: move hardware offload step after building the chain blob
      netfilter: x_tables: remove pr_debug
      netfilter: xt_cgroup: use pr_info_ratelimited()
      netfilter: nf_tables: skip double clone set expressions on element insert
      netfilter: nf_tables: set on dead bit when performing early element removal
      netfilter: nf_tables: remove leftover set_update_list

 include/net/netfilter/nf_tables.h   |  2 -
 net/ipv4/netfilter/ipt_ah.c         | 10 +----
 net/ipv4/netfilter/nf_tproxy_ipv4.c |  2 +-
 net/ipv6/netfilter/ip6t_ah.c        | 27 +-----------
 net/ipv6/netfilter/ip6t_frag.c      | 41 +-----------------
 net/ipv6/netfilter/ip6t_hbh.c       | 40 ++++--------------
 net/ipv6/netfilter/ip6t_mh.c        |  3 --
 net/ipv6/netfilter/ip6t_rt.c        |  6 +--
 net/ipv6/netfilter/nf_tproxy_ipv6.c |  2 +-
 net/netfilter/nf_conncount.c        |  3 +-
 net/netfilter/nf_tables_api.c       | 84 +++++++++++++++++--------------------
 net/netfilter/nft_dynset.c          | 25 ++++++-----
 net/netfilter/nft_set_pipapo_avx2.c | 17 ++++----
 net/netfilter/xt_IDLETIMER.c        | 68 ++++++------------------------
 net/netfilter/xt_LOG.c              |  4 +-
 net/netfilter/xt_MASQUERADE.c       |  4 +-
 net/netfilter/xt_NETMAP.c           |  4 +-
 net/netfilter/xt_REDIRECT.c         |  4 +-
 net/netfilter/xt_cgroup.c           | 10 ++---
 net/netfilter/xt_esp.c              | 10 +----
 net/netfilter/xt_ipcomp.c           |  8 +---
 net/netfilter/xt_iprange.c          | 32 ++------------
 net/netfilter/xt_ipvs.c             |  1 -
 net/netfilter/xt_multiport.c        |  4 --
 net/netfilter/xt_sctp.c             | 19 +--------
 net/netfilter/xt_tcpudp.c           |  8 +---
 26 files changed, 117 insertions(+), 321 deletions(-)

[PATCH net 01/10] netfilter: tproxy: use DEBUG_NET_WARN_ON_ONCE for protocol fallbacks

[Pablo Neira Ayuso]

From: Fernando Fernandez Mancera <fmancera@suse.de>

Replace WARN_ON calls with DEBUG_NET_WARN_ON_ONCE in the default switch
blocks of nf_tproxy_get_sock_v4 and v6. Unsupported transport protocols
are already safely handled by returning a NULL socket pointer. This
prevents unnecessary system panics when panic_on_warn=1 is enabled in
production systems.

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/nf_tproxy_ipv4.c | 2 +-
 net/ipv6/netfilter/nf_tproxy_ipv6.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/netfilter/nf_tproxy_ipv4.c b/net/ipv4/netfilter/nf_tproxy_ipv4.c
index 041c3f37f237..5eab7a2dc8ef 100644
--- a/net/ipv4/netfilter/nf_tproxy_ipv4.c
+++ b/net/ipv4/netfilter/nf_tproxy_ipv4.c
@@ -137,7 +137,7 @@ nf_tproxy_get_sock_v4(struct net *net, struct sk_buff *skb,
 		}
 		break;
 	default:
-		WARN_ON(1);
+		DEBUG_NET_WARN_ON_ONCE(1);
 		sk = NULL;
 	}
 
diff --git a/net/ipv6/netfilter/nf_tproxy_ipv6.c b/net/ipv6/netfilter/nf_tproxy_ipv6.c
index b2f59ed9d7cc..12ec36a6be2e 100644
--- a/net/ipv6/netfilter/nf_tproxy_ipv6.c
+++ b/net/ipv6/netfilter/nf_tproxy_ipv6.c
@@ -136,7 +136,7 @@ nf_tproxy_get_sock_v6(struct net *net, struct sk_buff *skb, int thoff,
 		}
 		break;
 	default:
-		WARN_ON(1);
+		DEBUG_NET_WARN_ON_ONCE(1);
 		sk = NULL;
 	}
 
-- 
2.47.3

[PATCH net 02/10] netfilter: conncount: use DEBUG_NET_WARN_ON_ONCE on reaching count limit

[Pablo Neira Ayuso]

From: Fernando Fernandez Mancera <fmancera@suse.de>

Replace WARN_ON_ONCE with DEBUG_NET_WARN_ON_ONCE in __nf_conncount_add.
The function handles count limit breaches safely by returning
-EOVERFLOW, so a production backtrace is not needed. This prevents
unnecessary system panics when panic_on_warn=1 is enabled in production
systems.

Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conncount.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index 85487f92af50..4292c3d9addd 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -251,7 +251,8 @@ static int __nf_conncount_add(struct net *net,
 	list->last_gc_count = list->count;
 
 add_new_node:
-	if (WARN_ON_ONCE(list->count > INT_MAX)) {
+	if (unlikely(list->count > INT_MAX)) {
+		DEBUG_NET_WARN_ON_ONCE(1);
 		err = -EOVERFLOW;
 		goto out_put;
 	}
-- 
2.47.3

[PATCH net 03/10] netfilter: nf_tables: move hardware offload step after building the chain blob

[Pablo Neira Ayuso]

Allocate the chain blob before the ruleset offload to reduce chances of
entering an inconsistent state where the offloaded ruleset in the nic
and the software ruleset differ.

Fixes: c9626a2cbdb2 ("netfilter: nf_tables: add hardware offload support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c112ecc4fca3..71f4227d7ac7 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -10982,10 +10982,6 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		return -EAGAIN;
 	}
 
-	err = nft_flow_rule_offload_commit(net);
-	if (err < 0)
-		return err;
-
 	/* 1.  Allocate space for next generation rules_gen_X[] */
 	list_for_each_entry_safe(trans, next, &nft_net->commit_list, list) {
 		struct nft_table *table = trans->table;
@@ -11010,6 +11006,16 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 		}
 	}
 
+	/* must be last, so audit and chain blob set up does not leave hardware
+	 * in consistent state.
+	 */
+	err = nft_flow_rule_offload_commit(net);
+	if (err < 0) {
+		nf_tables_commit_chain_prepare_cancel(net);
+		nf_tables_commit_audit_free(&adl);
+		return err;
+	}
+
 	/* step 2.  Make rules_gen_X visible to packet path */
 	nft_set_commit_update(&ctx, nft_net);
 
-- 
2.47.3

[PATCH net 04/10] netfilter: nft_set_pipapo_avx2: add missing vzeroupper

[Pablo Neira Ayuso]

From: Eric Biggers <ebiggers@kernel.org>

Since pipapo_get_avx2() uses YMM registers, execute vzeroupper before
returning from it.  This is needed to avoid degrading the performance of
any later SSE code that may happen to be executed.

Fixes: 7400b063969b ("nft_set_pipapo: Introduce AVX2-based lookup implementation")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_set_pipapo_avx2.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/nft_set_pipapo_avx2.c b/net/netfilter/nft_set_pipapo_avx2.c
index b3f105520a85..21f5be68c703 100644
--- a/net/netfilter/nft_set_pipapo_avx2.c
+++ b/net/netfilter/nft_set_pipapo_avx2.c
@@ -1134,6 +1134,7 @@ struct nft_pipapo_elem *pipapo_get_avx2(const struct nft_pipapo_match *m,
 	struct nft_pipapo_scratch *scratch;
 	const struct nft_pipapo_field *f;
 	unsigned long *res, *fill, *map;
+	struct nft_pipapo_elem *e;
 	bool map_index;
 	int ret = 0;
 	int i;
@@ -1207,14 +1208,11 @@ struct nft_pipapo_elem *pipapo_get_avx2(const struct nft_pipapo_match *m,
 next_match:
 		if (ret < 0) {
 			scratch->map_index = map_index;
-			kernel_fpu_end();
-			__local_unlock_nested_bh(&scratch->bh_lock);
-			return NULL;
+			e = NULL;
+			goto out;
 		}
 
 		if (last) {
-			struct nft_pipapo_elem *e;
-
 			e = f->mt[ret].e;
 			if (unlikely(__nft_set_elem_expired(&e->ext, tstamp) ||
 				     !nft_set_elem_active(&e->ext, genmask))) {
@@ -1224,9 +1222,7 @@ struct nft_pipapo_elem *pipapo_get_avx2(const struct nft_pipapo_match *m,
 			}
 
 			scratch->map_index = map_index;
-			kernel_fpu_end();
-			__local_unlock_nested_bh(&scratch->bh_lock);
-			return e;
+			goto out;
 		}
 
 		map_index = !map_index;
@@ -1234,9 +1230,12 @@ struct nft_pipapo_elem *pipapo_get_avx2(const struct nft_pipapo_match *m,
 		data += NFT_PIPAPO_GROUPS_PADDED_SIZE(f);
 	}
 
+	e = NULL;
+out:
+	asm volatile("vzeroupper");
 	kernel_fpu_end();
 	__local_unlock_nested_bh(&scratch->bh_lock);
-	return NULL;
+	return e;
 }
 
 /**
-- 
2.47.3

[PATCH net 05/10] netfilter: x_tables: remove pr_debug

[Pablo Neira Ayuso]

Remove pr_debug() for these xtables extensions, these have no use these
days. Still, turn pr_debug() into pr_info_ratelimited() in the .check
path since this helps provide a hint via dmesg in legacy iptables.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/ipt_ah.c    | 10 +----
 net/ipv6/netfilter/ip6t_ah.c   | 27 +-------------
 net/ipv6/netfilter/ip6t_frag.c | 41 +-------------------
 net/ipv6/netfilter/ip6t_hbh.c  | 40 +++++---------------
 net/ipv6/netfilter/ip6t_mh.c   |  3 --
 net/ipv6/netfilter/ip6t_rt.c   |  6 +--
 net/netfilter/xt_IDLETIMER.c   | 68 ++++++----------------------------
 net/netfilter/xt_LOG.c         |  4 +-
 net/netfilter/xt_MASQUERADE.c  |  4 +-
 net/netfilter/xt_NETMAP.c      |  4 +-
 net/netfilter/xt_REDIRECT.c    |  4 +-
 net/netfilter/xt_esp.c         | 10 +----
 net/netfilter/xt_ipcomp.c      |  8 +---
 net/netfilter/xt_iprange.c     | 32 ++--------------
 net/netfilter/xt_ipvs.c        |  1 -
 net/netfilter/xt_multiport.c   |  4 --
 net/netfilter/xt_sctp.c        | 19 +---------
 net/netfilter/xt_tcpudp.c      |  8 +---
 18 files changed, 47 insertions(+), 246 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_ah.c b/net/ipv4/netfilter/ipt_ah.c
index 161ba412cb08..7131f297ada2 100644
--- a/net/ipv4/netfilter/ipt_ah.c
+++ b/net/ipv4/netfilter/ipt_ah.c
@@ -19,12 +19,7 @@ MODULE_DESCRIPTION("Xtables: IPv4 IPsec-AH SPI match");
 static inline bool
 spi_match(u_int32_t min, u_int32_t max, u_int32_t spi, bool invert)
 {
-	bool r;
-	pr_debug("spi_match:%c 0x%x <= 0x%x <= 0x%x\n",
-		 invert ? '!' : ' ', min, spi, max);
-	r = (spi >= min && spi <= max) ^ invert;
-	pr_debug(" result %s\n", r ? "PASS" : "FAILED");
-	return r;
+	return (spi >= min && spi <= max) ^ invert;
 }
 
 static bool ah_mt(const struct sk_buff *skb, struct xt_action_param *par)
@@ -42,7 +37,6 @@ static bool ah_mt(const struct sk_buff *skb, struct xt_action_param *par)
 		/* We've been asked to examine this packet, and we
 		 * can't.  Hence, no choice but to drop.
 		 */
-		pr_debug("Dropping evil AH tinygram.\n");
 		par->hotdrop = true;
 		return false;
 	}
@@ -58,7 +52,7 @@ static int ah_mt_check(const struct xt_mtchk_param *par)
 
 	/* Must specify no unknown invflags */
 	if (ahinfo->invflags & ~IPT_AH_INV_MASK) {
-		pr_debug("unknown flags %X\n", ahinfo->invflags);
+		pr_info_ratelimited("unknown flags %X\n", ahinfo->invflags);
 		return -EINVAL;
 	}
 	return 0;
diff --git a/net/ipv6/netfilter/ip6t_ah.c b/net/ipv6/netfilter/ip6t_ah.c
index 1258783ed876..dab7dbc6a675 100644
--- a/net/ipv6/netfilter/ip6t_ah.c
+++ b/net/ipv6/netfilter/ip6t_ah.c
@@ -24,13 +24,7 @@ MODULE_AUTHOR("Andras Kis-Szabo <kisza@sch.bme.hu>");
 static inline bool
 spi_match(u_int32_t min, u_int32_t max, u_int32_t spi, bool invert)
 {
-	bool r;
-
-	pr_debug("spi_match:%c 0x%x <= 0x%x <= 0x%x\n",
-		 invert ? '!' : ' ', min, spi, max);
-	r = (spi >= min && spi <= max) ^ invert;
-	pr_debug(" result %s\n", r ? "PASS" : "FAILED");
-	return r;
+	return (spi >= min && spi <= max) ^ invert;
 }
 
 static bool ah_mt6(const struct sk_buff *skb, struct xt_action_param *par)
@@ -62,23 +56,6 @@ static bool ah_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 		return false;
 	}
 
-	pr_debug("IPv6 AH LEN %u %u ", hdrlen, ah->hdrlen);
-	pr_debug("RES %04X ", ah->reserved);
-	pr_debug("SPI %u %08X\n", ntohl(ah->spi), ntohl(ah->spi));
-
-	pr_debug("IPv6 AH spi %02X ",
-		 spi_match(ahinfo->spis[0], ahinfo->spis[1],
-			   ntohl(ah->spi),
-			   !!(ahinfo->invflags & IP6T_AH_INV_SPI)));
-	pr_debug("len %02X %04X %02X ",
-		 ahinfo->hdrlen, hdrlen,
-		 (!ahinfo->hdrlen ||
-		  (ahinfo->hdrlen == hdrlen) ^
-		  !!(ahinfo->invflags & IP6T_AH_INV_LEN)));
-	pr_debug("res %02X %04X %02X\n",
-		 ahinfo->hdrres, ah->reserved,
-		 !(ahinfo->hdrres && ah->reserved));
-
 	return spi_match(ahinfo->spis[0], ahinfo->spis[1],
 			  ntohl(ah->spi),
 			  !!(ahinfo->invflags & IP6T_AH_INV_SPI)) &&
@@ -93,7 +70,7 @@ static int ah_mt6_check(const struct xt_mtchk_param *par)
 	const struct ip6t_ah *ahinfo = par->matchinfo;
 
 	if (ahinfo->invflags & ~IP6T_AH_INV_MASK) {
-		pr_debug("unknown flags %X\n", ahinfo->invflags);
+		pr_info_ratelimited("unknown flags %X\n", ahinfo->invflags);
 		return -EINVAL;
 	}
 	return 0;
diff --git a/net/ipv6/netfilter/ip6t_frag.c b/net/ipv6/netfilter/ip6t_frag.c
index 3aad6439386b..f5f3cfb8704c 100644
--- a/net/ipv6/netfilter/ip6t_frag.c
+++ b/net/ipv6/netfilter/ip6t_frag.c
@@ -23,12 +23,7 @@ MODULE_AUTHOR("Andras Kis-Szabo <kisza@sch.bme.hu>");
 static inline bool
 id_match(u_int32_t min, u_int32_t max, u_int32_t id, bool invert)
 {
-	bool r;
-	pr_debug("id_match:%c 0x%x <= 0x%x <= 0x%x\n", invert ? '!' : ' ',
-		 min, id, max);
-	r = (id >= min && id <= max) ^ invert;
-	pr_debug(" result %s\n", r ? "PASS" : "FAILED");
-	return r;
+	return (id >= min && id <= max) ^ invert;
 }
 
 static bool
@@ -53,38 +48,6 @@ frag_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 		return false;
 	}
 
-	pr_debug("INFO %04X ", fh->frag_off);
-	pr_debug("OFFSET %04X ", ntohs(fh->frag_off) & ~0x7);
-	pr_debug("RES %02X %04X", fh->reserved, ntohs(fh->frag_off) & 0x6);
-	pr_debug("MF %04X ", fh->frag_off & htons(IP6_MF));
-	pr_debug("ID %u %08X\n", ntohl(fh->identification),
-		 ntohl(fh->identification));
-
-	pr_debug("IPv6 FRAG id %02X ",
-		 id_match(fraginfo->ids[0], fraginfo->ids[1],
-			  ntohl(fh->identification),
-			  !!(fraginfo->invflags & IP6T_FRAG_INV_IDS)));
-	pr_debug("res %02X %02X%04X %02X ",
-		 fraginfo->flags & IP6T_FRAG_RES, fh->reserved,
-		 ntohs(fh->frag_off) & 0x6,
-		 !((fraginfo->flags & IP6T_FRAG_RES) &&
-		   (fh->reserved || (ntohs(fh->frag_off) & 0x06))));
-	pr_debug("first %02X %02X %02X ",
-		 fraginfo->flags & IP6T_FRAG_FST,
-		 ntohs(fh->frag_off) & ~0x7,
-		 !((fraginfo->flags & IP6T_FRAG_FST) &&
-		   (ntohs(fh->frag_off) & ~0x7)));
-	pr_debug("mf %02X %02X %02X ",
-		 fraginfo->flags & IP6T_FRAG_MF,
-		 ntohs(fh->frag_off) & IP6_MF,
-		 !((fraginfo->flags & IP6T_FRAG_MF) &&
-		   !((ntohs(fh->frag_off) & IP6_MF))));
-	pr_debug("last %02X %02X %02X\n",
-		 fraginfo->flags & IP6T_FRAG_NMF,
-		 ntohs(fh->frag_off) & IP6_MF,
-		 !((fraginfo->flags & IP6T_FRAG_NMF) &&
-		   (ntohs(fh->frag_off) & IP6_MF)));
-
 	return id_match(fraginfo->ids[0], fraginfo->ids[1],
 			 ntohl(fh->identification),
 			 !!(fraginfo->invflags & IP6T_FRAG_INV_IDS)) &&
@@ -103,7 +66,7 @@ static int frag_mt6_check(const struct xt_mtchk_param *par)
 	const struct ip6t_frag *fraginfo = par->matchinfo;
 
 	if (fraginfo->invflags & ~IP6T_FRAG_INV_MASK) {
-		pr_debug("unknown flags %X\n", fraginfo->invflags);
+		pr_info_ratelimited("unknown flags %X\n", fraginfo->invflags);
 		return -EINVAL;
 	}
 	return 0;
diff --git a/net/ipv6/netfilter/ip6t_hbh.c b/net/ipv6/netfilter/ip6t_hbh.c
index 6d1a5d2026a6..37c207d3152a 100644
--- a/net/ipv6/netfilter/ip6t_hbh.c
+++ b/net/ipv6/netfilter/ip6t_hbh.c
@@ -79,14 +79,6 @@ hbh_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 		return false;
 	}
 
-	pr_debug("IPv6 OPTS LEN %u %u ", hdrlen, oh->hdrlen);
-
-	pr_debug("len %02X %04X %02X ",
-		 optinfo->hdrlen, hdrlen,
-		 (!(optinfo->flags & IP6T_OPTS_LEN) ||
-		  ((optinfo->hdrlen == hdrlen) ^
-		   !!(optinfo->invflags & IP6T_OPTS_INV_LEN))));
-
 	ret = (!(optinfo->flags & IP6T_OPTS_LEN) ||
 	       ((optinfo->hdrlen == hdrlen) ^
 		!!(optinfo->invflags & IP6T_OPTS_INV_LEN)));
@@ -96,8 +88,6 @@ hbh_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 	if (!(optinfo->flags & IP6T_OPTS_OPTS)) {
 		return ret;
 	} else {
-		pr_debug("Strict ");
-		pr_debug("#%d ", optinfo->optsnr);
 		for (temp = 0; temp < optinfo->optsnr; temp++) {
 			/* type field exists ? */
 			if (hdrlen < 1)
@@ -108,13 +98,9 @@ hbh_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 				break;
 
 			/* Type check */
-			if (*tp != (optinfo->opts[temp] & 0xFF00) >> 8) {
-				pr_debug("Tbad %02X %02X\n", *tp,
-					 (optinfo->opts[temp] & 0xFF00) >> 8);
+			if (*tp != (optinfo->opts[temp] & 0xFF00) >> 8)
 				return false;
-			} else {
-				pr_debug("Tok ");
-			}
+
 			/* Length check */
 			if (*tp) {
 				u16 spec_len;
@@ -129,26 +115,18 @@ hbh_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 					break;
 				spec_len = optinfo->opts[temp] & 0x00FF;
 
-				if (spec_len != 0x00FF && spec_len != *lp) {
-					pr_debug("Lbad %02X %04X\n", *lp,
-						 spec_len);
+				if (spec_len != 0x00FF && spec_len != *lp)
 					return false;
-				}
-				pr_debug("Lok ");
+
 				optlen = *lp + 2;
 			} else {
-				pr_debug("Pad1\n");
 				optlen = 1;
 			}
 
-			/* Step to the next */
-			pr_debug("len%04X\n", optlen);
-
 			if ((ptr > skb->len - optlen || hdrlen < optlen) &&
-			    temp < optinfo->optsnr - 1) {
-				pr_debug("new pointer is too large!\n");
+			    temp < optinfo->optsnr - 1)
 				break;
-			}
+
 			ptr += optlen;
 			hdrlen -= optlen;
 		}
@@ -166,16 +144,16 @@ static int hbh_mt6_check(const struct xt_mtchk_param *par)
 	const struct ip6t_opts *optsinfo = par->matchinfo;
 
 	if (optsinfo->invflags & ~IP6T_OPTS_INV_MASK) {
-		pr_debug("unknown flags %X\n", optsinfo->invflags);
+		pr_info_ratelimited("unknown flags %X\n", optsinfo->invflags);
 		return -EINVAL;
 	}
 	if (optsinfo->optsnr > IP6T_OPTS_OPTSNR) {
-		pr_debug("too many supported opts specified\n");
+		pr_info_ratelimited("too many supported opts specified\n");
 		return -EINVAL;
 	}
 
 	if (optsinfo->flags & IP6T_OPTS_NSTRICT) {
-		pr_debug("Not strict - not implemented");
+		pr_info_ratelimited("Not strict - not implemented");
 		return -EINVAL;
 	}
 
diff --git a/net/ipv6/netfilter/ip6t_mh.c b/net/ipv6/netfilter/ip6t_mh.c
index fd492b69acbc..ba6dcc7791a0 100644
--- a/net/ipv6/netfilter/ip6t_mh.c
+++ b/net/ipv6/netfilter/ip6t_mh.c
@@ -42,14 +42,11 @@ static bool mh_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 	if (mh == NULL) {
 		/* We've been asked to examine this packet, and we
 		   can't.  Hence, no choice but to drop. */
-		pr_debug("Dropping evil MH tinygram.\n");
 		par->hotdrop = true;
 		return false;
 	}
 
 	if (mh->ip6mh_proto != IPPROTO_NONE) {
-		pr_debug("Dropping invalid MH Payload Proto: %u\n",
-			 mh->ip6mh_proto);
 		par->hotdrop = true;
 		return false;
 	}
diff --git a/net/ipv6/netfilter/ip6t_rt.c b/net/ipv6/netfilter/ip6t_rt.c
index 278b52752f36..0c01ac2746d1 100644
--- a/net/ipv6/netfilter/ip6t_rt.c
+++ b/net/ipv6/netfilter/ip6t_rt.c
@@ -155,18 +155,18 @@ static int rt_mt6_check(const struct xt_mtchk_param *par)
 	const struct ip6t_rt *rtinfo = par->matchinfo;
 
 	if (rtinfo->invflags & ~IP6T_RT_INV_MASK) {
-		pr_debug("unknown flags %X\n", rtinfo->invflags);
+		pr_info_ratelimited("unknown flags %X\n", rtinfo->invflags);
 		return -EINVAL;
 	}
 	if (rtinfo->addrnr > IP6T_RT_HOPS) {
-		pr_debug("too many addresses specified\n");
+		pr_info_ratelimited("too many addresses specified\n");
 		return -EINVAL;
 	}
 	if ((rtinfo->flags & (IP6T_RT_RES | IP6T_RT_FST_MASK)) &&
 	    (!(rtinfo->flags & IP6T_RT_TYP) ||
 	     (rtinfo->rt_type != 0) ||
 	     (rtinfo->invflags & IP6T_RT_INV_TYP))) {
-		pr_debug("`--rt-type 0' required before `--rt-0-*'");
+		pr_info_ratelimited("`--rt-type 0' required before `--rt-0-*'");
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_IDLETIMER.c b/net/netfilter/xt_IDLETIMER.c
index bfcf2d44e93d..15b698ea98aa 100644
--- a/net/netfilter/xt_IDLETIMER.c
+++ b/net/netfilter/xt_IDLETIMER.c
@@ -102,8 +102,6 @@ static void idletimer_tg_expired(struct timer_list *t)
 {
 	struct idletimer_tg *timer = timer_container_of(timer, t, timer);
 
-	pr_debug("timer %s expired\n", timer->attr.attr.name);
-
 	schedule_work(&timer->work);
 }
 
@@ -111,7 +109,6 @@ static void idletimer_tg_alarmproc(struct alarm *alarm, ktime_t now)
 {
 	struct idletimer_tg *timer = alarm->data;
 
-	pr_debug("alarm %s expired\n", timer->attr.attr.name);
 	schedule_work(&timer->work);
 }
 
@@ -171,7 +168,7 @@ static int idletimer_tg_create(struct idletimer_tg_info *info)
 
 	ret = sysfs_create_file(idletimer_tg_kobj, &info->timer->attr.attr);
 	if (ret < 0) {
-		pr_debug("couldn't add file to sysfs");
+		pr_info_ratelimited("couldn't add file to sysfs");
 		goto out_free_attr;
 	}
 
@@ -220,7 +217,7 @@ static int idletimer_tg_create_v1(struct idletimer_tg_info_v1 *info)
 
 	ret = sysfs_create_file(idletimer_tg_kobj, &info->timer->attr.attr);
 	if (ret < 0) {
-		pr_debug("couldn't add file to sysfs");
+		pr_info_ratelimited("couldn't add file to sysfs");
 		goto out_free_attr;
 	}
 
@@ -228,7 +225,6 @@ static int idletimer_tg_create_v1(struct idletimer_tg_info_v1 *info)
 	kobject_uevent(idletimer_tg_kobj,KOBJ_ADD);
 
 	list_add(&info->timer->entry, &idletimer_tg_list);
-	pr_debug("timer type value is %u", info->timer_type);
 	info->timer->timer_type = info->timer_type;
 	info->timer->refcnt = 1;
 
@@ -263,9 +259,6 @@ static unsigned int idletimer_tg_target(struct sk_buff *skb,
 {
 	const struct idletimer_tg_info *info = par->targinfo;
 
-	pr_debug("resetting timer %s, timeout period %u\n",
-		 info->label, info->timeout);
-
 	mod_timer(&info->timer->timer,
 		  secs_to_jiffies(info->timeout) + jiffies);
 
@@ -280,9 +273,6 @@ static unsigned int idletimer_tg_target_v1(struct sk_buff *skb,
 {
 	const struct idletimer_tg_info_v1 *info = par->targinfo;
 
-	pr_debug("resetting timer %s, timeout period %u\n",
-		 info->label, info->timeout);
-
 	if (info->timer->timer_type & XT_IDLETIMER_ALARM) {
 		idletimer_start_alarm_sec(info->timer, info->timeout);
 	} else {
@@ -296,17 +286,17 @@ static unsigned int idletimer_tg_target_v1(struct sk_buff *skb,
 static int idletimer_tg_helper(struct idletimer_tg_info *info)
 {
 	if (info->timeout == 0) {
-		pr_debug("timeout value is zero\n");
+		pr_info_ratelimited("timeout value is zero\n");
 		return -EINVAL;
 	}
 	if (info->timeout >= INT_MAX / 1000) {
-		pr_debug("timeout value is too big\n");
+		pr_info_ratelimited("timeout value is too big\n");
 		return -EINVAL;
 	}
 	if (info->label[0] == '\0' ||
 	    strnlen(info->label,
 		    MAX_IDLETIMER_LABEL_SIZE) == MAX_IDLETIMER_LABEL_SIZE) {
-		pr_debug("label is empty or not nul-terminated\n");
+		pr_info_ratelimited("label is empty or not nul-terminated\n");
 		return -EINVAL;
 	}
 	return 0;
@@ -318,34 +308,25 @@ static int idletimer_tg_checkentry(const struct xt_tgchk_param *par)
 	struct idletimer_tg_info *info = par->targinfo;
 	int ret;
 
-	pr_debug("checkentry targinfo%s\n", info->label);
-
 	ret = idletimer_tg_helper(info);
 	if(ret < 0)
-	{
-		pr_debug("checkentry helper return invalid\n");
 		return -EINVAL;
-	}
 	mutex_lock(&list_mutex);
 
 	info->timer = __idletimer_tg_find_by_label(info->label);
 	if (info->timer) {
 		if (info->timer->timer_type & XT_IDLETIMER_ALARM) {
-			pr_debug("Adding/Replacing rule with same label and different timer type is not allowed\n");
 			mutex_unlock(&list_mutex);
+			pr_info_ratelimited("Adding/Replacing rule with same label and different timer type is not allowed\n");
 			return -EINVAL;
 		}
 
 		info->timer->refcnt++;
 		mod_timer(&info->timer->timer,
 			  secs_to_jiffies(info->timeout) + jiffies);
-
-		pr_debug("increased refcnt of timer %s to %u\n",
-			 info->label, info->timer->refcnt);
 	} else {
 		ret = idletimer_tg_create(info);
 		if (ret < 0) {
-			pr_debug("failed to create timer\n");
 			mutex_unlock(&list_mutex);
 			return ret;
 		}
@@ -360,30 +341,23 @@ static int idletimer_tg_checkentry_v1(const struct xt_tgchk_param *par)
 	struct idletimer_tg_info_v1 *info = par->targinfo;
 	int ret;
 
-	pr_debug("checkentry targinfo%s\n", info->label);
-
 	if (info->send_nl_msg)
 		return -EOPNOTSUPP;
 
 	ret = idletimer_tg_helper((struct idletimer_tg_info *)info);
 	if(ret < 0)
-	{
-		pr_debug("checkentry helper return invalid\n");
 		return -EINVAL;
-	}
 
-	if (info->timer_type > XT_IDLETIMER_ALARM) {
-		pr_debug("invalid value for timer type\n");
+	if (info->timer_type > XT_IDLETIMER_ALARM)
 		return -EINVAL;
-	}
 
 	mutex_lock(&list_mutex);
 
 	info->timer = __idletimer_tg_find_by_label(info->label);
 	if (info->timer) {
 		if (info->timer->timer_type != info->timer_type) {
-			pr_debug("Adding/Replacing rule with same label and different timer type is not allowed\n");
 			mutex_unlock(&list_mutex);
+			pr_info_ratelimited("Adding/Replacing rule with same label and different timer type is not allowed\n");
 			return -EINVAL;
 		}
 
@@ -393,21 +367,15 @@ static int idletimer_tg_checkentry_v1(const struct xt_tgchk_param *par)
 			ktime_t tout = alarm_expires_remaining(&info->timer->alarm);
 			struct timespec64 ktimespec = ktime_to_timespec64(tout);
 
-			if (ktimespec.tv_sec > 0) {
-				pr_debug("time_expiry_remaining %lld\n",
-					 ktimespec.tv_sec);
+			if (ktimespec.tv_sec > 0)
 				idletimer_start_alarm_ktime(info->timer, tout);
-			}
 		} else {
 				mod_timer(&info->timer->timer,
 					secs_to_jiffies(info->timeout) + jiffies);
 		}
-		pr_debug("increased refcnt of timer %s to %u\n",
-			 info->label, info->timer->refcnt);
 	} else {
 		ret = idletimer_tg_create_v1(info);
 		if (ret < 0) {
-			pr_debug("failed to create timer\n");
 			mutex_unlock(&list_mutex);
 			return ret;
 		}
@@ -421,19 +389,13 @@ static void idletimer_tg_destroy(const struct xt_tgdtor_param *par)
 {
 	const struct idletimer_tg_info *info = par->targinfo;
 
-	pr_debug("destroy targinfo %s\n", info->label);
-
 	mutex_lock(&list_mutex);
 
 	if (--info->timer->refcnt > 0) {
-		pr_debug("decreased refcnt of timer %s to %u\n",
-			 info->label, info->timer->refcnt);
 		mutex_unlock(&list_mutex);
 		return;
 	}
 
-	pr_debug("deleting timer %s\n", info->label);
-
 	list_del(&info->timer->entry);
 	mutex_unlock(&list_mutex);
 
@@ -448,19 +410,13 @@ static void idletimer_tg_destroy_v1(const struct xt_tgdtor_param *par)
 {
 	const struct idletimer_tg_info_v1 *info = par->targinfo;
 
-	pr_debug("destroy targinfo %s\n", info->label);
-
 	mutex_lock(&list_mutex);
 
 	if (--info->timer->refcnt > 0) {
-		pr_debug("decreased refcnt of timer %s to %u\n",
-			 info->label, info->timer->refcnt);
 		mutex_unlock(&list_mutex);
 		return;
 	}
 
-	pr_debug("deleting timer %s\n", info->label);
-
 	list_del(&info->timer->entry);
 	mutex_unlock(&list_mutex);
 
@@ -534,7 +490,7 @@ static int __init idletimer_tg_init(void)
 	idletimer_tg_class = class_create("xt_idletimer");
 	err = PTR_ERR(idletimer_tg_class);
 	if (IS_ERR(idletimer_tg_class)) {
-		pr_debug("couldn't register device class\n");
+		pr_info_ratelimited("couldn't register device class\n");
 		goto out;
 	}
 
@@ -542,7 +498,7 @@ static int __init idletimer_tg_init(void)
 					    MKDEV(0, 0), NULL, "timers");
 	err = PTR_ERR(idletimer_tg_device);
 	if (IS_ERR(idletimer_tg_device)) {
-		pr_debug("couldn't register system device\n");
+		pr_info_ratelimited("couldn't register system device\n");
 		goto out_class;
 	}
 
@@ -551,7 +507,7 @@ static int __init idletimer_tg_init(void)
 	err = xt_register_targets(idletimer_tg, ARRAY_SIZE(idletimer_tg));
 
 	if (err < 0) {
-		pr_debug("couldn't register xt target\n");
+		pr_info_ratelimited("couldn't register xt target\n");
 		goto out_dev;
 	}
 
diff --git a/net/netfilter/xt_LOG.c b/net/netfilter/xt_LOG.c
index f39244f9c0ed..de3f176792a0 100644
--- a/net/netfilter/xt_LOG.c
+++ b/net/netfilter/xt_LOG.c
@@ -50,12 +50,12 @@ static int log_tg_check(const struct xt_tgchk_param *par)
 		return -EINVAL;
 
 	if (loginfo->level >= 8) {
-		pr_debug("level %u >= 8\n", loginfo->level);
+		pr_info_ratelimited("level %u >= 8\n", loginfo->level);
 		return -EINVAL;
 	}
 
 	if (loginfo->prefix[sizeof(loginfo->prefix)-1] != '\0') {
-		pr_debug("prefix is not null-terminated\n");
+		pr_info_ratelimited("prefix is not null-terminated\n");
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_MASQUERADE.c b/net/netfilter/xt_MASQUERADE.c
index eae05c178336..cea488cec544 100644
--- a/net/netfilter/xt_MASQUERADE.c
+++ b/net/netfilter/xt_MASQUERADE.c
@@ -21,11 +21,11 @@ static int masquerade_tg_check(const struct xt_tgchk_param *par)
 	const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo;
 
 	if (mr->range[0].flags & NF_NAT_RANGE_MAP_IPS) {
-		pr_debug("bad MAP_IPS.\n");
+		pr_info_ratelimited("bad MAP_IPS.\n");
 		return -EINVAL;
 	}
 	if (mr->rangesize != 1) {
-		pr_debug("bad rangesize %u\n", mr->rangesize);
+		pr_info_ratelimited("bad rangesize %u\n", mr->rangesize);
 		return -EINVAL;
 	}
 	return nf_ct_netns_get(par->net, par->family);
diff --git a/net/netfilter/xt_NETMAP.c b/net/netfilter/xt_NETMAP.c
index cb2ee80d84fa..8c86d9a43373 100644
--- a/net/netfilter/xt_NETMAP.c
+++ b/net/netfilter/xt_NETMAP.c
@@ -106,11 +106,11 @@ static int netmap_tg4_check(const struct xt_tgchk_param *par)
 	const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo;
 
 	if (!(mr->range[0].flags & NF_NAT_RANGE_MAP_IPS)) {
-		pr_debug("bad MAP_IPS.\n");
+		pr_info_ratelimited("bad MAP_IPS.\n");
 		return -EINVAL;
 	}
 	if (mr->rangesize != 1) {
-		pr_debug("bad rangesize %u.\n", mr->rangesize);
+		pr_info_ratelimited("bad rangesize %u.\n", mr->rangesize);
 		return -EINVAL;
 	}
 	return nf_ct_netns_get(par->net, par->family);
diff --git a/net/netfilter/xt_REDIRECT.c b/net/netfilter/xt_REDIRECT.c
index ff66b56a3f97..27bc0545c4f7 100644
--- a/net/netfilter/xt_REDIRECT.c
+++ b/net/netfilter/xt_REDIRECT.c
@@ -51,11 +51,11 @@ static int redirect_tg4_check(const struct xt_tgchk_param *par)
 	const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo;
 
 	if (mr->range[0].flags & NF_NAT_RANGE_MAP_IPS) {
-		pr_debug("bad MAP_IPS.\n");
+		pr_info_ratelimited("bad MAP_IPS.\n");
 		return -EINVAL;
 	}
 	if (mr->rangesize != 1) {
-		pr_debug("bad rangesize %u.\n", mr->rangesize);
+		pr_info_ratelimited("bad rangesize %u.\n", mr->rangesize);
 		return -EINVAL;
 	}
 	return nf_ct_netns_get(par->net, par->family);
diff --git a/net/netfilter/xt_esp.c b/net/netfilter/xt_esp.c
index 2a1c0ad0ff07..68fd75884268 100644
--- a/net/netfilter/xt_esp.c
+++ b/net/netfilter/xt_esp.c
@@ -25,12 +25,7 @@ MODULE_ALIAS("ip6t_esp");
 static inline bool
 spi_match(u_int32_t min, u_int32_t max, u_int32_t spi, bool invert)
 {
-	bool r;
-	pr_debug("spi_match:%c 0x%x <= 0x%x <= 0x%x\n",
-		 invert ? '!' : ' ', min, spi, max);
-	r = (spi >= min && spi <= max) ^ invert;
-	pr_debug(" result %s\n", r ? "PASS" : "FAILED");
-	return r;
+	return (spi >= min && spi <= max) ^ invert;
 }
 
 static bool esp_mt(const struct sk_buff *skb, struct xt_action_param *par)
@@ -48,7 +43,6 @@ static bool esp_mt(const struct sk_buff *skb, struct xt_action_param *par)
 		/* We've been asked to examine this packet, and we
 		 * can't.  Hence, no choice but to drop.
 		 */
-		pr_debug("Dropping evil ESP tinygram.\n");
 		par->hotdrop = true;
 		return false;
 	}
@@ -62,7 +56,7 @@ static int esp_mt_check(const struct xt_mtchk_param *par)
 	const struct xt_esp *espinfo = par->matchinfo;
 
 	if (espinfo->invflags & ~XT_ESP_INV_MASK) {
-		pr_debug("unknown flags %X\n", espinfo->invflags);
+		pr_info_ratelimited("unknown flags %X\n", espinfo->invflags);
 		return -EINVAL;
 	}
 
diff --git a/net/netfilter/xt_ipcomp.c b/net/netfilter/xt_ipcomp.c
index 472da639a32e..3299c1ea60f9 100644
--- a/net/netfilter/xt_ipcomp.c
+++ b/net/netfilter/xt_ipcomp.c
@@ -29,12 +29,7 @@ MODULE_ALIAS("ip6t_ipcomp");
 static inline bool
 spi_match(u_int32_t min, u_int32_t max, u_int32_t spi, bool invert)
 {
-	bool r;
-	pr_debug("spi_match:%c 0x%x <= 0x%x <= 0x%x\n",
-		 invert ? '!' : ' ', min, spi, max);
-	r = (spi >= min && spi <= max) ^ invert;
-	pr_debug(" result %s\n", r ? "PASS" : "FAILED");
-	return r;
+	return (spi >= min && spi <= max) ^ invert;
 }
 
 static bool comp_mt(const struct sk_buff *skb, struct xt_action_param *par)
@@ -52,7 +47,6 @@ static bool comp_mt(const struct sk_buff *skb, struct xt_action_param *par)
 		/* We've been asked to examine this packet, and we
 		 * can't.  Hence, no choice but to drop.
 		 */
-		pr_debug("Dropping evil IPComp tinygram.\n");
 		par->hotdrop = true;
 		return false;
 	}
diff --git a/net/netfilter/xt_iprange.c b/net/netfilter/xt_iprange.c
index 0c9e014e30b4..bf61141fb785 100644
--- a/net/netfilter/xt_iprange.c
+++ b/net/netfilter/xt_iprange.c
@@ -24,27 +24,15 @@ iprange_mt4(const struct sk_buff *skb, struct xt_action_param *par)
 		m  = ntohl(iph->saddr) < ntohl(info->src_min.ip);
 		m |= ntohl(iph->saddr) > ntohl(info->src_max.ip);
 		m ^= !!(info->flags & IPRANGE_SRC_INV);
-		if (m) {
-			pr_debug("src IP %pI4 NOT in range %s%pI4-%pI4\n",
-			         &iph->saddr,
-			         (info->flags & IPRANGE_SRC_INV) ? "(INV) " : "",
-			         &info->src_min.ip,
-			         &info->src_max.ip);
+		if (m)
 			return false;
-		}
 	}
 	if (info->flags & IPRANGE_DST) {
 		m  = ntohl(iph->daddr) < ntohl(info->dst_min.ip);
 		m |= ntohl(iph->daddr) > ntohl(info->dst_max.ip);
 		m ^= !!(info->flags & IPRANGE_DST_INV);
-		if (m) {
-			pr_debug("dst IP %pI4 NOT in range %s%pI4-%pI4\n",
-			         &iph->daddr,
-			         (info->flags & IPRANGE_DST_INV) ? "(INV) " : "",
-			         &info->dst_min.ip,
-			         &info->dst_max.ip);
+		if (m)
 			return false;
-		}
 	}
 	return true;
 }
@@ -73,27 +61,15 @@ iprange_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 		m  = iprange_ipv6_lt(&iph->saddr, &info->src_min.in6);
 		m |= iprange_ipv6_lt(&info->src_max.in6, &iph->saddr);
 		m ^= !!(info->flags & IPRANGE_SRC_INV);
-		if (m) {
-			pr_debug("src IP %pI6 NOT in range %s%pI6-%pI6\n",
-				 &iph->saddr,
-				 (info->flags & IPRANGE_SRC_INV) ? "(INV) " : "",
-				 &info->src_min.in6,
-				 &info->src_max.in6);
+		if (m)
 			return false;
-		}
 	}
 	if (info->flags & IPRANGE_DST) {
 		m  = iprange_ipv6_lt(&iph->daddr, &info->dst_min.in6);
 		m |= iprange_ipv6_lt(&info->dst_max.in6, &iph->daddr);
 		m ^= !!(info->flags & IPRANGE_DST_INV);
-		if (m) {
-			pr_debug("dst IP %pI6 NOT in range %s%pI6-%pI6\n",
-				 &iph->daddr,
-				 (info->flags & IPRANGE_DST_INV) ? "(INV) " : "",
-				 &info->dst_min.in6,
-				 &info->dst_max.in6);
+		if (m)
 			return false;
-		}
 	}
 	return true;
 }
diff --git a/net/netfilter/xt_ipvs.c b/net/netfilter/xt_ipvs.c
index 253c71cc9a63..e13c0ffb73a9 100644
--- a/net/netfilter/xt_ipvs.c
+++ b/net/netfilter/xt_ipvs.c
@@ -148,7 +148,6 @@ ipvs_mt(const struct sk_buff *skb, struct xt_action_param *par)
 out_put_cp:
 	__ip_vs_conn_put(cp);
 out:
-	pr_debug("match=%d\n", match);
 	return match;
 }
 
diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c
index a1691ff405d3..bff5f53a9bef 100644
--- a/net/netfilter/xt_multiport.c
+++ b/net/netfilter/xt_multiport.c
@@ -37,7 +37,6 @@ ports_match_v1(const struct xt_multiport_v1 *minfo,
 		if (minfo->pflags[i]) {
 			/* range port matching */
 			e = minfo->ports[++i];
-			pr_debug("src or dst matches with %d-%d?\n", s, e);
 
 			switch (minfo->flags) {
 			case XT_MULTIPORT_SOURCE:
@@ -58,8 +57,6 @@ ports_match_v1(const struct xt_multiport_v1 *minfo,
 			}
 		} else {
 			/* exact port matching */
-			pr_debug("src or dst matches with %d?\n", s);
-
 			switch (minfo->flags) {
 			case XT_MULTIPORT_SOURCE:
 				if (src == s)
@@ -97,7 +94,6 @@ multiport_mt(const struct sk_buff *skb, struct xt_action_param *par)
 		/* We've been asked to examine this packet, and we
 		 * can't.  Hence, no choice but to drop.
 		 */
-		pr_debug("Dropping evil offset=0 tinygram.\n");
 		par->hotdrop = true;
 		return false;
 	}
diff --git a/net/netfilter/xt_sctp.c b/net/netfilter/xt_sctp.c
index b46a6a512058..d35c21d9651b 100644
--- a/net/netfilter/xt_sctp.c
+++ b/net/netfilter/xt_sctp.c
@@ -48,30 +48,17 @@ match_packet(const struct sk_buff *skb,
 	const struct xt_sctp_flag_info *flag_info = info->flag_info;
 	int flag_count = info->flag_count;
 
-#ifdef DEBUG
-	int i = 0;
-#endif
-
 	if (chunk_match_type == SCTP_CHUNK_MATCH_ALL)
 		SCTP_CHUNKMAP_COPY(chunkmapcopy, info->chunkmap);
 
 	do {
 		sch = skb_header_pointer(skb, offset, sizeof(_sch), &_sch);
 		if (sch == NULL || sch->length == 0) {
-			pr_debug("Dropping invalid SCTP packet.\n");
 			*hotdrop = true;
 			return false;
 		}
-#ifdef DEBUG
-		pr_debug("Chunk num: %d\toffset: %d\ttype: %d\tlength: %d"
-			 "\tflags: %x\n",
-			 ++i, offset, sch->type, htons(sch->length),
-			 sch->flags);
-#endif
 		offset += SCTP_PAD4(ntohs(sch->length));
 
-		pr_debug("skb->len: %d\toffset: %d\n", skb->len, offset);
-
 		if (SCTP_CHUNKMAP_IS_SET(info->chunkmap, sch->type)) {
 			switch (chunk_match_type) {
 			case SCTP_CHUNK_MATCH_ANY:
@@ -121,18 +108,14 @@ sctp_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	const struct sctphdr *sh;
 	struct sctphdr _sh;
 
-	if (par->fragoff != 0) {
-		pr_debug("Dropping non-first fragment.. FIXME\n");
+	if (par->fragoff != 0)
 		return false;
-	}
 
 	sh = skb_header_pointer(skb, par->thoff, sizeof(_sh), &_sh);
 	if (sh == NULL) {
-		pr_debug("Dropping evil TCP offset=0 tinygram.\n");
 		par->hotdrop = true;
 		return false;
 	}
-	pr_debug("spt: %d\tdpt: %d\n", ntohs(sh->source), ntohs(sh->dest));
 
 	return  SCCHECK(ntohs(sh->source) >= info->spts[0]
 			&& ntohs(sh->source) <= info->spts[1],
diff --git a/net/netfilter/xt_tcpudp.c b/net/netfilter/xt_tcpudp.c
index f76cf18f1a24..70608b8d06ab 100644
--- a/net/netfilter/xt_tcpudp.c
+++ b/net/netfilter/xt_tcpudp.c
@@ -44,8 +44,6 @@ tcp_find_option(u_int8_t option,
 	u_int8_t _opt[60 - sizeof(struct tcphdr)];
 	unsigned int i;
 
-	pr_debug("finding option\n");
-
 	if (!optlen)
 		return invert;
 
@@ -81,10 +79,8 @@ static bool tcp_mt(const struct sk_buff *skb, struct xt_action_param *par)
 		   causes this. Its a cracker trying to break in by doing a
 		   flag overwrite to pass the direction checks.
 		*/
-		if (par->fragoff == 1) {
-			pr_debug("Dropping evil TCP offset=1 frag.\n");
+		if (par->fragoff == 1)
 			par->hotdrop = true;
-		}
 		/* Must not be a fragment. */
 		return false;
 	}
@@ -93,7 +89,6 @@ static bool tcp_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	if (th == NULL) {
 		/* We've been asked to examine this packet, and we
 		   can't.  Hence, no choice but to drop. */
-		pr_debug("Dropping evil TCP offset=0 tinygram.\n");
 		par->hotdrop = true;
 		return false;
 	}
@@ -145,7 +140,6 @@ static bool udp_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	if (uh == NULL) {
 		/* We've been asked to examine this packet, and we
 		   can't.  Hence, no choice but to drop. */
-		pr_debug("Dropping evil UDP tinygram.\n");
 		par->hotdrop = true;
 		return false;
 	}
-- 
2.47.3

Re: [PATCH net 05/10] netfilter: x_tables: remove pr_debug

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
netfilter: x_tables: remove pr_debug

This drops the pr_debug() calls from a set of xtables matches and
targets, and converts the ones in the .check paths into
pr_info_ratelimited() so that legacy iptables users get a hint in dmesg
when a rule is rejected.

A few observations below.

> diff --git a/net/ipv6/netfilter/ip6t_hbh.c b/net/ipv6/netfilter/ip6t_hbh.c
> index 6d1a5d2026a67..37c207d3152a4 100644
> --- a/net/ipv6/netfilter/ip6t_hbh.c
> +++ b/net/ipv6/netfilter/ip6t_hbh.c

[ ... ]

> @@ -166,16 +144,16 @@ static int hbh_mt6_check(const struct xt_mtchk_param *par)
>  	const struct ip6t_opts *optsinfo = par->matchinfo;
>  
>  	if (optsinfo->invflags & ~IP6T_OPTS_INV_MASK) {
> -		pr_debug("unknown flags %X\n", optsinfo->invflags);
> +		pr_info_ratelimited("unknown flags %X\n", optsinfo->invflags);
>  		return -EINVAL;
>  	}
>  	if (optsinfo->optsnr > IP6T_OPTS_OPTSNR) {
> -		pr_debug("too many supported opts specified\n");
> +		pr_info_ratelimited("too many supported opts specified\n");
>  		return -EINVAL;
>  	}
>  
>  	if (optsinfo->flags & IP6T_OPTS_NSTRICT) {
> -		pr_debug("Not strict - not implemented");
> +		pr_info_ratelimited("Not strict - not implemented");
>  		return -EINVAL;
>  	}

[Severity: Low]
Should this string get a trailing newline like its two siblings in
hbh_mt6_check()?

The missing '\n' was harmless while this was pr_debug() (compiled out or
dynamic-debug gated), but as an unconditional pr_info_ratelimited() the
record is left open for continuation:

kernel/printk/printk.c:printk_sprint() {
	/* Mark and strip a trailing newline. */
	if (text_len && text[text_len - 1] == '\n') {
		text_len--;
		*flags |= LOG_NEWLINE;
	}
}

kernel/printk/printk.c:vprintk_store() {
	/* A message without a trailing newline can be continued. */
	if (!(flags & LOG_NEWLINE))
		prb_commit(&e);
	else
		prb_final_commit(&e);
}

So an iptables-legacy rule with IP6T_OPTS_NSTRICT set produces a dmesg
line that gets glued to whatever is printed next, which works against the
"hint via dmesg" goal in the changelog.

The same missing '\n' appears in three more of the converted strings, noted
below.

> diff --git a/net/ipv6/netfilter/ip6t_rt.c b/net/ipv6/netfilter/ip6t_rt.c
> index 278b52752f364..0c01ac2746d1f 100644
> --- a/net/ipv6/netfilter/ip6t_rt.c
> +++ b/net/ipv6/netfilter/ip6t_rt.c
> @@ -155,18 +155,18 @@ static int rt_mt6_check(const struct xt_mtchk_param *par)
>  	const struct ip6t_rt *rtinfo = par->matchinfo;
>  
>  	if (rtinfo->invflags & ~IP6T_RT_INV_MASK) {
> -		pr_debug("unknown flags %X\n", rtinfo->invflags);
> +		pr_info_ratelimited("unknown flags %X\n", rtinfo->invflags);
>  		return -EINVAL;
>  	}
>  	if (rtinfo->addrnr > IP6T_RT_HOPS) {
> -		pr_debug("too many addresses specified\n");
> +		pr_info_ratelimited("too many addresses specified\n");
>  		return -EINVAL;
>  	}
>  	if ((rtinfo->flags & (IP6T_RT_RES | IP6T_RT_FST_MASK)) &&
>  	    (!(rtinfo->flags & IP6T_RT_TYP) ||
>  	     (rtinfo->rt_type != 0) ||
>  	     (rtinfo->invflags & IP6T_RT_INV_TYP))) {
> -		pr_debug("`--rt-type 0' required before `--rt-0-*'");
> +		pr_info_ratelimited("`--rt-type 0' required before `--rt-0-*'");
>  		return -EINVAL;
>  	}

[Severity: Low]
Same question here in rt_mt6_check(): the two messages above it end in
'\n' but this one does not, so a rule with IP6T_RT_RES or IP6T_RT_FST_MASK
and rt_type != 0 leaves the record un-terminated.

> diff --git a/net/netfilter/xt_IDLETIMER.c b/net/netfilter/xt_IDLETIMER.c
> index bfcf2d44e93db..15b698ea98aa6 100644
> --- a/net/netfilter/xt_IDLETIMER.c
> +++ b/net/netfilter/xt_IDLETIMER.c

[ ... ]

> @@ -171,7 +168,7 @@ static int idletimer_tg_create(struct idletimer_tg_info *info)
>  
>  	ret = sysfs_create_file(idletimer_tg_kobj, &info->timer->attr.attr);
>  	if (ret < 0) {
> -		pr_debug("couldn't add file to sysfs");
> +		pr_info_ratelimited("couldn't add file to sysfs");
>  		goto out_free_attr;
>  	}

[Severity: Low]
Would it be worth adding the '\n' here in idletimer_tg_create() too?  This
one is reached from idletimer_tg_checkentry() when sysfs_create_file()
fails, and the other strings converted in this file
("timeout value is zero\n", "label is empty or not nul-terminated\n") are
newline terminated.

> @@ -220,7 +217,7 @@ static int idletimer_tg_create_v1(struct idletimer_tg_info_v1 *info)
>  
>  	ret = sysfs_create_file(idletimer_tg_kobj, &info->timer->attr.attr);
>  	if (ret < 0) {
> -		pr_debug("couldn't add file to sysfs");
> +		pr_info_ratelimited("couldn't add file to sysfs");
>  		goto out_free_attr;
>  	}

[Severity: Low]
And the second copy of the same string, in idletimer_tg_create_v1(),
reached from idletimer_tg_checkentry_v1().

[ ... ]

> @@ -534,7 +490,7 @@ static int __init idletimer_tg_init(void)
>  	idletimer_tg_class = class_create("xt_idletimer");
>  	err = PTR_ERR(idletimer_tg_class);
>  	if (IS_ERR(idletimer_tg_class)) {
> -		pr_debug("couldn't register device class\n");
> +		pr_info_ratelimited("couldn't register device class\n");
>  		goto out;
>  	}
>  
> @@ -542,7 +498,7 @@ static int __init idletimer_tg_init(void)
>  					    MKDEV(0, 0), NULL, "timers");
>  	err = PTR_ERR(idletimer_tg_device);
>  	if (IS_ERR(idletimer_tg_device)) {
> -		pr_debug("couldn't register system device\n");
> +		pr_info_ratelimited("couldn't register system device\n");
>  		goto out_class;
>  	}
>  
> @@ -551,7 +507,7 @@ static int __init idletimer_tg_init(void)
>  	err = xt_register_targets(idletimer_tg, ARRAY_SIZE(idletimer_tg));
>  
>  	if (err < 0) {
> -		pr_debug("couldn't register xt target\n");
> +		pr_info_ratelimited("couldn't register xt target\n");
>  		goto out_dev;
>  	}

[Severity: Low]
The changelog says:

    Still, turn pr_debug() into pr_info_ratelimited() in the .check
    path since this helps provide a hint via dmesg in legacy iptables.

These three conversions are in idletimer_tg_init(), which is __init and not
an xtables checkentry callback, so they fall outside the ".check path"
described above.

Were these intended?  Ratelimiting can never trigger on a one-shot module
init failure, and pr_err() would be the usual level for class_create(),
device_create() and xt_register_targets() failing.

> diff --git a/net/netfilter/xt_NETMAP.c b/net/netfilter/xt_NETMAP.c
> index cb2ee80d84fa8..8c86d9a43373a 100644
> --- a/net/netfilter/xt_NETMAP.c
> +++ b/net/netfilter/xt_NETMAP.c
> @@ -106,11 +106,11 @@ static int netmap_tg4_check(const struct xt_tgchk_param *par)
>  	const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo;
>  
>  	if (!(mr->range[0].flags & NF_NAT_RANGE_MAP_IPS)) {
> -		pr_debug("bad MAP_IPS.\n");
> +		pr_info_ratelimited("bad MAP_IPS.\n");
>  		return -EINVAL;
>  	}
>  	if (mr->rangesize != 1) {
> -		pr_debug("bad rangesize %u.\n", mr->rangesize);
> +		pr_info_ratelimited("bad rangesize %u.\n", mr->rangesize);
>  		return -EINVAL;
>  	}
>  	return nf_ct_netns_get(par->net, par->family);

[Severity: Low]
Do these new messages need a pr_fmt() in this file to be identifiable?

xt_NETMAP.c and xt_REDIRECT.c do not define pr_fmt(), so the fallback in
include/linux/printk.h applies:

	#define pr_fmt(fmt) fmt

while the peer site does define one:

net/netfilter/xt_MASQUERADE.c:
	#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

and prints the byte-identical strings "bad MAP_IPS." and
"bad rangesize %u".  The result is that MASQUERADE reports
"xt_MASQUERADE: bad MAP_IPS." while NETMAP and REDIRECT report a bare
"bad MAP_IPS.", so dmesg no longer says which target rejected the rule.
With pr_debug() the dynamic debug machinery supplied module, file and
function, so this did not matter before.

> diff --git a/net/netfilter/xt_REDIRECT.c b/net/netfilter/xt_REDIRECT.c
> index ff66b56a3f97d..27bc0545c4f74 100644
> --- a/net/netfilter/xt_REDIRECT.c
> +++ b/net/netfilter/xt_REDIRECT.c
> @@ -51,11 +51,11 @@ static int redirect_tg4_check(const struct xt_tgchk_param *par)
>  	const struct nf_nat_ipv4_multi_range_compat *mr = par->targinfo;
>  
>  	if (mr->range[0].flags & NF_NAT_RANGE_MAP_IPS) {
> -		pr_debug("bad MAP_IPS.\n");
> +		pr_info_ratelimited("bad MAP_IPS.\n");
>  		return -EINVAL;
>  	}
>  	if (mr->rangesize != 1) {
> -		pr_debug("bad rangesize %u.\n", mr->rangesize);
> +		pr_info_ratelimited("bad rangesize %u.\n", mr->rangesize);
>  		return -EINVAL;
>  	}
>  	return nf_ct_netns_get(par->net, par->family);

[Severity: Low]
Same for redirect_tg4_check() in xt_REDIRECT.c, which also has no pr_fmt()
definition.

[PATCH net 06/10] netfilter: xt_cgroup: use pr_info_ratelimited()

[Pablo Neira Ayuso]

This seems to be the last xtables extension not using the ratelimited
version, update it.

Fixes: c38c4597e4bf ("netfilter: implement xt_cgroup cgroup2 path match")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/xt_cgroup.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/xt_cgroup.c b/net/netfilter/xt_cgroup.c
index 43d2ae2be628..ac1b7e71cd7f 100644
--- a/net/netfilter/xt_cgroup.c
+++ b/net/netfilter/xt_cgroup.c
@@ -33,7 +33,7 @@ static int cgroup_mt_check_v0(const struct xt_mtchk_param *par)
 		return -EINVAL;
 
 	if (!IS_ENABLED(CONFIG_CGROUP_NET_CLASSID)) {
-		pr_info(NET_CLS_CLASSID_INVALID_MSG);
+		pr_info_ratelimited(NET_CLS_CLASSID_INVALID_MSG);
 		return -EINVAL;
 	}
 
@@ -49,7 +49,7 @@ static int cgroup_mt_check_v1(const struct xt_mtchk_param *par)
 		return -EINVAL;
 
 	if (!info->has_path && !info->has_classid) {
-		pr_info("xt_cgroup: no path or classid specified\n");
+		pr_info_ratelimited("xt_cgroup: no path or classid specified\n");
 		return -EINVAL;
 	}
 
@@ -59,7 +59,7 @@ static int cgroup_mt_check_v1(const struct xt_mtchk_param *par)
 	}
 
 	if (info->has_classid && !IS_ENABLED(CONFIG_CGROUP_NET_CLASSID)) {
-		pr_info(NET_CLS_CLASSID_INVALID_MSG);
+		pr_info_ratelimited(NET_CLS_CLASSID_INVALID_MSG);
 		return -EINVAL;
 	}
 
@@ -89,7 +89,7 @@ static int cgroup_mt_check_v2(const struct xt_mtchk_param *par)
 		return -EINVAL;
 
 	if (!info->has_path && !info->has_classid) {
-		pr_info("xt_cgroup: no path or classid specified\n");
+		pr_info_ratelimited("xt_cgroup: no path or classid specified\n");
 		return -EINVAL;
 	}
 
@@ -99,7 +99,7 @@ static int cgroup_mt_check_v2(const struct xt_mtchk_param *par)
 	}
 
 	if (info->has_classid && !IS_ENABLED(CONFIG_CGROUP_NET_CLASSID)) {
-		pr_info(NET_CLS_CLASSID_INVALID_MSG);
+		pr_info_ratelimited(NET_CLS_CLASSID_INVALID_MSG);
 		return -EINVAL;
 	}
 
-- 
2.47.3

[PATCH net 07/10] netfilter: nf_tables: skip double clone set expressions on element insert

[Pablo Neira Ayuso]

Both the dynset and newsetelem path clone the existing set expressions
when setting set element expressions if no override expressions are
provided. This results in a double clone, once to clone the template set
expressions then another clone on the new element. Add a flag to
annotate if userspace provides a override expression (ie. expression of
the same type of the set but different configuration), otherwise borrow
the existing expression from the set. Add conditionals to release
expression iif they represent an override. Use this new override_exprs
flag to dump the dynset expression override to userspace.

This simplifies the existing logic and it also fixes a bug with the
connlimit expression which results in a module refcount imbalance
WARNING splat when resorting on the default set expressions.

Fixes: 65038428b2c6 ("netfilter: nf_tables: allow to specify stateful expression in set definition")
Fixes: fca05d4d61e6 ("netfilter: nft_dynset: honor stateful expressions in set definition")
Reported-by: Xingyuan Mo <hdthky0@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h |  2 --
 net/netfilter/nf_tables_api.c     | 56 ++++++++++---------------------
 net/netfilter/nft_dynset.c        | 25 ++++++++------
 3 files changed, 33 insertions(+), 50 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 238f6ecb90e9..9d597482363d 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -870,8 +870,6 @@ struct nft_elem_priv *nft_set_elem_init(const struct nft_set *set,
 					const u32 *key, const u32 *key_end,
 					const u32 *data,
 					u64 timeout, u64 expiration, gfp_t gfp);
-int nft_set_elem_expr_clone(const struct nft_ctx *ctx, struct nft_set *set,
-			    struct nft_expr *expr_array[]);
 void nft_set_elem_expr_destroy(const struct nft_ctx *ctx,
 			       struct nft_set_elem_expr *elem_expr);
 void nft_set_elem_destroy(const struct nft_set *set,
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 71f4227d7ac7..20c562174b27 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -6921,39 +6921,11 @@ static void nft_trans_elems_destroy(const struct nft_ctx *ctx,
 		nf_tables_set_elem_destroy(ctx, te->set, te->elems[i].priv);
 }
 
-int nft_set_elem_expr_clone(const struct nft_ctx *ctx, struct nft_set *set,
-			    struct nft_expr *expr_array[])
-{
-	struct nft_expr *expr;
-	int err, i, k;
-
-	for (i = 0; i < set->num_exprs; i++) {
-		expr = kzalloc(set->exprs[i]->ops->size, GFP_KERNEL_ACCOUNT);
-		if (!expr)
-			goto err_expr;
-
-		err = nft_expr_clone(expr, set->exprs[i], GFP_KERNEL_ACCOUNT);
-		if (err < 0) {
-			kfree(expr);
-			goto err_expr;
-		}
-		expr_array[i] = expr;
-	}
-
-	return 0;
-
-err_expr:
-	for (k = i - 1; k >= 0; k--)
-		nft_expr_destroy(ctx, expr_array[k]);
-
-	return -ENOMEM;
-}
-
 static int nft_set_elem_expr_setup(struct nft_ctx *ctx,
 				   const struct nft_set_ext_tmpl *tmpl,
 				   const struct nft_set_ext *ext,
 				   struct nft_expr *expr_array[],
-				   u32 num_exprs)
+				   u32 num_exprs, bool override_exprs)
 {
 	struct nft_set_elem_expr *elem_expr = nft_set_ext_expr(ext);
 	u32 len = sizeof(struct nft_set_elem_expr);
@@ -6976,7 +6948,8 @@ static int nft_set_elem_expr_setup(struct nft_ctx *ctx,
 			goto err_elem_expr_setup;
 
 		elem_expr->size += expr_array[i]->ops->size;
-		nft_expr_destroy(ctx, expr_array[i]);
+		if (override_exprs)
+			nft_expr_destroy(ctx, expr_array[i]);
 		expr_array[i] = NULL;
 	}
 
@@ -6984,7 +6957,9 @@ static int nft_set_elem_expr_setup(struct nft_ctx *ctx,
 
 err_elem_expr_setup:
 	for (; i < num_exprs; i++) {
-		nft_expr_destroy(ctx, expr_array[i]);
+		if (override_exprs)
+			nft_expr_destroy(ctx, expr_array[i]);
+
 		expr_array[i] = NULL;
 	}
 
@@ -7280,6 +7255,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 	struct nft_set_binding *binding;
 	struct nft_elem_priv *elem_priv;
 	struct nft_object *obj = NULL;
+	bool override_exprs = false;
 	struct nft_userdata *udata;
 	struct nft_data_desc desc;
 	enum nft_registers dreg;
@@ -7385,6 +7361,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 
 		expr_array[0] = expr;
 		num_exprs = 1;
+		override_exprs = true;
 
 		if (set->num_exprs && set->exprs[0]->ops != expr->ops) {
 			err = -EOPNOTSUPP;
@@ -7413,6 +7390,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 			}
 			expr_array[i] = expr;
 			num_exprs++;
+			override_exprs = true;
 
 			if (set->num_exprs && expr->ops != set->exprs[i]->ops) {
 				err = -EOPNOTSUPP;
@@ -7426,9 +7404,8 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 		}
 	} else if (set->num_exprs > 0 &&
 		   !(flags & NFT_SET_ELEM_INTERVAL_END)) {
-		err = nft_set_elem_expr_clone(ctx, set, expr_array);
-		if (err < 0)
-			goto err_set_elem_expr_clone;
+		for (i = 0; i < set->num_exprs; i++)
+			expr_array[i] = set->exprs[i];
 
 		num_exprs = set->num_exprs;
 	}
@@ -7567,7 +7544,8 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 		udata->len = ulen - 1;
 		nla_memcpy(&udata->data, nla[NFTA_SET_ELEM_USERDATA], ulen);
 	}
-	err = nft_set_elem_expr_setup(ctx, &tmpl, ext, expr_array, num_exprs);
+	err = nft_set_elem_expr_setup(ctx, &tmpl, ext, expr_array, num_exprs,
+				      override_exprs);
 	if (err < 0)
 		goto err_elem_free;
 
@@ -7675,9 +7653,11 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 err_parse_key:
 	nft_data_release(&elem.key.val, NFT_DATA_VALUE);
 err_set_elem_expr:
-	for (i = 0; i < num_exprs && expr_array[i]; i++)
-		nft_expr_destroy(ctx, expr_array[i]);
-err_set_elem_expr_clone:
+	if (override_exprs) {
+		for (i = 0; i < num_exprs && expr_array[i]; i++)
+			nft_expr_destroy(ctx, expr_array[i]);
+	}
+
 	return err;
 }
 
diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c
index ee9d3e7b1ecf..fa4da694e92d 100644
--- a/net/netfilter/nft_dynset.c
+++ b/net/netfilter/nft_dynset.c
@@ -19,7 +19,8 @@ struct nft_dynset {
 	u8				sreg_key;
 	u8				sreg_data;
 	bool				invert;
-	bool				expr;
+	bool				expr:1,
+					override_exprs:1;
 	u8				num_exprs;
 	u64				timeout;
 	struct nft_expr			*expr_array[NFT_SET_EXPR_MAX];
@@ -257,6 +258,7 @@ static int nft_dynset_init(const struct nft_ctx *ctx,
 
 		priv->num_exprs++;
 		priv->expr_array[0] = dynset_expr;
+		priv->override_exprs = true;
 
 		if (set->num_exprs > 1 ||
 		    (set->num_exprs == 1 &&
@@ -289,6 +291,7 @@ static int nft_dynset_init(const struct nft_ctx *ctx,
 			}
 			priv->expr_array[i] = dynset_expr;
 			priv->num_exprs++;
+			priv->override_exprs = true;
 
 			if (set->num_exprs) {
 				if (i >= set->num_exprs) {
@@ -307,9 +310,8 @@ static int nft_dynset_init(const struct nft_ctx *ctx,
 			goto err_expr_free;
 		}
 	} else if (set->num_exprs > 0) {
-		err = nft_set_elem_expr_clone(ctx, set, priv->expr_array);
-		if (err < 0)
-			return err;
+		for (i = 0; i < set->num_exprs; i++)
+			priv->expr_array[i] = set->exprs[i];
 
 		priv->num_exprs = set->num_exprs;
 	}
@@ -339,8 +341,10 @@ static int nft_dynset_init(const struct nft_ctx *ctx,
 	return 0;
 
 err_expr_free:
-	for (i = 0; i < priv->num_exprs; i++)
-		nft_expr_destroy(ctx, priv->expr_array[i]);
+	if (priv->override_exprs) {
+		for (i = 0; i < priv->num_exprs; i++)
+			nft_expr_destroy(ctx, priv->expr_array[i]);
+	}
 	return err;
 }
 
@@ -367,9 +371,10 @@ static void nft_dynset_destroy(const struct nft_ctx *ctx,
 	struct nft_dynset *priv = nft_expr_priv(expr);
 	int i;
 
-	for (i = 0; i < priv->num_exprs; i++)
-		nft_expr_destroy(ctx, priv->expr_array[i]);
-
+	if (priv->override_exprs) {
+		for (i = 0; i < priv->num_exprs; i++)
+			nft_expr_destroy(ctx, priv->expr_array[i]);
+	}
 	nf_tables_destroy_set(ctx, priv->set);
 }
 
@@ -393,7 +398,7 @@ static int nft_dynset_dump(struct sk_buff *skb,
 			 nf_jiffies64_to_msecs(priv->timeout),
 			 NFTA_DYNSET_PAD))
 		goto nla_put_failure;
-	if (priv->set->num_exprs == 0) {
+	if (priv->set->num_exprs == 0 || priv->override_exprs) {
 		if (priv->num_exprs == 1) {
 			if (nft_expr_dump(skb, NFTA_DYNSET_EXPR,
 					  priv->expr_array[0], reset))
-- 
2.47.3

Re: [PATCH net 07/10] netfilter: nf_tables: skip double clone set expressions on element insert

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
netfilter: nf_tables: skip double clone set expressions on element insert

This adds an override_exprs flag so that the dynset and newsetelem paths
borrow the set's expressions instead of cloning them when userspace does
not supply an override, and releases the expression array only when it
really is an override.  The same flag is now used to decide whether the
dynset expression override is dumped back to userspace.

> diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c
> index ee9d3e7b1ecf1..fa4da694e92d8 100644
> --- a/net/netfilter/nft_dynset.c
> +++ b/net/netfilter/nft_dynset.c
> @@ -19,7 +19,8 @@ struct nft_dynset {
>  	u8				sreg_key;
>  	u8				sreg_data;
>  	bool				invert;
> -	bool				expr;
> +	bool				expr:1,
> +					override_exprs:1;
>  	u8				num_exprs;
>  	u64				timeout;
>  	struct nft_expr			*expr_array[NFT_SET_EXPR_MAX];

[ ... ]

> @@ -393,7 +398,7 @@ static int nft_dynset_dump(struct sk_buff *skb,
>  			 nf_jiffies64_to_msecs(priv->timeout),
>  			 NFTA_DYNSET_PAD))
>  		goto nla_put_failure;
> -	if (priv->set->num_exprs == 0) {
> +	if (priv->set->num_exprs == 0 || priv->override_exprs) {
>  		if (priv->num_exprs == 1) {
>  			if (nft_expr_dump(skb, NFTA_DYNSET_EXPR,
>  					  priv->expr_array[0], reset))

[Severity: Medium]
Can the message emitted by nft_dynset_dump() after this change be rejected
by nft_dynset_init() when it is replayed?

With the widened condition, a dynset attached to a set that owns
expressions (set->num_exprs > 0) now dumps its override.  For
priv->num_exprs > 1 that override is emitted as an NFTA_DYNSET_EXPRESSIONS
nest:

net/netfilter/nft_dynset.c:nft_dynset_dump() {
	...
	u32 flags = priv->invert ? NFT_DYNSET_F_INV : 0;
	...
	} else if (priv->num_exprs > 1) {
		struct nlattr *nest;

		nest = nla_nest_start_noflag(skb, NFTA_DYNSET_EXPRESSIONS);
	...
	if (nla_put_be32(skb, NFTA_DYNSET_FLAGS, htonl(flags)))
	...
}

The dumped NFTA_DYNSET_FLAGS is computed only from priv->invert, so
NFT_DYNSET_F_EXPR is never set, even though priv->expr records that
userspace passed it at creation time.  On the parse side the
NFTA_DYNSET_EXPRESSIONS branch requires that flag:

net/netfilter/nft_dynset.c:nft_dynset_init() {
	...
	} else if (tb[NFTA_DYNSET_EXPRESSIONS]) {
		...
		if (!priv->expr)
			return -EINVAL;
	...
}

So a rule with two override expressions on an NFT_SET_EVAL set that also
has two set expressions (for example counter plus quota) appears to be
dumped in a form that the kernel's own parser refuses with -EINVAL when the
netlink message is sent back (raw netlink echo, or libnftnl parse-then-build
restore paths).  Before this patch that configuration emitted no expression
attributes at all, so the replay was accepted and silently fell back to the
set's expressions.

The missing NFT_DYNSET_F_EXPR in the dump path predates this patch (it is
already reachable for priv->set->num_exprs == 0 with more than one
expression), but the new condition extends the asymmetry to
override-on-eval-set rules that used to round-trip.  Should
nft_dynset_dump() also report NFT_DYNSET_F_EXPR when priv->expr is set?

[PATCH net 08/10] netfilter: nf_tables: fix device name and prefix match in hook lookup

[Pablo Neira Ayuso]

From: Fernando Fernandez Mancera <fmancera@suse.de>

Currently, a netdev chain or flowtable hooked to a device prefix can be
unintentionally deleted or updated by a control-plane request targeting
an exact device name or even a shorter one due to the usage of min() to
calculate the length to match.

Fix this by making sure an exact device match never matches a prefix and
that both the target and the candidate have the same length.

Reported-by: Wei Fang <void0red@gmail.com>
Closes: https://lore.kernel.org/netfilter-devel/CANE+tVrDeNCHQVmsqkV2ozeBqyE3GtRDMhZgsg1bhw10yGNTRQ@mail.gmail.com/
Fixes: 6d07a289504a ("netfilter: nf_tables: Support wildcard netdev hook specs")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 20c562174b27..d160f6ea0dc2 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1978,7 +1978,7 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats)
 	return -ENOSPC;
 }
 
-static bool hook_is_prefix(struct nft_hook *hook)
+static bool hook_is_prefix(const struct nft_hook *hook)
 {
 	return strlen(hook->ifname) >= hook->ifnamelen;
 }
@@ -2445,8 +2445,11 @@ static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
 	struct nft_hook *hook;
 
 	list_for_each_entry(hook, hook_list, list) {
-		if (!strncmp(hook->ifname, this->ifname,
-			     min(hook->ifnamelen, this->ifnamelen))) {
+		if (hook_is_prefix(hook) != hook_is_prefix(this))
+			continue;
+		if (hook->ifnamelen != this->ifnamelen)
+			continue;
+		if (!strncmp(hook->ifname, this->ifname, hook->ifnamelen)) {
 			if (hook->flags & NFT_HOOK_REMOVE)
 				continue;
 
-- 
2.47.3

Re: [PATCH net 08/10] netfilter: nf_tables: fix device name and prefix match in hook lookup

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
netfilter: nf_tables: fix device name and prefix match in hook lookup

This changes nft_hook_list_find() so a hook spec only matches another
spec of the same kind (prefix vs exact name) and the same length,
replacing the previous strncmp() over min(hook->ifnamelen,
this->ifnamelen).  hook_is_prefix() also gains a const argument.

Closes: https://lore.kernel.org/netfilter-devel/CANE+tVrDeNCHQVmsqkV2ozeBqyE3GtRDMhZgsg1bhw10yGNTRQ@mail.gmail.com/

> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 20c562174b278..d160f6ea0dc29 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c

[ ... ]

> @@ -2445,8 +2445,11 @@ static struct nft_hook *nft_hook_list_find(struct list_head *hook_list,
>  	struct nft_hook *hook;
>  
>  	list_for_each_entry(hook, hook_list, list) {
> -		if (!strncmp(hook->ifname, this->ifname,
> -			     min(hook->ifnamelen, this->ifnamelen))) {
> +		if (hook_is_prefix(hook) != hook_is_prefix(this))
> +			continue;
> +		if (hook->ifnamelen != this->ifnamelen)
> +			continue;
> +		if (!strncmp(hook->ifname, this->ifname, hook->ifnamelen)) {
>  			if (hook->flags & NFT_HOOK_REMOVE)
>  				continue;
>  

[Severity: Critical]
nft_hook_list_find() has two callers with different expectations, and this
turns it into a pure identity comparator for both.  The delete/get paths
(nf_tables_delchain_hook(), nft_flowtable_del_hook()) want identity, but
the add paths use it as an overlap rejector:

nf_tables_parse_netdev_hooks()
	if (nft_hook_list_find(hook_list, hook)) {
		NL_SET_BAD_ATTR(extack, tmp);
		nft_netdev_hook_free(hook);
		err = -EEXIST;
		goto err_hook;
	}

and the same in nf_tables_updchain(), nft_register_flowtable_net_hooks()
and nft_flowtable_update().

The Fixes-referenced commit 6d07a289504a states the min() compare was
deliberate: "When checking for duplicate hooks, compare the shortest
prefix so a device may never match more than a single hook spec."  Does
this change intentionally drop that invariant?

Concretely, is this still rejected after the patch?

  nft add chain netdev t c \
    '{ type filter hook ingress devices = { "eth*", "eth0" } ; }'

Before, prefix "eth" (ifnamelen 3) and exact "eth0" (ifnamelen 5) matched
via strncmp(..., min(3, 5)) and gave -EEXIST.  Now
hook_is_prefix(hook) != hook_is_prefix(this) short-circuits, so both hooks
end up in the same hook_list.  Prefix "eth" versus prefix "eth0" is
likewise separated by the ifnamelen compare.

nft_netdev_hook_alloc() then builds one nf_hook_ops per matching netdev
per hook:

	for_each_netdev(net, dev) {
		if (strncmp(dev->name, hook->ifname, hook->ifnamelen))
			continue;
		...
		ops->dev = dev;
		list_add_tail(&ops->list, &hook->ops_list);
	}

so two distinct nft_hook objects each own an ops with ops->dev == eth0,
and nft_netdev_register_hooks() registers both.  nf_hook_entries_grow()
only rejects duplicates for NF_HOOK_OP_BPF, so both land in
eth0->nf_hooks_ingress and the base chain is evaluated twice per packet.
For flowtables the cross-flowtable -EEXIST guard in
nft_register_flowtable_net_hooks() is bypassed the same way, so the same
device gets two FLOW_BLOCK_BIND calls.

Can this also leave a dangling struct net_device pointer?  Both notifier
loops service only the first hook that owns an ops for the device, because
of the unconditional break after the switch:

net/netfilter/nft_chain_filter.c:nft_netdev_event() {
	list_for_each_entry(hook, &basechain->hook_list, list) {
		ops = nft_hook_find_ops(hook, dev);
		...
		switch (event) {
		case NETDEV_UNREGISTER:
			...
			nf_unregister_net_hook(dev_net(dev), ops);
			list_del_rcu(&ops->list);
			kfree_rcu(ops, rcu);
			break;
		...
		}
		break;
	}
	return 0;
}

nft_flowtable_event() in nf_tables_api.c has the same trailing break.  On
ip link del eth0 only the first hook's ops is unregistered and freed; the
second hook keeps a registered nf_hook_ops whose ops->dev points at the
device that is then freed, since nf_hook_ops holds no device reference.

A later teardown (nft delete chain, nft delete table, dormant toggle,
netns exit) walks every ops:

net/netfilter/nf_tables_api.c:nft_netdev_unregister_hooks() {
	list_for_each_entry_safe(hook, next, hook_list, list) {
		if (!(table->flags & NFT_TABLE_F_DORMANT)) {
			list_for_each_entry(ops, &hook->ops_list, list)
				nf_unregister_net_hook(net, ops);
		}
	...
}

and __nf_unregister_net_hook() reaches nf_hook_entry_head(net, pf,
reg->hooknum, reg->dev), which does dev_net(dev) and returns
&dev->nf_hooks_ingress, i.e. a read and then a write through the stale
pointer.  For flowtables, nft_unregister_flowtable_ops() additionally
calls ->setup(..., ops->dev, FLOW_BLOCK_UNBIND) on the freed device.  The
orphaned nf_hook_entries blob is also leaked.

On NETDEV_REGISTER the same break means only one of two covering hooks
gains an ops, so the other stays permanently unhooked.

Would it work to keep the strict identity match only for the
delete/update lookups and retain the prefix-overlap semantics for the
add/-EEXIST paths, e.g. as two separate helpers?  Alternatively, should
the unconditional break in nft_netdev_event() and nft_flowtable_event()
be dropped so every matching hook is serviced?

The commit message describes only the delete/update lookup semantics; the
fact that configurations previously rejected with -EEXIST are now
accepted is not mentioned.  Could that be spelled out?

[PATCH net 09/10] netfilter: nf_tables: set on dead bit when performing early element removal

[Pablo Neira Ayuso]

.commit call for sets is skipped if set->dead flag is set on, but this
flag is set on later in the commit path.

This also reintroduces the bug fixed in commit 7315dc1e122c8
("netfilter: nf_tables: skip set commit for deleted/destroyed sets").

Fixes: 1e3b9e1c77fe ("netfilter: nf_tables: call set ops .commit when building new ruleset blob")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index d160f6ea0dc2..4f3be3b10f2f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -10877,6 +10877,10 @@ static void nft_set_commit_update(struct nft_ctx *ctx,
 		nft_ctx_update(ctx, trans);
 
 		switch (trans->msg_type) {
+		case NFT_MSG_DELSET:
+		case NFT_MSG_DESTROYSET:
+			nft_trans_set(trans)->dead = 1;
+			break;
 		case NFT_MSG_DELSETELEM:
 			te = nft_trans_container_elem(trans);
 			if (!te->set->ops->commit)
-- 
2.47.3

[PATCH net 10/10] netfilter: nf_tables: remove leftover set_update_list

[Pablo Neira Ayuso]

This list has been moved to per-netns, remove onstack list which is not
used anymore.

Fixes: b343ededb3f9 netfilter: nf_tables: move set_update_list to nftables per-netns
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 4f3be3b10f2f..5cee9e46cca4 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -11298,7 +11298,6 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 {
 	struct nftables_pernet *nft_net = nft_pernet(net);
 	struct nft_trans *trans, *next;
-	LIST_HEAD(set_update_list);
 	struct nft_trans_elem *te;
 	struct nft_ctx ctx = {
 		.net = net,
-- 
2.47.3