netdev - Re: [net] Question about ipvs->sysctl_sync_threshold and READ

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 for Android: free password hash cracker in your pocket

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

Message-ID: <4ca70110-b6fb-3842-0d9c-538dbc8cfde3@ssi.bg>
Date: Wed, 20 Mar 2024 20:03:45 +0200 (EET)
From: Julian Anastasov <ja@....bg>
To: Zijie Zhao <zzjas98@...il.com>
cc: horms@...ge.net.au, davem@...emloft.net, dsahern@...nel.org,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        netdev@...r.kernel.org, lvs-devel@...r.kernel.org,
        chenyuan0y@...il.com
Subject: Re: [net] Question about ipvs->sysctl_sync_threshold and READ_ONCE


	Hello,

On Tue, 19 Mar 2024, Zijie Zhao wrote:

> Dear IPVS maintainers,
> 
> We encountered an unusual usage of sysctl parameter while analyzing kernel
> source code.
> 
> 
> In include/net/ip_vs.h, line 1062 - 1070:
> 
> ```
> static inline int sysctl_sync_threshold(struct netns_ipvs *ipvs)
> {
> 	return ipvs->sysctl_sync_threshold[0];
> }
> 
> static inline int sysctl_sync_period(struct netns_ipvs *ipvs)
> {
> 	return READ_ONCE(ipvs->sysctl_sync_threshold[1]);
> }
> ```
> 
> Here, sysctl_sync_threshold[1] is accessed behind `READ_ONCE`, but
> sysctl_sync_threshold[0] is not. Should sysctl_sync_threshold[0] also be
> guarded by `READ_ONCE`?
> 
> Please kindly let us know if we missed any key information and this is
> actually intended. We appreciate your information and time! Thanks!

	Difference comes from commit 749c42b620a9 where we protect us
from division by zero by using ACCESS_ONCE at that time. The idea was
to read the value only once. Nowadays, READ_ONCE also has the role to
avoid load tearing, so, yes, all sysctl vars should be accessed with
READ_ONCE but this is a low priority goal for now.

> Links to the code:
> https://elixir.bootlin.com/linux/v6.8.1/source/include/net/ip_vs.h#L1064
> https://elixir.bootlin.com/linux/v6.8.1/source/include/net/ip_vs.h#L1069
> 
> Best,
> Zijie

Regards

--
Julian Anastasov <ja@....bg>