2022-07-12 01:11:56

by Kuniyuki Iwashima

[permalink] [raw]
Subject: [PATCH v1 net 00/15] sysctl: Fix data-races around ipv4_net_table (Round 1).

This series fixes data-races around the first 13 knobs and
nexthop_compat_mode in ipv4_net_table.

I will post another patch for three early_demux knobs later,
so the next round will start from ip_default_ttl.


Kuniyuki Iwashima (15):
sysctl: Fix data-races in proc_dou8vec_minmax().
sysctl: Fix data-races in proc_dointvec_ms_jiffies().
tcp: Fix a data-race around sysctl_max_tw_buckets.
icmp: Fix a data-race around sysctl_icmp_echo_ignore_all.
icmp: Fix data-races around sysctl_icmp_echo_enable_probe.
icmp: Fix a data-race around sysctl_icmp_echo_ignore_broadcasts.
icmp: Fix a data-race around sysctl_icmp_ignore_bogus_error_responses.
icmp: Fix a data-race around sysctl_icmp_errors_use_inbound_ifaddr.
icmp: Fix a data-race around sysctl_icmp_ratelimit.
icmp: Fix a data-race around sysctl_icmp_ratemask.
raw: Fix a data-race around sysctl_raw_l3mdev_accept.
tcp: Fix data-races around sysctl_tcp_ecn.
tcp: Fix a data-race around sysctl_tcp_ecn_fallback.
ipv4: Fix data-races around sysctl_ip_dynaddr.
nexthop: Fix data-races around nexthop_compat_mode.

Documentation/networking/ip-sysctl.rst | 2 +-
.../chelsio/inline_crypto/chtls/chtls_cm.c | 2 +-
include/net/raw.h | 2 +-
kernel/sysctl.c | 12 ++++++------
net/ipv4/af_inet.c | 4 ++--
net/ipv4/fib_semantics.c | 2 +-
net/ipv4/icmp.c | 15 ++++++++-------
net/ipv4/inet_timewait_sock.c | 3 ++-
net/ipv4/nexthop.c | 5 +++--
net/ipv4/syncookies.c | 2 +-
net/ipv4/sysctl_net_ipv4.c | 12 ++++++++++++
net/ipv4/tcp_input.c | 2 +-
net/ipv4/tcp_output.c | 4 ++--
net/ipv6/icmp.c | 2 +-
net/ipv6/route.c | 2 +-
15 files changed, 43 insertions(+), 28 deletions(-)

--
2.30.2


2022-07-12 01:12:42

by Kuniyuki Iwashima

[permalink] [raw]
Subject: [PATCH v1 net 15/15] nexthop: Fix data-races around nexthop_compat_mode.

While reading nexthop_compat_mode, it can be changed concurrently.
Thus, we need to add READ_ONCE() to its readers.

Fixes: 4f80116d3df3 ("net: ipv4: add sysctl for nexthop api compatibility mode")
Signed-off-by: Kuniyuki Iwashima <[email protected]>
---
CC: Roopa Prabhu <[email protected]>
---
net/ipv4/fib_semantics.c | 2 +-
net/ipv4/nexthop.c | 5 +++--
net/ipv6/route.c | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index a57ba23571c9..16dbd5075284 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1811,7 +1811,7 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
goto nla_put_failure;
if (nexthop_is_blackhole(fi->nh))
rtm->rtm_type = RTN_BLACKHOLE;
- if (!fi->fib_net->ipv4.sysctl_nexthop_compat_mode)
+ if (!READ_ONCE(fi->fib_net->ipv4.sysctl_nexthop_compat_mode))
goto offload;
}

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index e459a391e607..853a75a8fbaf 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -1858,7 +1858,7 @@ static void __remove_nexthop_fib(struct net *net, struct nexthop *nh)
/* __ip6_del_rt does a release, so do a hold here */
fib6_info_hold(f6i);
ipv6_stub->ip6_del_rt(net, f6i,
- !net->ipv4.sysctl_nexthop_compat_mode);
+ !READ_ONCE(net->ipv4.sysctl_nexthop_compat_mode));
}
}

