linux-kernel - Re: [RESEND PATCH 1/1] netfilter: load nf_log_syslog on enabling nf_conntrack_log

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aC2lyYN72raND8S0@calendula>
Date: Wed, 21 May 2025 12:07:05 +0200
From: Pablo Neira Ayuso <pablo@...filter.org>
To: Lance Yang <ioworker0@...il.com>
Cc: kadlec@...filter.org, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
	coreteam@...filter.org, linux-kernel@...r.kernel.org,
	netfilter-devel@...r.kernel.org, Zi Li <zi.li@...ux.dev>,
	Lance Yang <lance.yang@...ux.dev>, fw@...len.de
Subject: Re: [RESEND PATCH 1/1] netfilter: load nf_log_syslog on enabling
 nf_conntrack_log_invalid

Hi,

Cc: Florian Westphal.

On Wed, May 14, 2025 at 01:37:51PM +0800, Lance Yang wrote:
> From: Lance Yang <lance.yang@...ux.dev>
> 
> When nf_log_syslog is not loaded, 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.

I have been beaten by this usability issue in the past, it happens
since conntrack is loaded on demand.

Maybe add an inconditionally soft dependency? This is a oneliner patch.

        MODULE_SOFTDEP("pre: nf_log_syslog");

Florian, do you prefer this patch (on-demand) or a oneliner to load
this module when conntrack gets loaded too?

It is a bit more memory to make it inconditional, but better to expose
to users this soft dependency via lsmod.

Thanks.

> Signed-off-by: Zi Li <zi.li@...ux.dev>
> Signed-off-by: Lance Yang <lance.yang@...ux.dev>
> ---
>  net/netfilter/nf_conntrack_standalone.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index 2f666751c7e7..b4acff01088f 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -543,6 +543,24 @@ 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;
> +
> +	ret = proc_dou8vec_minmax(table, write, buffer, lenp, ppos);
> +	if (ret < 0 || !write)
> +		return ret;
> +
> +	if (*(u8 *)table->data == 0)
> +		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 +667,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",
> -- 
> 2.49.0
>