[PATCH v2 net-next] ipv6: annotate data-races around devconf->rpl_seg_enabled

[PATCH v2 net-next] ipv6: annotate data-races around devconf->rpl_seg_enabled

[Yue Haibing]

devconf->rpl_seg_enabled can be changed concurrently from
/proc/sys/net/ipv6/conf, annotate lockless reads on it.
Also initializes extra1 and extra2 to SYSCTL_ZERO and SYSCTL_ONE
respectively to avoid negative value writes, which may lead to
unexpected results in ipv6_rpl_srh_rcv().

Fixes: 8610c7c6e3bd ("net: ipv6: add support for rpl sr exthdr")
Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
---
v2: add extra1/2 check
---
 net/ipv6/addrconf.c | 4 +++-
 net/ipv6/exthdrs.c  | 6 ++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 1c1d5cb6a7c1..265238574aab 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -7240,7 +7240,9 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data		= &ipv6_devconf.rpl_seg_enabled,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_ONE,
 	},
 	{
 		.procname	= "ioam6_enabled",
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index d1ef9644f826..a23eb8734e15 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -494,10 +494,8 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb)
 
 	idev = __in6_dev_get(skb->dev);
 
-	accept_rpl_seg = net->ipv6.devconf_all->rpl_seg_enabled;
-	if (accept_rpl_seg > idev->cnf.rpl_seg_enabled)
-		accept_rpl_seg = idev->cnf.rpl_seg_enabled;
-
+	accept_rpl_seg = min(READ_ONCE(net->ipv6.devconf_all->rpl_seg_enabled),
+			     READ_ONCE(idev->cnf.rpl_seg_enabled));
 	if (!accept_rpl_seg) {
 		kfree_skb(skb);
 		return -1;
-- 
2.34.1

Re: [PATCH v2 net-next] ipv6: annotate data-races around devconf->rpl_seg_enabled

[Jakub Kicinski]

On Wed, 27 Aug 2025 16:12:43 +0800 Yue Haibing wrote:
> Also initializes extra1 and extra2 to SYSCTL_ZERO and SYSCTL_ONE
> respectively to avoid negative value writes, which may lead to
> unexpected results in ipv6_rpl_srh_rcv().

By unexpected results you mean that min() is intended to return 0
when either value is zero, but if one of the values is negative it
will in fact return non-zero?

That's a fair point, but I'm not sure whether we should be sending
that up as a fix. It's more of a sanity check that prevents
unintentional misconfiguration.. Please split this patch into two
separate ones, and send the minmax one without a Fixes tag.
Please include more of the explanation I have provided in the first
paragraph in the commit message, "unexpected results" is too vague
by itself.

Re: [PATCH v2 net-next] ipv6: annotate data-races around devconf->rpl_seg_enabled

[Yue Haibing]

On 2025/8/30 10:27, Jakub Kicinski wrote:
> On Wed, 27 Aug 2025 16:12:43 +0800 Yue Haibing wrote:
>> Also initializes extra1 and extra2 to SYSCTL_ZERO and SYSCTL_ONE
>> respectively to avoid negative value writes, which may lead to
>> unexpected results in ipv6_rpl_srh_rcv().
> 
> By unexpected results you mean that min() is intended to return 0
> when either value is zero, but if one of the values is negative it
> will in fact return non-zero?

Yes，this is exact.
> 
> That's a fair point, but I'm not sure whether we should be sending
> that up as a fix. It's more of a sanity check that prevents
> unintentional misconfiguration.. Please split this patch into two
> separate ones, and send the minmax one without a Fixes tag.
> Please include more of the explanation I have provided in the first
> paragraph in the commit message, "unexpected results" is too vague
> by itself.
Ok, thanks, will split and resend with this.
>