Skip to contentSkip to conversation

netfilter: load nf_log_syslog on enabling nf_conntrack_log_invalid

PATCH In review Lance Yang 4 participants 7 messages May 26, 2025 – Jul 24, 2025

Subsystems (3)
Networking Stable branch netfilter
lore
Share & tools
lore

Lance Yang

author
#0 lore raw

PATCH v2 1/1 netfilter: load nf_log_syslog on enabling nf_conntrack_log_invalid

From: Lance Yang <[email protected]>

When no logger is registered, nf_conntrack_log_invalid fails to log invalid packets, leaving users unaware of actual invalid traffic. Improve this by loading nf_log_syslog, similar to how 'iptables -I FORWARD 1 -m conntrack --ctstate INVALID -j LOG' triggers it.

Suggested-by Florian Westphal <[email protected]>
Signed-off-by Zi Li <[email protected]>
Signed-off-by Lance Yang <[email protected]>

---
v1 -> v2:
- Add a new, simpler helper (per Florian)
- Load the module only when no logger is registered (per Florian)
- https://lore.kernel.org/all/[email protected]/

 include/net/netfilter/nf_log.h          |  3 +++
 net/netfilter/nf_conntrack_standalone.c | 26 +++++++++++++++++++++++-
 net/netfilter/nf_log.c                  | 27 +++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 1 deletion(-)
include/net/netfilter/nf_log.h +3 -0
index e55eedc84ed7..00506792a06d 100644
@@ -59,6 +59,9 @@ extern int sysctl_nf_log_all_netns;
 int nf_log_register(u_int8_t pf, struct nf_logger *logger);
 void nf_log_unregister(struct nf_logger *logger);
 
+/* Check if any logger is registered for a given protocol family. */
+bool nf_log_is_registered(u_int8_t pf);
+
 int nf_log_set(struct net *net, u_int8_t pf, const struct nf_logger *logger);
 void nf_log_unset(struct net *net, const struct nf_logger *logger);
net/netfilter/nf_conntrack_standalone.c +25 -1
index 2f666751c7e7..cdc27424f84a 100644
@@ -14,6 +14,7 @@
 #include <linux/sysctl.h>
 #endif
 
+#include <net/netfilter/nf_log.h>
 #include <net/netfilter/nf_conntrack.h>
 #include <net/netfilter/nf_conntrack_core.h>
 #include <net/netfilter/nf_conntrack_l4proto.h>

@@ -543,6 +544,29 @@ nf_conntrack_hash_sysctl(const struct ctl_table *table, int write,
 	return ret;
 }
 
