Re: [PATCH net-next 08/15] netfilter: lwtunnel: expose read-only sysctl nf_hooks_lwtunnel for non init-netns

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch sets the per-netns copy of
  net.netfilter.nf_hooks_lwtunnel to mode 0444 in nf_lwtunnel_net_init()…
- [Low] The nf_hooks_lwtunnel entry in
  Documentation/networking/nf_conntrack-sysctl.rst (lines 212-217)…

--

netfilter: lwtunnel: expose read-only sysctl nf_hooks_lwtunnel for non init-netns

The commit registers the per-netns copy of the nf_hooks_lwtunnel sysctl
with mode 0444 instead of 0644 in every netns except init_net. This stops
unprivileged users from flipping the global nf_hooks_lwtunnel_enabled
static key from inside a user namespace.

> diff --git a/net/netfilter/nf_hooks_lwtunnel.c b/net/netfilter/nf_hooks_lwtunnel.c
> index 40c6fce14a63f..7e2ee8677f1fc 100644
> --- a/net/netfilter/nf_hooks_lwtunnel.c
> +++ b/net/netfilter/nf_hooks_lwtunnel.c
> @@ -68,14 +68,18 @@ static int __net_init nf_lwtunnel_net_init(struct net *net)

[ ... ]

>  	if (!net_eq(net, &init_net)) {
> -		table = kmemdup(nf_lwtunnel_sysctl_table,
> -				sizeof(nf_lwtunnel_sysctl_table),
> -				GFP_KERNEL_ACCOUNT);
> -		if (!table)
> +		dup = kmemdup(nf_lwtunnel_sysctl_table,
> +			      sizeof(nf_lwtunnel_sysctl_table),
> +			      GFP_KERNEL_ACCOUNT);
> +		if (!dup)
>  			goto err_alloc;
> +
> +		dup->mode = 0444;
> +		table = dup;

[Severity: Medium]
Should the SRv6 netfilter selftests be updated along with this change?

With 0444, nobody can write the knob from a child netns, including global
root with CAP_NET_ADMIN. net_ctl_permissions() copies only the owner bits:

net/sysctl_net.c:net_ctl_permissions() {
    ...
	if (ns_capable_noaudit(net->user_ns, CAP_NET_ADMIN)) {
		int mode = (table->mode >> 6) & 7;
		return (mode << 6) | (mode << 3) | mode;
	}
    ...
}

test_perm() in fs/proc/proc_sysctl.c does not let root override this, so
the write returns -EACCES.

Both tools/testing/selftests/net/srv6_end_dx4_netfilter_test.sh and
srv6_end_dx6_netfilter_test.sh write the knob from the rt-N netns in
setup_rt_netfilter():

	ip netns exec ${nsname} sysctl -wq net.netfilter.nf_hooks_lwtunnel=1

The scripts don't check the exit status, so the failure goes unnoticed.
nf_hooks_lwtunnel_enabled then stays off, and decapsulated SRv6 packets
never reach the NF_HOOK(NF_INET_PRE_ROUTING) calls in seg6_local.c.

router_netfilter_tests() only checks ping connectivity:

	setup_rt_netfilter 1
	setup_rt_netfilter 2

	check_and_log_hs_connectivity 1 2 100

Plain SRv6 forwarding works without the hooks, so the "netfilter enabled
in routers" tests would still pass without running the netfilter path at
all.

Userspace sees the same change: a privileged container manager that used
to enable this knob as real root from inside a pod netns will now get
EACCES and has to write it in init_net instead.

The restriction itself matches other global netfilter sysctls, such as
those in nf_conntrack_standalone.c and ipvs. The concern is only that the
dependent selftests stay as they are. As of 3e59905baa0f, both scripts
still write the sysctl from rt-N.

[Severity: Low]
This isn't a bug, but should the nf_hooks_lwtunnel entry in
Documentation/networking/nf_conntrack-sysctl.rst mention the new
restriction?

The entry currently reads:

	nf_hooks_lwtunnel - BOOLEAN
		- 0 - disabled (default)
		- not 0 - enabled

		If this option is enabled, the lightweight tunnel netfilter hooks are
		enabled. This option cannot be disabled once it is enabled.

The same document already has this note for nf_conntrack_buckets:

	This sysctl is only writeable in the initial net namespace.

A similar note here would explain the EACCES that users now get when they
write the knob from a child netns. As of 3e59905baa0f, the entry is
unchanged.

>  	}
>  
>  	hdr = register_net_sysctl_sz(net, "net/netfilter", table,

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261113031251.2437074-2-pablo%40netfilter.org