@@ -2361,7 +2361,8 @@ static int insert_nexthop(struct net *net, struct nexthop *new_nh,
if (!rc) {
nh_base_seq_inc(net);
nexthop_notify(RTM_NEWNEXTHOP, new_nh, &cfg->nlinfo);
- if (replace_notify && net->ipv4.sysctl_nexthop_compat_mode)
+ if (replace_notify &&
+ READ_ONCE(net->ipv4.sysctl_nexthop_compat_mode))
nexthop_replace_notify(net, new_nh, &cfg->nlinfo);
}

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 828355710c57..916417944ec8 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -5741,7 +5741,7 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb,
if (nexthop_is_blackhole(rt->nh))
rtm->rtm_type = RTN_BLACKHOLE;

- if (net->ipv4.sysctl_nexthop_compat_mode &&
+ if (READ_ONCE(net->ipv4.sysctl_nexthop_compat_mode) &&
rt6_fill_node_nexthop(skb, rt->nh, &nh_flags) < 0)
goto nla_put_failure;

--
2.30.2

2022-07-12 01:13:10

by Kuniyuki Iwashima

[permalink] [raw]
Subject: [PATCH v1 net 06/15] icmp: Fix a data-race around sysctl_icmp_echo_ignore_broadcasts.

While reading sysctl_icmp_echo_ignore_broadcasts, it can be changed
concurrently. Thus, we need to add READ_ONCE() to its reader.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kuniyuki Iwashima <[email protected]>
---
net/ipv4/icmp.c | 2 +-
net/ipv4/sysctl_net_ipv4.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index c406e324c594..4b1d84d3feaa 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -1249,7 +1249,7 @@ int icmp_rcv(struct sk_buff *skb)
*/
if ((icmph->type == ICMP_ECHO ||
icmph->type == ICMP_TIMESTAMP) &&
- net->ipv4.sysctl_icmp_echo_ignore_broadcasts) {
+ READ_ONCE(net->ipv4.sysctl_icmp_echo_ignore_broadcasts)) {
reason = SKB_DROP_REASON_INVALID_PROTO;
goto error;
}
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 64682a01914a..cb53476a292e 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -617,6 +617,8 @@ static struct ctl_table ipv4_net_table[] = {
.maxlen = sizeof(u8),
.mode = 0644,
.proc_handler = proc_dou8vec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE
},
{
.procname = "icmp_ignore_bogus_error_responses",
--
2.30.2

2022-07-12 01:13:11

by Kuniyuki Iwashima

[permalink] [raw]
Subject: [PATCH v1 net 02/15] sysctl: Fix data-races in proc_dointvec_ms_jiffies().

A sysctl variable is accessed concurrently, and there is always a chance
of data-race. So, all readers and writers need some basic protection to
avoid load/store-tearing.

This patch changes proc_dointvec_ms_jiffies() to use READ_ONCE() and
WRITE_ONCE() internally to fix data-races on the sysctl side. For now,
proc_dointvec_ms_jiffies() itself is tolerant to a data-race, but we still
need to add annotations on the other subsystem's side.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kuniyuki Iwashima <[email protected]>
---
kernel/sysctl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index b016d68da08a..d99bc3945445 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1224,9 +1224,9 @@ static int do_proc_dointvec_ms_jiffies_conv(bool *negp, unsigned long *lvalp,

if (jif > INT_MAX)
return 1;
- *valp = (int)jif;
+ WRITE_ONCE(*valp, (int)jif);
} else {
- int val = *valp;
+ int val = READ_ONCE(*valp);
unsigned long lval;
if (val < 0) {
*negp = true;
@@ -1294,8 +1294,8 @@ int proc_dointvec_userhz_jiffies(struct ctl_table *table, int write,
* @ppos: the current position in the file
*
* Reads/writes up to table->maxlen/sizeof(unsigned int) integer
- * values from/to the user buffer, treated as an ASCII string.
- * The values read are assumed to be in 1/1000 seconds, and
+ * values from/to the user buffer, treated as an ASCII string.
+ * The values read are assumed to be in 1/1000 seconds, and
* are converted into jiffies.
*
* Returns 0 on success.
--
2.30.2

2022-07-12 01:15:14

by Kuniyuki Iwashima

[permalink] [raw]
Subject: [PATCH v1 net 09/15] icmp: Fix a data-race around sysctl_icmp_ratelimit.

While reading sysctl_icmp_ratelimit, it can be changed concurrently.
Thus, we need to add READ_ONCE() to its reader.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kuniyuki Iwashima <[email protected]>
---
net/ipv4/icmp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index a49b68d6b969..1d454cc41713 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -320,7 +320,8 @@ static bool icmpv4_xrlim_allow(struct net *net, struct rtable *rt,

vif = l3mdev_master_ifindex(dst->dev);
peer = inet_getpeer_v4(net->ipv4.peers, fl4->daddr, vif, 1);
- rc = inet_peer_xrlim_allow(peer, net->ipv4.sysctl_icmp_ratelimit);
+ rc = inet_peer_xrlim_allow(peer,
+ READ_ONCE(net->ipv4.sysctl_icmp_ratelimit));
if (peer)
inet_putpeer(peer);
out:
--
2.30.2

2022-07-13 12:36:23

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH v1 net 00/15] sysctl: Fix data-races around ipv4_net_table (Round 1).

Hello:

This series was applied to netdev/net.git (master)
by David S. Miller <[email protected]>:

On Mon, 11 Jul 2022 17:15:18 -0700 you wrote:
> This series fixes data-races around the first 13 knobs and
> nexthop_compat_mode in ipv4_net_table.
>
> I will post another patch for three early_demux knobs later,
> so the next round will start from ip_default_ttl.
>
>
> [...]

Here is the summary with links:
- [v1,net,01/15] sysctl: Fix data-races in proc_dou8vec_minmax().
https://git.kernel.org/netdev/net/c/7dee5d7747a6
- [v1,net,02/15] sysctl: Fix data-races in proc_dointvec_ms_jiffies().
https://git.kernel.org/netdev/net/c/7d1025e55978
- [v1,net,03/15] tcp: Fix a data-race around sysctl_max_tw_buckets.
https://git.kernel.org/netdev/net/c/6f605b57f378
- [v1,net,04/15] icmp: Fix a data-race around sysctl_icmp_echo_ignore_all.
https://git.kernel.org/netdev/net/c/bb7bb35a63b4
- [v1,net,05/15] icmp: Fix data-races around sysctl_icmp_echo_enable_probe.
https://git.kernel.org/netdev/net/c/4a2f7083cc6c
- [v1,net,06/15] icmp: Fix a data-race around sysctl_icmp_echo_ignore_broadcasts.
https://git.kernel.org/netdev/net/c/66484bb98ed2
- [v1,net,07/15] icmp: Fix a data-race around sysctl_icmp_ignore_bogus_error_responses.
https://git.kernel.org/netdev/net/c/b04f9b7e85c7
- [v1,net,08/15] icmp: Fix a data-race around sysctl_icmp_errors_use_inbound_ifaddr.
https://git.kernel.org/netdev/net/c/d2efabce81db
- [v1,net,09/15] icmp: Fix a data-race around sysctl_icmp_ratelimit.
https://git.kernel.org/netdev/net/c/2a4eb714841f
- [v1,net,10/15] icmp: Fix a data-race around sysctl_icmp_ratemask.
https://git.kernel.org/netdev/net/c/1ebcb25ad6fc
- [v1,net,11/15] raw: Fix a data-race around sysctl_raw_l3mdev_accept.
https://git.kernel.org/netdev/net/c/1dace014928e
- [v1,net,12/15] tcp: Fix data-races around sysctl_tcp_ecn.
https://git.kernel.org/netdev/net/c/4785a66702f0
- [v1,net,13/15] tcp: Fix a data-race around sysctl_tcp_ecn_fallback.
https://git.kernel.org/netdev/net/c/12b8d9ca7e67
- [v1,net,14/15] ipv4: Fix data-races around sysctl_ip_dynaddr.
https://git.kernel.org/netdev/net/c/e49e4aff7ec1
- [v1,net,15/15] nexthop: Fix data-races around nexthop_compat_mode.
https://git.kernel.org/netdev/net/c/bdf00bf24bef

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html