* [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications
@ 2026-06-18 16:22 Fernando Fernandez Mancera
2026-06-18 16:22 ` [PATCH net 1/6] ipv6: fix error handling in disable_ipv6 sysctl Fernando Fernandez Mancera
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Fernando Fernandez Mancera @ 2026-06-18 16:22 UTC (permalink / raw)
To: netdev
Cc: nicolas.dichtel, shemminger, dforster, gospo, ddutt, brian.haley,
horms, pabeni, kuba, edumazet, davem, idosch, dsahern,
Fernando Fernandez Mancera
While working on a different IPv6 patch series I have spotted multiple
minor bugs around sysctl error handling and notifications. In general,
they are not serious issues.
In addition, there is one more issue in forwarding sysctl as it does not
check for CAP_NET_ADMIN for the namespace. I am keeping that patch out
of this series and I am aiming it at the net-next tree once it re-opens.
Fernando Fernandez Mancera (6):
ipv6: fix error handling in disable_ipv6 sysctl
ipv6: fix error handling in ignore_routes_with_linkdown sysctl
ipv6: fix error handling in forwarding sysctl
ipv6: fix error handling in disable_policy sysctl
ipv6: reset value and position for proxy_ndp sysctl restart
ipv6: fix missing notification for ignore_routes_with_linkdown
net/ipv6/addrconf.c | 35 +++++++++++++++++++++++++++--------
1 file changed, 27 insertions(+), 8 deletions(-)
--
2.54.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net 1/6] ipv6: fix error handling in disable_ipv6 sysctl
2026-06-18 16:22 [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
@ 2026-06-18 16:22 ` Fernando Fernandez Mancera
2026-06-19 9:33 ` Nicolas Dichtel
2026-06-18 16:22 ` [PATCH net 2/6] ipv6: fix error handling in ignore_routes_with_linkdown sysctl Fernando Fernandez Mancera
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Fernando Fernandez Mancera @ 2026-06-18 16:22 UTC (permalink / raw)
To: netdev
Cc: nicolas.dichtel, shemminger, dforster, gospo, ddutt, brian.haley,
horms, pabeni, kuba, edumazet, davem, idosch, dsahern,
Fernando Fernandez Mancera
When writing to the disable_ipv6 sysctl, if proc_dointvec() fails to
parse the input, it returns a negative error code. The current
implementation is overwriting that error for write operations.
This results in a silent failure, it returns a successful write although
the configuration was not modified at all. When modifying the "all"
variant it can also modify the configuration of existing interfaces to
the wrong value.
Fix this by checking the return value of proc_dointvec() and returning
early on failure.
Fixes: 56d417b12e57 ("IPv6: Add 'autoconf' and 'disable_ipv6' module parameters")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/ipv6/addrconf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 1f21ccb55caa..c901b444a995 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6467,6 +6467,8 @@ static int addrconf_sysctl_disable(const struct ctl_table *ctl, int write,
lctl.data = &val;
ret = proc_dointvec(&lctl, write, buffer, lenp, ppos);
+ if (ret)
+ return ret;
if (write)
ret = addrconf_disable_ipv6(ctl, valp, val);
--
2.54.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 2/6] ipv6: fix error handling in ignore_routes_with_linkdown sysctl
2026-06-18 16:22 [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
2026-06-18 16:22 ` [PATCH net 1/6] ipv6: fix error handling in disable_ipv6 sysctl Fernando Fernandez Mancera
@ 2026-06-18 16:22 ` Fernando Fernandez Mancera
2026-06-19 9:34 ` Nicolas Dichtel
2026-06-18 16:22 ` [PATCH net 3/6] ipv6: fix error handling in forwarding sysctl Fernando Fernandez Mancera
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Fernando Fernandez Mancera @ 2026-06-18 16:22 UTC (permalink / raw)
To: netdev
Cc: nicolas.dichtel, shemminger, dforster, gospo, ddutt, brian.haley,
horms, pabeni, kuba, edumazet, davem, idosch, dsahern,
Fernando Fernandez Mancera
When writing to the ignore_routes_with_linkdown sysctl, if
proc_dointvec() fails to parse the input, it returns a negative error
code. The current implementation is overwriting that error for write
operations.
This results in a silent failure, it returns a successful write although
the configuration was not modified at all. When modifying the "all"
variant it can also modify the configuration of existing interfaces to
the wrong value.
Fix this by checking the return value of proc_dointvec() and returning
early on failure.
Fixes: 35103d11173b ("net: ipv6 sysctl option to ignore routes when nexthop link is down")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/ipv6/addrconf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c901b444a995..70058d971205 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6671,6 +6671,8 @@ int addrconf_sysctl_ignore_routes_with_linkdown(const struct ctl_table *ctl,
lctl.data = &val;
ret = proc_dointvec(&lctl, write, buffer, lenp, ppos);
+ if (ret)
+ return ret;
if (write)
ret = addrconf_fixup_linkdown(ctl, valp, val);
--
2.54.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 3/6] ipv6: fix error handling in forwarding sysctl
2026-06-18 16:22 [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
2026-06-18 16:22 ` [PATCH net 1/6] ipv6: fix error handling in disable_ipv6 sysctl Fernando Fernandez Mancera
2026-06-18 16:22 ` [PATCH net 2/6] ipv6: fix error handling in ignore_routes_with_linkdown sysctl Fernando Fernandez Mancera
@ 2026-06-18 16:22 ` Fernando Fernandez Mancera
2026-06-19 9:34 ` Nicolas Dichtel
2026-06-18 16:22 ` [PATCH net 4/6] ipv6: fix error handling in disable_policy sysctl Fernando Fernandez Mancera
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Fernando Fernandez Mancera @ 2026-06-18 16:22 UTC (permalink / raw)
To: netdev
Cc: nicolas.dichtel, shemminger, dforster, gospo, ddutt, brian.haley,
horms, pabeni, kuba, edumazet, davem, idosch, dsahern,
Fernando Fernandez Mancera
When writing to the forwarding sysctl, if proc_dointvec() fails to parse
the input, it returns a negative error code. The current implementation
is overwriting that error for write operations.
This results in a silent failure, it returns a successful write although
the configuration was not modified at all. When modifying the "all"
variant it can also modify the configuration of existing interfaces to
the wrong value.
Fix this by checking the return value of proc_dointvec() and returning
early on failure.
Fixes: b325fddb7f86 ("ipv6: Fix sysctl unregistration deadlock")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/ipv6/addrconf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 70058d971205..0b128b6cff60 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6370,6 +6370,8 @@ static int addrconf_sysctl_forward(const struct ctl_table *ctl, int write,
lctl.data = &val;
ret = proc_dointvec(&lctl, write, buffer, lenp, ppos);
+ if (ret)
+ return ret;
if (write)
ret = addrconf_fixup_forwarding(ctl, valp, val);
--
2.54.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 4/6] ipv6: fix error handling in disable_policy sysctl
2026-06-18 16:22 [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
` (2 preceding siblings ...)
2026-06-18 16:22 ` [PATCH net 3/6] ipv6: fix error handling in forwarding sysctl Fernando Fernandez Mancera
@ 2026-06-18 16:22 ` Fernando Fernandez Mancera
2026-06-19 9:35 ` Nicolas Dichtel
2026-06-18 16:22 ` [PATCH net 5/6] ipv6: reset value and position for proxy_ndp sysctl restart Fernando Fernandez Mancera
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Fernando Fernandez Mancera @ 2026-06-18 16:22 UTC (permalink / raw)
To: netdev
Cc: nicolas.dichtel, shemminger, dforster, gospo, ddutt, brian.haley,
horms, pabeni, kuba, edumazet, davem, idosch, dsahern,
Fernando Fernandez Mancera
When writing to the disable_policy sysctl, if proc_dointvec() fails to
parse the input, it returns a negative error code. The current
implementation is resetting the position argument even if an error
occurred during proc_dointvec() and not only during sysctl restart.
Fix this by checking the return value of proc_dointvec() and returning
early on failure.
Fixes: df789fe75206 ("ipv6: Provide ipv6 version of "disable_policy" sysctl")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/ipv6/addrconf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 0b128b6cff60..8ff015975e27 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6769,6 +6769,8 @@ static int addrconf_sysctl_disable_policy(const struct ctl_table *ctl, int write
lctl = *ctl;
lctl.data = &val;
ret = proc_dointvec(&lctl, write, buffer, lenp, ppos);
+ if (ret)
+ return ret;
if (write && (*valp != val))
ret = addrconf_disable_policy(ctl, valp, val);
--
2.54.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 5/6] ipv6: reset value and position for proxy_ndp sysctl restart
2026-06-18 16:22 [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
` (3 preceding siblings ...)
2026-06-18 16:22 ` [PATCH net 4/6] ipv6: fix error handling in disable_policy sysctl Fernando Fernandez Mancera
@ 2026-06-18 16:22 ` Fernando Fernandez Mancera
2026-06-19 9:58 ` Nicolas Dichtel
2026-06-18 16:22 ` [PATCH net 6/6] ipv6: fix missing notification for ignore_routes_with_linkdown Fernando Fernandez Mancera
2026-06-19 16:42 ` [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
6 siblings, 1 reply; 17+ messages in thread
From: Fernando Fernandez Mancera @ 2026-06-18 16:22 UTC (permalink / raw)
To: netdev
Cc: nicolas.dichtel, shemminger, dforster, gospo, ddutt, brian.haley,
horms, pabeni, kuba, edumazet, davem, idosch, dsahern,
Fernando Fernandez Mancera
When handling proxy_ndp, if rtnl_net_trylock() fails, the operation is
retried but as the value was already modified by the initial
proc_dointvec() call, the restarted syscall will read the newly modified
value as the 'old' state.
Fix this by restoring the original value and position pointer before
restarting the syscall.
Fixes: c92d5491a6d9 ("netconf: add support for IPv6 proxy_ndp")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/ipv6/addrconf.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 8ff015975e27..1cfb223476bd 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6483,8 +6483,9 @@ static int addrconf_sysctl_proxy_ndp(const struct ctl_table *ctl, int write,
void *buffer, size_t *lenp, loff_t *ppos)
{
int *valp = ctl->data;
- int ret;
+ loff_t pos = *ppos;
int old, new;
+ int ret;
old = *valp;
ret = proc_dointvec(ctl, write, buffer, lenp, ppos);
@@ -6493,8 +6494,12 @@ static int addrconf_sysctl_proxy_ndp(const struct ctl_table *ctl, int write,
if (write && old != new) {
struct net *net = ctl->extra2;
- if (!rtnl_net_trylock(net))
+ if (!rtnl_net_trylock(net)) {
+ /* Restore the original values before restarting */
+ *valp = old;
+ *ppos = pos;
return restart_syscall();
+ }
if (valp == &net->ipv6.devconf_dflt->proxy_ndp) {
inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
--
2.54.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net 6/6] ipv6: fix missing notification for ignore_routes_with_linkdown
2026-06-18 16:22 [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
` (4 preceding siblings ...)
2026-06-18 16:22 ` [PATCH net 5/6] ipv6: reset value and position for proxy_ndp sysctl restart Fernando Fernandez Mancera
@ 2026-06-18 16:22 ` Fernando Fernandez Mancera
2026-06-19 10:02 ` Nicolas Dichtel
2026-06-19 16:42 ` [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
6 siblings, 1 reply; 17+ messages in thread
From: Fernando Fernandez Mancera @ 2026-06-18 16:22 UTC (permalink / raw)
To: netdev
Cc: nicolas.dichtel, shemminger, dforster, gospo, ddutt, brian.haley,
horms, pabeni, kuba, edumazet, davem, idosch, dsahern,
Fernando Fernandez Mancera
When changing the ignore_routes_with_linkdown sysctl for a specific
interface, the RTM_NEWNETCONF netlink notification was not being emitted
to userspace. Fix this by emitting the notification when needed.
In addition, fix bogus return value for successful "all" and specific
interface write operation leading to a wrong reset of the position
pointer.
Fixes: 35103d11173b ("net: ipv6 sysctl option to ignore routes when nexthop link is down")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/ipv6/addrconf.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 1cfb223476bd..2d81569b692b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -955,11 +955,7 @@ static int addrconf_fixup_linkdown(const struct ctl_table *table, int *p, int ne
NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
NETCONFA_IFINDEX_DEFAULT,
net->ipv6.devconf_dflt);
- rtnl_net_unlock(net);
- return 0;
- }
-
- if (p == &net->ipv6.devconf_all->ignore_routes_with_linkdown) {
+ } else if (p == &net->ipv6.devconf_all->ignore_routes_with_linkdown) {
WRITE_ONCE(net->ipv6.devconf_dflt->ignore_routes_with_linkdown, newf);
addrconf_linkdown_change(net, newf);
if ((!newf) ^ (!old))
@@ -968,11 +964,21 @@ static int addrconf_fixup_linkdown(const struct ctl_table *table, int *p, int ne
NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
NETCONFA_IFINDEX_ALL,
net->ipv6.devconf_all);
+ } else {
+ if (!newf ^ !old) {
+ struct inet6_dev *idev = table->extra1;
+
+ inet6_netconf_notify_devconf(net,
+ RTM_NEWNETCONF,
+ NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
+ idev->dev->ifindex,
+ &idev->cnf);
+ }
}
rtnl_net_unlock(net);
- return 1;
+ return 0;
}
#endif
--
2.54.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net 1/6] ipv6: fix error handling in disable_ipv6 sysctl
2026-06-18 16:22 ` [PATCH net 1/6] ipv6: fix error handling in disable_ipv6 sysctl Fernando Fernandez Mancera
@ 2026-06-19 9:33 ` Nicolas Dichtel
0 siblings, 0 replies; 17+ messages in thread
From: Nicolas Dichtel @ 2026-06-19 9:33 UTC (permalink / raw)
To: Fernando Fernandez Mancera, netdev
Cc: shemminger, dforster, gospo, ddutt, brian.haley, horms, pabeni,
kuba, edumazet, davem, idosch, dsahern
Le 18/06/2026 à 18:22, Fernando Fernandez Mancera a écrit :
> When writing to the disable_ipv6 sysctl, if proc_dointvec() fails to
> parse the input, it returns a negative error code. The current
> implementation is overwriting that error for write operations.
>
> This results in a silent failure, it returns a successful write although
> the configuration was not modified at all. When modifying the "all"
> variant it can also modify the configuration of existing interfaces to
> the wrong value.
>
> Fix this by checking the return value of proc_dointvec() and returning
> early on failure.
>
> Fixes: 56d417b12e57 ("IPv6: Add 'autoconf' and 'disable_ipv6' module parameters")
> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 2/6] ipv6: fix error handling in ignore_routes_with_linkdown sysctl
2026-06-18 16:22 ` [PATCH net 2/6] ipv6: fix error handling in ignore_routes_with_linkdown sysctl Fernando Fernandez Mancera
@ 2026-06-19 9:34 ` Nicolas Dichtel
0 siblings, 0 replies; 17+ messages in thread
From: Nicolas Dichtel @ 2026-06-19 9:34 UTC (permalink / raw)
To: Fernando Fernandez Mancera, netdev
Cc: shemminger, dforster, gospo, ddutt, brian.haley, horms, pabeni,
kuba, edumazet, davem, idosch, dsahern
Le 18/06/2026 à 18:22, Fernando Fernandez Mancera a écrit :
> When writing to the ignore_routes_with_linkdown sysctl, if
> proc_dointvec() fails to parse the input, it returns a negative error
> code. The current implementation is overwriting that error for write
> operations.
>
> This results in a silent failure, it returns a successful write although
> the configuration was not modified at all. When modifying the "all"
> variant it can also modify the configuration of existing interfaces to
> the wrong value.
>
> Fix this by checking the return value of proc_dointvec() and returning
> early on failure.
>
> Fixes: 35103d11173b ("net: ipv6 sysctl option to ignore routes when nexthop link is down")
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 3/6] ipv6: fix error handling in forwarding sysctl
2026-06-18 16:22 ` [PATCH net 3/6] ipv6: fix error handling in forwarding sysctl Fernando Fernandez Mancera
@ 2026-06-19 9:34 ` Nicolas Dichtel
2026-06-19 10:28 ` Fernando Fernandez Mancera
0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Dichtel @ 2026-06-19 9:34 UTC (permalink / raw)
To: Fernando Fernandez Mancera, netdev
Cc: shemminger, dforster, gospo, ddutt, brian.haley, horms, pabeni,
kuba, edumazet, davem, idosch, dsahern
Le 18/06/2026 à 18:22, Fernando Fernandez Mancera a écrit :
> When writing to the forwarding sysctl, if proc_dointvec() fails to parse
> the input, it returns a negative error code. The current implementation
> is overwriting that error for write operations.
>
> This results in a silent failure, it returns a successful write although
> the configuration was not modified at all. When modifying the "all"
> variant it can also modify the configuration of existing interfaces to
> the wrong value.
>
> Fix this by checking the return value of proc_dointvec() and returning
> early on failure.
>
> Fixes: b325fddb7f86 ("ipv6: Fix sysctl unregistration deadlock")
The bug existed before the git era.
Maybe
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 4/6] ipv6: fix error handling in disable_policy sysctl
2026-06-18 16:22 ` [PATCH net 4/6] ipv6: fix error handling in disable_policy sysctl Fernando Fernandez Mancera
@ 2026-06-19 9:35 ` Nicolas Dichtel
0 siblings, 0 replies; 17+ messages in thread
From: Nicolas Dichtel @ 2026-06-19 9:35 UTC (permalink / raw)
To: Fernando Fernandez Mancera, netdev
Cc: shemminger, dforster, gospo, ddutt, brian.haley, horms, pabeni,
kuba, edumazet, davem, idosch, dsahern
Le 18/06/2026 à 18:22, Fernando Fernandez Mancera a écrit :
> When writing to the disable_policy sysctl, if proc_dointvec() fails to
> parse the input, it returns a negative error code. The current
> implementation is resetting the position argument even if an error
> occurred during proc_dointvec() and not only during sysctl restart.
>
> Fix this by checking the return value of proc_dointvec() and returning
> early on failure.
>
> Fixes: df789fe75206 ("ipv6: Provide ipv6 version of "disable_policy" sysctl")
> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 5/6] ipv6: reset value and position for proxy_ndp sysctl restart
2026-06-18 16:22 ` [PATCH net 5/6] ipv6: reset value and position for proxy_ndp sysctl restart Fernando Fernandez Mancera
@ 2026-06-19 9:58 ` Nicolas Dichtel
2026-06-19 10:09 ` Fernando Fernandez Mancera
0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Dichtel @ 2026-06-19 9:58 UTC (permalink / raw)
To: Fernando Fernandez Mancera, netdev
Cc: shemminger, dforster, gospo, ddutt, brian.haley, horms, pabeni,
kuba, edumazet, davem, idosch, dsahern
Le 18/06/2026 à 18:22, Fernando Fernandez Mancera a écrit :
> When handling proxy_ndp, if rtnl_net_trylock() fails, the operation is
> retried but as the value was already modified by the initial
> proc_dointvec() call, the restarted syscall will read the newly modified
> value as the 'old' state.
>
> Fix this by restoring the original value and position pointer before
> restarting the syscall.
Is it not better to call rtnl_net_trylock() at the beginning of the function?
It avoids flapping the sysctl value.
>
> Fixes: c92d5491a6d9 ("netconf: add support for IPv6 proxy_ndp")
> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
> ---
> net/ipv6/addrconf.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 8ff015975e27..1cfb223476bd 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -6483,8 +6483,9 @@ static int addrconf_sysctl_proxy_ndp(const struct ctl_table *ctl, int write,
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> int *valp = ctl->data;
> - int ret;
> + loff_t pos = *ppos;
> int old, new;
> + int ret;
>
> old = *valp;
> ret = proc_dointvec(ctl, write, buffer, lenp, ppos);
> @@ -6493,8 +6494,12 @@ static int addrconf_sysctl_proxy_ndp(const struct ctl_table *ctl, int write,
> if (write && old != new) {
> struct net *net = ctl->extra2;
>
> - if (!rtnl_net_trylock(net))
> + if (!rtnl_net_trylock(net)) {
> + /* Restore the original values before restarting */
> + *valp = old;
> + *ppos = pos;
> return restart_syscall();
> + }
>
> if (valp == &net->ipv6.devconf_dflt->proxy_ndp) {
> inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 6/6] ipv6: fix missing notification for ignore_routes_with_linkdown
2026-06-18 16:22 ` [PATCH net 6/6] ipv6: fix missing notification for ignore_routes_with_linkdown Fernando Fernandez Mancera
@ 2026-06-19 10:02 ` Nicolas Dichtel
0 siblings, 0 replies; 17+ messages in thread
From: Nicolas Dichtel @ 2026-06-19 10:02 UTC (permalink / raw)
To: Fernando Fernandez Mancera, netdev
Cc: shemminger, dforster, gospo, ddutt, brian.haley, horms, pabeni,
kuba, edumazet, davem, idosch, dsahern
Le 18/06/2026 à 18:22, Fernando Fernandez Mancera a écrit :
> When changing the ignore_routes_with_linkdown sysctl for a specific
> interface, the RTM_NEWNETCONF netlink notification was not being emitted
> to userspace. Fix this by emitting the notification when needed.
>
> In addition, fix bogus return value for successful "all" and specific
> interface write operation leading to a wrong reset of the position
> pointer.
>
> Fixes: 35103d11173b ("net: ipv6 sysctl option to ignore routes when nexthop link is down")
> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 5/6] ipv6: reset value and position for proxy_ndp sysctl restart
2026-06-19 9:58 ` Nicolas Dichtel
@ 2026-06-19 10:09 ` Fernando Fernandez Mancera
0 siblings, 0 replies; 17+ messages in thread
From: Fernando Fernandez Mancera @ 2026-06-19 10:09 UTC (permalink / raw)
To: nicolas.dichtel, netdev
Cc: shemminger, dforster, gospo, ddutt, brian.haley, horms, pabeni,
kuba, edumazet, davem, idosch, dsahern
On 6/19/26 11:58 AM, Nicolas Dichtel wrote:
> Le 18/06/2026 à 18:22, Fernando Fernandez Mancera a écrit :
>> When handling proxy_ndp, if rtnl_net_trylock() fails, the operation is
>> retried but as the value was already modified by the initial
>> proc_dointvec() call, the restarted syscall will read the newly modified
>> value as the 'old' state.
>>
>> Fix this by restoring the original value and position pointer before
>> restarting the syscall.
> Is it not better to call rtnl_net_trylock() at the beginning of the function?
> It avoids flapping the sysctl value.
>
IMHO it is not better if we want to reduce the time we are holding RTNL
lock. I think the idea is that if the user introduces a invalid value,
we don't need to take the lock at all.
That is the general pattern I see around the sysctl code (IPv4 and
IPv6). Given the current efforts to reduce the usage of RTNL I think
this approach would be better.
In any case, it is not a blocker for me so if we all agree that your
suggestion is better I don't mind taking that path.
Thanks for all the reviews!
>>
>> Fixes: c92d5491a6d9 ("netconf: add support for IPv6 proxy_ndp")
>> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
>> ---
>> net/ipv6/addrconf.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index 8ff015975e27..1cfb223476bd 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -6483,8 +6483,9 @@ static int addrconf_sysctl_proxy_ndp(const struct ctl_table *ctl, int write,
>> void *buffer, size_t *lenp, loff_t *ppos)
>> {
>> int *valp = ctl->data;
>> - int ret;
>> + loff_t pos = *ppos;
>> int old, new;
>> + int ret;
>>
>> old = *valp;
>> ret = proc_dointvec(ctl, write, buffer, lenp, ppos);
>> @@ -6493,8 +6494,12 @@ static int addrconf_sysctl_proxy_ndp(const struct ctl_table *ctl, int write,
>> if (write && old != new) {
>> struct net *net = ctl->extra2;
>>
>> - if (!rtnl_net_trylock(net))
>> + if (!rtnl_net_trylock(net)) {
>> + /* Restore the original values before restarting */
>> + *valp = old;
>> + *ppos = pos;
>> return restart_syscall();
>> + }
>>
>> if (valp == &net->ipv6.devconf_dflt->proxy_ndp) {
>> inet6_netconf_notify_devconf(net, RTM_NEWNETCONF,
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 3/6] ipv6: fix error handling in forwarding sysctl
2026-06-19 9:34 ` Nicolas Dichtel
@ 2026-06-19 10:28 ` Fernando Fernandez Mancera
2026-06-19 13:04 ` Nicolas Dichtel
0 siblings, 1 reply; 17+ messages in thread
From: Fernando Fernandez Mancera @ 2026-06-19 10:28 UTC (permalink / raw)
To: nicolas.dichtel, netdev
Cc: shemminger, dforster, gospo, ddutt, brian.haley, horms, pabeni,
kuba, edumazet, davem, idosch, dsahern
On 6/19/26 11:34 AM, Nicolas Dichtel wrote:
> Le 18/06/2026 à 18:22, Fernando Fernandez Mancera a écrit :
>> When writing to the forwarding sysctl, if proc_dointvec() fails to parse
>> the input, it returns a negative error code. The current implementation
>> is overwriting that error for write operations.
>>
>> This results in a silent failure, it returns a successful write although
>> the configuration was not modified at all. When modifying the "all"
>> variant it can also modify the configuration of existing interfaces to
>> the wrong value.
>>
>> Fix this by checking the return value of proc_dointvec() and returning
>> early on failure.
>>
>> Fixes: b325fddb7f86 ("ipv6: Fix sysctl unregistration deadlock")
> The bug existed before the git era.
> Maybe
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>
>
Hm, not really, AFAICS b325fddb7f86 is the first commit overwriting the
return value from proc_dointvec(). See:
@@ -3983,7 +3986,7 @@ int addrconf_sysctl_forward(ctl_table *ctl, int
write, struct file * filp,
ret = proc_dointvec(ctl, write, filp, buffer, lenp, ppos);
if (write)
- addrconf_fixup_forwarding(ctl, valp, val);
+ ret = addrconf_fixup_forwarding(ctl, valp, val);
return ret;
}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 3/6] ipv6: fix error handling in forwarding sysctl
2026-06-19 10:28 ` Fernando Fernandez Mancera
@ 2026-06-19 13:04 ` Nicolas Dichtel
0 siblings, 0 replies; 17+ messages in thread
From: Nicolas Dichtel @ 2026-06-19 13:04 UTC (permalink / raw)
To: Fernando Fernandez Mancera, netdev
Cc: shemminger, dforster, gospo, ddutt, brian.haley, horms, pabeni,
kuba, edumazet, davem, idosch, dsahern
Le 19/06/2026 à 12:28, Fernando Fernandez Mancera a écrit :
> On 6/19/26 11:34 AM, Nicolas Dichtel wrote:
>> Le 18/06/2026 à 18:22, Fernando Fernandez Mancera a écrit :
>>> When writing to the forwarding sysctl, if proc_dointvec() fails to parse
>>> the input, it returns a negative error code. The current implementation
>>> is overwriting that error for write operations.
>>>
>>> This results in a silent failure, it returns a successful write although
>>> the configuration was not modified at all. When modifying the "all"
>>> variant it can also modify the configuration of existing interfaces to
>>> the wrong value.
>>>
>>> Fix this by checking the return value of proc_dointvec() and returning
>>> early on failure.
>>>
>>> Fixes: b325fddb7f86 ("ipv6: Fix sysctl unregistration deadlock")
>> The bug existed before the git era.
>> Maybe
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>
>>
>
>
> Hm, not really, AFAICS b325fddb7f86 is the first commit overwriting the return
> value from proc_dointvec(). See:
>
> @@ -3983,7 +3986,7 @@ int addrconf_sysctl_forward(ctl_table *ctl, int write,
> struct file * filp,
> ret = proc_dointvec(ctl, write, filp, buffer, lenp, ppos);
>
> if (write)
> - addrconf_fixup_forwarding(ctl, valp, val);
> + ret = addrconf_fixup_forwarding(ctl, valp, val);
> return ret;
> }
Hmm, you're right.
Reviewed-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications
2026-06-18 16:22 [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
` (5 preceding siblings ...)
2026-06-18 16:22 ` [PATCH net 6/6] ipv6: fix missing notification for ignore_routes_with_linkdown Fernando Fernandez Mancera
@ 2026-06-19 16:42 ` Fernando Fernandez Mancera
6 siblings, 0 replies; 17+ messages in thread
From: Fernando Fernandez Mancera @ 2026-06-19 16:42 UTC (permalink / raw)
To: netdev
Cc: nicolas.dichtel, shemminger, dforster, gospo, ddutt, brian.haley,
horms, pabeni, kuba, edumazet, davem, idosch, dsahern
On 6/18/26 6:22 PM, Fernando Fernandez Mancera wrote:
> While working on a different IPv6 patch series I have spotted multiple
> minor bugs around sysctl error handling and notifications. In general,
> they are not serious issues.
>
> In addition, there is one more issue in forwarding sysctl as it does not
> check for CAP_NET_ADMIN for the namespace. I am keeping that patch out
> of this series and I am aiming it at the net-next tree once it re-opens.
>
> Fernando Fernandez Mancera (6):
> ipv6: fix error handling in disable_ipv6 sysctl
> ipv6: fix error handling in ignore_routes_with_linkdown sysctl
> ipv6: fix error handling in forwarding sysctl
> ipv6: fix error handling in disable_policy sysctl
> ipv6: reset value and position for proxy_ndp sysctl restart
> ipv6: fix missing notification for ignore_routes_with_linkdown
>
> net/ipv6/addrconf.c | 35 +++++++++++++++++++++++++++--------
> 1 file changed, 27 insertions(+), 8 deletions(-)
>
So I have checked the feedback from sashiko.dev [1]. In general, it is
real but I have doubts about this one. While it is real, I am not so
sure we actually care about this scenario. In any case, this is a
pattern around kernel code and it would require fixes everywhere
(IPv4/IPv6/other places (?)).
The easier thing to do is what Nicolas suggested about taking the lock
before parsing the value i.e before calling proc_dointvec().
I also do wonder if I should fix them in individual commits or together
in the same commit as they have different fixes tag. Or maybe we can aim
this last part to net-next. Thoughts?
Please, see the feedback attached below.
Thanks!
[1] https://sashiko.dev/#/patchset/20260618162225.4588-1-fmancera%40suse.de
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 8ff015975e279..1cfb223476bd6 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
[ ... ]
> @@ -6493,8 +6494,12 @@ static int addrconf_sysctl_proxy_ndp(const
struct ctl_table *ctl, int write,
> if (write && old != new) {
> struct net *net = ctl->extra2;
>
> - if (!rtnl_net_trylock(net))
> + if (!rtnl_net_trylock(net)) {
> + /* Restore the original values before restarting */
> + *valp = old;
> + *ppos = pos;
> return restart_syscall();
> + }
>
Does this rollback introduce a race condition that can silently discard
concurrent writes?
The sysctl core does not serialize concurrent writes to the same sysctl
parameter, and proc_dointvec() modifies the global sysctl memory pointed to
by ctl->data directly.
If Thread A and Thread B concurrently write to proxy_ndp, and Thread A fails
to acquire the RTNL lock, setting *valp = old will unconditionally reset the
global memory back to what Thread A observed before its write. This seems
like it would clobber Thread B's concurrent update.
If Thread B successfully acquires the RTNL lock, it will evaluate its local
old != new stack variables as true and emit a RTM_NEWNETCONF netlink
notification. However, the notification will read the corrupted *valp
memory,
leaving the emitted event and internal sysctl state out of sync.
Could this be avoided by duplicating the ctl_table, pointing the data field
to a local stack variable, calling proc_dointvec() on the local variable,
and only updating the global *valp after successfully acquiring the lock,
similar to how it is handled in addrconf_sysctl_forward()?
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-06-19 16:48 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18 16:22 [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
2026-06-18 16:22 ` [PATCH net 1/6] ipv6: fix error handling in disable_ipv6 sysctl Fernando Fernandez Mancera
2026-06-19 9:33 ` Nicolas Dichtel
2026-06-18 16:22 ` [PATCH net 2/6] ipv6: fix error handling in ignore_routes_with_linkdown sysctl Fernando Fernandez Mancera
2026-06-19 9:34 ` Nicolas Dichtel
2026-06-18 16:22 ` [PATCH net 3/6] ipv6: fix error handling in forwarding sysctl Fernando Fernandez Mancera
2026-06-19 9:34 ` Nicolas Dichtel
2026-06-19 10:28 ` Fernando Fernandez Mancera
2026-06-19 13:04 ` Nicolas Dichtel
2026-06-18 16:22 ` [PATCH net 4/6] ipv6: fix error handling in disable_policy sysctl Fernando Fernandez Mancera
2026-06-19 9:35 ` Nicolas Dichtel
2026-06-18 16:22 ` [PATCH net 5/6] ipv6: reset value and position for proxy_ndp sysctl restart Fernando Fernandez Mancera
2026-06-19 9:58 ` Nicolas Dichtel
2026-06-19 10:09 ` Fernando Fernandez Mancera
2026-06-18 16:22 ` [PATCH net 6/6] ipv6: fix missing notification for ignore_routes_with_linkdown Fernando Fernandez Mancera
2026-06-19 10:02 ` Nicolas Dichtel
2026-06-19 16:42 ` [PATCH net 0/6] ipv6: fix sysctl error handling and missing notifications Fernando Fernandez Mancera
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox