[PATCH net 0/2] ipv4: preserve devinet inheritance after unparsed writes

[PATCH net 0/2] ipv4: preserve devinet inheritance after unparsed writes

[Yingjie Wang]

devinet_conf_proc() currently marks per-device IPv4 settings as
explicitly configured even when a sysctl write fails or is ignored before
parsing a value. A later conf/default update then skips the device.

Patch 1 updates inheritance state and runs post-set handling only after
successful integer conversion. Patch 2 adds a regression test for a
failed write and the complementary valid same-value case. Before either
case, the test verifies that lo still inherits conf/default, so inherited
init_net state cannot turn the result into a false pass.

On the current net/main base, the failed-write selftest case fails while
the same-value control passes. Both cases pass with this series. A
separate eight-case VM test also covers invalid, blank, zero-byte and
strict nonzero-offset writes plus valid-write controls.

Full x86-64 allyesconfig and allmodconfig builds with W=1 completed after
disabling CONFIG_WERROR; the changed object produced no diagnostics. An
i386 W=1 build of net/ipv4/devinet.o also passed.

checkpatch reports the expected CONST_STRUCT warning for the mutable
call-local ctl_table copy in patch 1 and the generic MAINTAINERS prompt
for the new selftest in patch 2. The copy carries per-call converter
state, and tools/testing/selftests/net is already covered by NETWORKING
[GENERAL].

Yingjie Wang (2):
  ipv4: devinet: update inheritance state only after parsed writes
  selftests: net: test devinet inheritance after failed writes

 net/ipv4/devinet.c                          |  29 +++++-
 tools/testing/selftests/net/Makefile        |   1 +
 tools/testing/selftests/net/devinet_conf.sh | 101 ++++++++++++++++++++
 3 files changed, 128 insertions(+), 3 deletions(-)
 create mode 100755 tools/testing/selftests/net/devinet_conf.sh


base-commit: fcc00b15cb21a6de76826e5012b2c758505f5e65
-- 
2.43.0

[PATCH net 1/2] ipv4: devinet: update inheritance state only after parsed writes

[Yingjie Wang]

devinet_conf_proc() marks tracked per-device IPv4 settings as explicitly
configured after every write attempt. A rejected integer leaves the value
unchanged but sets its state bit, so a later update to conf/default skips
the device.

Zero-length writes and writes ignored because their file position is
nonzero in strict sysctl mode have the same effect. proc_dointvec()
returns success for both without parsing a value.

Use proc_dointvec_conv() with a converter that records successful integer
conversion. Update inheritance state and run post-set handling only after
both parsing and conversion succeed. The converter uses the same integer
helpers as proc_dointvec(), and valid same-value writes continue to mark a
setting as explicit.

The issue was reproduced with ordinary sysctl writes in fresh network
namespaces, without kernel modification or error injection. On the base
kernel, lo/arp_ignore remained 0 after default/arp_ignore was set to 1
following invalid, blank, zero-byte, or strict nonzero-offset writes. With
this change it inherited 1 in all four cases, while valid same-value and
changed-value writes retained 0 and 2.

Fixes: 31be308541e9 ("[IPV4]: Add default config support after inetdev_init")
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 net/ipv4/devinet.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 1e9d988849c9e..e17c82fcdefed 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2555,14 +2555,37 @@ static int devinet_conf_ifindex(struct net *net, struct ipv4_devconf *cnf)
 	}
 }
 