+static int
+nf_conntrack_log_invalid_sysctl(const struct ctl_table *table, int write,
+				void *buffer, size_t *lenp, loff_t *ppos)
+{
+	int ret, i;
+
+	ret = proc_dou8vec_minmax(table, write, buffer, lenp, ppos);
+	if (ret < 0 || !write)
+		return ret;
+
+	if (*(u8 *)table->data == 0)
+		return ret;
+
+	/* Load nf_log_syslog only if no logger is currently registered */
+	for (i = 0; i < NFPROTO_NUMPROTO; i++) {
+		if (nf_log_is_registered(i))
+			return ret;
+	}
+	request_module("%s", "nf_log_syslog");
+
+	return ret;
+}
+
 static struct ctl_table_header *nf_ct_netfilter_header;
 
 enum nf_ct_sysctl_index {

@@ -649,7 +673,7 @@ static struct ctl_table nf_ct_sysctl_table[] = {
 		.data		= &init_net.ct.sysctl_log_invalid,
 		.maxlen		= sizeof(u8),
 		.mode		= 0644,
-		.proc_handler	= proc_dou8vec_minmax,
+		.proc_handler	= nf_conntrack_log_invalid_sysctl,
 	},
 	[NF_SYSCTL_CT_EXPECT_MAX] = {
 		.procname	= "nf_conntrack_expect_max",
net/netfilter/nf_log.c +27 -0
index 6dd0de33eebd..c7dd5019a89d 100644
@@ -125,6 +125,33 @@ void nf_log_unregister(struct nf_logger *logger)
 }
 EXPORT_SYMBOL(nf_log_unregister);
 
+/**
+ * nf_log_is_registered - Check if any logger is registered for a given
+ * protocol family.
+ *
+ * @pf: Protocol family
+ *
+ * Returns: true if at least one logger is active for @pf, false otherwise.
+ */
+bool nf_log_is_registered(u_int8_t pf)
+{
+	int i;
+
+	/* Out of bounds. */
+	if (pf >= NFPROTO_NUMPROTO) {
+		WARN_ON_ONCE(1);
+		return false;
+	}
+
+	for (i = 0; i < NF_LOG_TYPE_MAX; i++) {
+		if (rcu_access_pointer(loggers[pf][i]))
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(nf_log_is_registered);
+
 int nf_log_bind_pf(struct net *net, u_int8_t pf,
 		   const struct nf_logger *logger)
 {
Signature · 1 line
2.49.0
maintainer Acked-by on v2 1/1
#1 lore raw
Quoted 6 lines · Lance Yang

From: Lance Yang <[email protected]>

When no logger is registered, nf_conntrack_log_invalid fails to log invalid packets, leaving users unaware of actual invalid traffic. Improve this by loading nf_log_syslog, similar to how 'iptables -I FORWARD 1 -m conntrack --ctstate INVALID -j LOG' triggers it.

Acked-by Florian Westphal <[email protected]>
#2 lore raw

Thanks for taking the time to review!

Quoted 9 lines · Florian Westphal
Quoted 6 lines · Lance Yang

From: Lance Yang <[email protected]>

When no logger is registered, nf_conntrack_log_invalid fails to log invalid packets, leaving users unaware of actual invalid traffic. Improve this by loading nf_log_syslog, similar to how 'iptables -I FORWARD 1 -m conntrack --ctstate INVALID -j LOG' triggers it.

Acked-by Florian Westphal <[email protected]>

Hmm... should this patch be backported to stable kernels? Without it, nf_conntrack_log_invalid won't log invalid packets when no logger is registered, causing unnecessary debugging effort ;)

Back then, I actually thought my machine wasn't seeing any invalid packets... turns out they just weren't logged in dmesg :(

Thanks,
Lance

#3 lore raw
Cc stable
Quoted 23 lines · Lance Yang

Thanks for taking the time to review!

Quoted 9 lines · Florian Westphal
Quoted 6 lines · Lance Yang

From: Lance Yang <[email protected]>

When no logger is registered, nf_conntrack_log_invalid fails to log invalid packets, leaving users unaware of actual invalid traffic. Improve this by loading nf_log_syslog, similar to how 'iptables -I FORWARD 1 -m conntrack --ctstate INVALID -j LOG' triggers it.

Acked-by Florian Westphal <[email protected]>

Hmm... should this patch be backported to stable kernels? Without it, nf_conntrack_log_invalid won't log invalid packets when no logger is registered, causing unnecessary debugging effort ;)

Back then, I actually thought my machine wasn't seeing any invalid packets... turns out they just weren't logged in dmesg :(

Thanks,
Lance

#4 lore raw
In reply to Florian Westphal
Quoted 9 lines · Florian Westphal
Quoted 6 lines · Lance Yang

From: Lance Yang <[email protected]>

When no logger is registered, nf_conntrack_log_invalid fails to log invalid packets, leaving users unaware of actual invalid traffic. Improve this by loading nf_log_syslog, similar to how 'iptables -I FORWARD 1 -m conntrack --ctstate INVALID -j LOG' triggers it.

Acked-by Florian Westphal <[email protected]>

A gentle reminder for this patch, as the merge window is approaching ;) Please let me know if any changes are needed.

Thanks,
Lance

maintainer
#5 lore raw
In reply to Lance Yang

Hi,

Quoted 18 lines · Lance Yang
net/netfilter/nf_conntrack_standalone.c +3 -0
index 2f666751c7e7..cdc27424f84a 100644
@@ -14,6 +14,7 @@
 #include <linux/sysctl.h>
 #endif
 
+#include <net/netfilter/nf_log.h>
 #include <net/netfilter/nf_conntrack.h>
 #include <net/netfilter/nf_conntrack_core.h>
 #include <net/netfilter/nf_conntrack_l4proto.h>

@@ -543,6 +544,29 @@ nf_conntrack_hash_sysctl(const struct ctl_table *table, int write,
 	return ret;
 }
 
+static int
+nf_conntrack_log_invalid_sysctl(const struct ctl_table *table, int write,

net/netfilter/nf_conntrack_standalone.c nf_conntrack_hash_sysctl(const struct ctl_table *table, int write, · line 558

Quoted 10 lines
net/netfilter/nf_conntrack_standalone.c +10 -0
+				void *buffer, size_t *lenp, loff_t *ppos)
+{
+	int ret, i;
+
+	ret = proc_dou8vec_minmax(table, write, buffer, lenp, ppos);
+	if (ret < 0 || !write)
+		return ret;
+
+	if (*(u8 *)table->data == 0)
+		return ret;

What is this table->data check for? I don't find any similar idiom like this in the existing proc_dou8vec_minmax() callers.

Quoted 34 lines
net/netfilter/nf_conntrack_standalone.c +12 -1
+
+	/* Load nf_log_syslog only if no logger is currently registered */
+	for (i = 0; i < NFPROTO_NUMPROTO; i++) {
+		if (nf_log_is_registered(i))
+			return ret;
+	}
+	request_module("%s", "nf_log_syslog");
+
+	return ret;
+}
+
 static struct ctl_table_header *nf_ct_netfilter_header;
 
 enum nf_ct_sysctl_index {

@@ -649,7 +673,7 @@ static struct ctl_table nf_ct_sysctl_table[] = {
 		.data		= &init_net.ct.sysctl_log_invalid,
 		.maxlen		= sizeof(u8),
 		.mode		= 0644,
-		.proc_handler	= proc_dou8vec_minmax,
+		.proc_handler	= nf_conntrack_log_invalid_sysctl,
 	},
 	[NF_SYSCTL_CT_EXPECT_MAX] = {
 		.procname	= "nf_conntrack_expect_max",
net/netfilter/nf_log.c +3 -0
index 6dd0de33eebd..c7dd5019a89d 100644
@@ -125,6 +125,33 @@ void nf_log_unregister(struct nf_logger *logger)
 }
 EXPORT_SYMBOL(nf_log_unregister);
 
+/**
+ * nf_log_is_registered - Check if any logger is registered for a given
+ * protocol family.

net/netfilter/nf_log.c void nf_log_unregister(struct nf_logger *logger) · line 140

Quoted 10 lines
net/netfilter/nf_log.c +10 -0
+ *
+ * @pf: Protocol family
+ *
+ * Returns: true if at least one logger is active for @pf, false otherwise.
+ */
+bool nf_log_is_registered(u_int8_t pf)
+{
+	int i;
+
+	/* Out of bounds. */

No need for this comment, please remove it.

Quoted 4 lines
net/netfilter/nf_log.c +4 -0
+	if (pf >= NFPROTO_NUMPROTO) {
+		WARN_ON_ONCE(1);
+		return false;
+	}
#6 lore raw
Quoted 19 lines · Pablo Neira Ayuso

Hi,

Quoted 16 lines · Lance Yang
net/netfilter/nf_conntrack_standalone.c +1 -0
index 2f666751c7e7..cdc27424f84a 100644
@@ -14,6 +14,7 @@
  #include <linux/sysctl.h>
  #endif
 
+#include <net/netfilter/nf_log.h>
  #include <net/netfilter/nf_conntrack.h>
  #include <net/netfilter/nf_conntrack_core.h>
  #include <net/netfilter/nf_conntrack_l4proto.h>

@@ -543,6 +544,29 @@ nf_conntrack_hash_sysctl(const struct ctl_table *table, int write,
  	return ret;
  }
Quoted 15 lines

net/netfilter/nf_conntrack_standalone.c nf_conntrack_hash_sysctl(const struct ctl_table *table, int write, · line 558

Quoted 12 lines
net/netfilter/nf_conntrack_standalone.c +12 -0
+static int
+nf_conntrack_log_invalid_sysctl(const struct ctl_table *table, int write,
+				void *buffer, size_t *lenp, loff_t *ppos)
+{
+	int ret, i;
+
+	ret = proc_dou8vec_minmax(table, write, buffer, lenp, ppos);
+	if (ret < 0 || !write)
+		return ret;
+
+	if (*(u8 *)table->data == 0)
+		return ret;

What is this table->data check for? I don't find any similar idiom like this in the existing proc_dou8vec_minmax() callers.

My intention was to avoid the unnecessary nf_log_is_registered() calls when a user disables the feature by writing '0' to the sysctl.

But it's not a big deal. I will remove this check in v3 ;)

Quoted 31 lines
Quoted 31 lines
net/netfilter/nf_conntrack_standalone.c +12 -1
+
+	/* Load nf_log_syslog only if no logger is currently registered */
+	for (i = 0; i < NFPROTO_NUMPROTO; i++) {
+		if (nf_log_is_registered(i))
+			return ret;
+	}
+	request_module("%s", "nf_log_syslog");
+
+	return ret;
+}
+
  static struct ctl_table_header *nf_ct_netfilter_header;
 
  enum nf_ct_sysctl_index {

@@ -649,7 +673,7 @@ static struct ctl_table nf_ct_sysctl_table[] = {
  		.data		= &init_net.ct.sysctl_log_invalid,
  		.maxlen		= sizeof(u8),
  		.mode		= 0644,
-		.proc_handler	= proc_dou8vec_minmax,
+		.proc_handler	= nf_conntrack_log_invalid_sysctl,
  	},
  	[NF_SYSCTL_CT_EXPECT_MAX] = {
  		.procname	= "nf_conntrack_expect_max",
net/netfilter/nf_log.c +0 -0
index 6dd0de33eebd..c7dd5019a89d 100644
@@ -125,6 +125,33 @@ void nf_log_unregister(struct nf_logger *logger)
  }
  EXPORT_SYMBOL(nf_log_unregister);
Quoted 15 lines

net/netfilter/nf_log.c void nf_log_unregister(struct nf_logger *logger) · line 140

Quoted 13 lines
net/netfilter/nf_log.c +13 -0
+/**
+ * nf_log_is_registered - Check if any logger is registered for a given
+ * protocol family.
+ *
+ * @pf: Protocol family
+ *
+ * Returns: true if at least one logger is active for @pf, false otherwise.
+ */
+bool nf_log_is_registered(u_int8_t pf)
+{
+	int i;
+
+	/* Out of bounds. */

No need for this comment, please remove it.

OK, will remove it.

Thanks,
Lance

Quoted 5 lines
Quoted 5 lines
net/netfilter/nf_log.c +4 -0
+	if (pf >= NFPROTO_NUMPROTO) {
+		WARN_ON_ONCE(1);
+		return false;
+	}