+static int devinet_conf_int_conv(bool *negp, unsigned long *u_ptr, int *k_ptr,
+				 int dir, const struct ctl_table *ctl)
+{
+	int ret;
+
+	if (dir)
+		ret = proc_int_u2k_conv_uop(u_ptr, k_ptr, negp, NULL);
+	else
+		ret = proc_int_k2u_conv_kop(u_ptr, k_ptr, negp, NULL);
+
+	if (!ret && dir)
+		*(bool *)ctl->extra1 = true;
+
+	return ret;
+}
+
 static int devinet_conf_proc(const struct ctl_table *ctl, int write,
 			     void *buffer, size_t *lenp, loff_t *ppos)
 {
 	int old_value = *(int *)ctl->data;
-	int ret = proc_dointvec(ctl, write, buffer, lenp, ppos);
-	int new_value = *(int *)ctl->data;
+	struct ctl_table tmp = *ctl;
+	bool parsed = false;
+	int ret, new_value;
+
+	/* An ignored write returns success without calling the converter. */
+	tmp.extra1 = &parsed;
+	ret = proc_dointvec_conv(&tmp, write, buffer, lenp, ppos,
+				 devinet_conf_int_conv);
+	new_value = *(int *)ctl->data;
 
-	if (write) {
+	if (write && !ret && parsed) {
 		struct ipv4_devconf *cnf = ctl->extra1;
 		int i = (int *)ctl->data - cnf->data;
 		struct net *net = ctl->extra2;
-- 
2.43.0

[PATCH net 2/2] selftests: net: test devinet inheritance after failed writes

[Yingjie Wang]

A failed per-device IPv4 sysctl write must not mark the setting as
explicitly configured. Otherwise, later conf/default updates stop
propagating to that device.

Add a test which rejects an invalid lo/arp_ignore write and then checks
that lo inherits a new default. Keep loopback unconfigured because adding
its address marks all devconf settings as explicit. Also verify the
complementary case: a valid same-value write marks the setting as explicit
and prevents inheritance.

Before each case, change the default to a value different from the current
lo value and require lo to inherit it. This avoids depending on init_net
defaults and skips the test if inherited state has already disabled
propagation.

The failed-write case fails on the base kernel. Both cases pass with the
preceding fix.

Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 tools/testing/selftests/net/Makefile        |   1 +
 tools/testing/selftests/net/devinet_conf.sh | 101 ++++++++++++++++++++
 2 files changed, 102 insertions(+)
 create mode 100755 tools/testing/selftests/net/devinet_conf.sh

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index df215e1a62621..f551bd40bab80 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -26,6 +26,7 @@ TEST_PROGS := \
 	cmsg_so_priority.sh \
 	cmsg_time.sh \
 	cork_fragsize.py \
+	devinet_conf.sh \
 	double_udp_encap.sh \
 	drop_monitor_tests.sh \
 	ecmp_rehash.sh \
diff --git a/tools/testing/selftests/net/devinet_conf.sh b/tools/testing/selftests/net/devinet_conf.sh
new file mode 100755
index 0000000000000..54655dc2b0bd8
--- /dev/null
+++ b/tools/testing/selftests/net/devinet_conf.sh
@@ -0,0 +1,101 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+source lib.sh
+
+cleanup()
+{
+	cleanup_all_ns
+}
+
+trap cleanup EXIT
+
+prepare_inheritance()
+{
+	local ns=$1
+	local result=$2
+	local current
+	local probe
+	local value
+
+	if ! current=$(ip netns exec "$ns" \
+		sysctl -n net.ipv4.conf.lo.arp_ignore); then
+		echo "ERROR: failed to read lo/arp_ignore in $ns"
+		return "$ksft_fail"
+	fi
+	if ((current == 0)); then
+		probe=1
+	else
+		probe=0
+	fi
+
+	if ! ip netns exec "$ns" sysctl -qw \
+		net.ipv4.conf.default.arp_ignore="$probe"; then
+		echo "ERROR: failed to update default/arp_ignore in $ns"
+		return "$ksft_fail"
+	fi
+	if ! value=$(ip netns exec "$ns" \
+		sysctl -n net.ipv4.conf.lo.arp_ignore); then
+		echo "ERROR: failed to read lo/arp_ignore in $ns"
+		return "$ksft_fail"
+	fi
+	if ((value != probe)); then
+		echo "SKIP: lo/arp_ignore does not inherit conf/default in $ns"
+		return "$ksft_skip"
+	fi
+
+	printf -v "$result" '%d' "$probe"
+}
+
+# Adding the loopback address marks all devconf settings as explicit.
+# Leave it unconfigured so default inheritance remains active.
+failed_ns="failed-$(mktemp -u XXXXXX)"
+if ! ip netns add "$failed_ns"; then
+	echo "Failed to create network namespace $failed_ns"
+	exit "$ksft_skip"
+fi
+NS_LIST+=("$failed_ns")
+
+explicit_ns="explicit-$(mktemp -u XXXXXX)"
+if ! ip netns add "$explicit_ns"; then
+	echo "Failed to create network namespace $explicit_ns"
+	exit "$ksft_skip"
+fi
+NS_LIST+=("$explicit_ns")
+
+prepare_inheritance "$failed_ns" failed_value
+ret=$?
+((ret == 0)) || exit "$ret"
+prepare_inheritance "$explicit_ns" explicit_value
+ret=$?
+((ret == 0)) || exit "$ret"
+failed_next=$((!failed_value))
+explicit_next=$((!explicit_value))
+
+RET=0
+ip netns exec "$failed_ns" sh -c \
+	'printf x > /proc/sys/net/ipv4/conf/lo/arp_ignore' 2>/dev/null
+check_fail $? "invalid arp_ignore write was accepted"
+ip netns exec "$failed_ns" sysctl -qw \
+	net.ipv4.conf.default.arp_ignore="$failed_next"
+check_err $? "failed to update default arp_ignore"
+value=$(ip netns exec "$failed_ns" sysctl -n net.ipv4.conf.lo.arp_ignore)
+check_err $? "failed to read lo arp_ignore"
+check_err "$((value != failed_next))" \
+	"failed write prevented lo from inheriting arp_ignore"
+log_test "Failed devinet write preserves inheritance"
+
+RET=0
+ip netns exec "$explicit_ns" sysctl -qw \
+	net.ipv4.conf.lo.arp_ignore="$explicit_value"
+check_err $? "failed to explicitly set lo arp_ignore"
+ip netns exec "$explicit_ns" sysctl -qw \
+	net.ipv4.conf.default.arp_ignore="$explicit_next"
+check_err $? "failed to update default arp_ignore"
+value=$(ip netns exec "$explicit_ns" sysctl -n net.ipv4.conf.lo.arp_ignore)
+check_err $? "failed to read lo arp_ignore"
+check_err "$((value != explicit_value))" \
+	"explicit lo arp_ignore unexpectedly inherited the default"
+log_test "Valid same-value devinet write prevents inheritance"
+
+exit "$EXIT_STATUS"
-- 
2.43.0

Re: [PATCH net 0/2] ipv4: preserve devinet inheritance after unparsed writes

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.