[PATCH net-next 00/14] net: annotate data-races around sysctl knobs

[PATCH net-next 00/14] net: annotate data-races around sysctl knobs

[Jiayuan Chen]

While testing the recent sysctl range fixes I noticed that a lot of our
sysctl knobs are read with plain loads while the handlers update them
with WRITE_ONCE() and no lock, so KCSAN reports them as data races. I
then used an LLM to go through every ctl_table under net/ and list the
readers that were left behind. This series annotates them with
READ_ONCE(), and adds WRITE_ONCE() to a few custom handlers and netlink
paths that store to the same fields with plain writes. Most of it is a
plain READ_ONCE() at each read; where a function reads the same knob
twice to make one decision, it now reads it once into a local, so the
comparison and the value it acts on cannot come from two different
writes.

There are no Fixes tags. Done one knob per patch this would be well over
a hundred commits, so I grouped them by subsystem and feature instead to
keep the review manageable. If maintainers prefer, I can split it up
further with a Fixes tag on each one, as several smaller series.

No functional change. KCSAN does hit these, e.g. toggling
net.ipv6.icmp.echo_ignore_all while receiving ICMPv6 echo requests:

  BUG: KCSAN: data-race in icmpv6_rcv / proc_dou8vec_minmax

  write (marked) to 0xffffffff8ee2940d of 1 bytes by task 504 on cpu 0:
   proc_dou8vec_minmax+0x1ab/0x1f0
   proc_sys_call_handler+0x282/0x3a0
   proc_sys_write+0x13/0x20
   ...

  read to 0xffffffff8ee2940d of 1 bytes by interrupt on cpu 3:
   icmpv6_rcv+0x99d/0xd40
   ip6_protocol_deliver_rcu+0x787/0x9a0
   ip6_input_finish+0xb0/0x1b0
   ip6_input+0xa8/0x250
   ipv6_rcv+0x14b/0x1c0
   ...

  value changed: 0x01 -> 0x00

Jiayuan Chen (14):
  ipv4: annotate data-races around ip_rt_* sysctls
  inet: frags: annotate data-races around fqdir->timeout and
    fqdir->max_dist
  ipv6: annotate data-races around addrconf sysctls
  ipv6: annotate data-races around icmpv6 reply sysctls
  calipso: annotate data-races around cache sysctls
  sctp: annotate data-races around sctp sysctls
  mptcp: annotate data-races around pernet sysctls
  rds: annotate data-races around core rds sysctls
  netfilter: conntrack: annotate data-races around conntrack sysctls
  netfilter: bridge: annotate data-races around br_netfilter and nf_log
    sysctls
  ipvs: annotate data-races around lblc_expiration and lblcr_expiration
  net/smc: annotate data-races around smc sysctls
  netfilter: conntrack: annotate data-races around timeout and tcp
    sysctls
  netfilter: cttimeout: annotate data-races around the default timeouts

 include/net/ipv6.h                            |   2 +-
 include/net/netfilter/nf_conntrack_acct.h     |   6 +-
 .../net/netfilter/nf_conntrack_timestamp.h    |   2 +-
 net/bridge/br_netfilter_hooks.c               |  20 ++--
 net/bridge/netfilter/ebt_log.c                |   2 +-
 net/ipv4/inet_fragment.c                      |   2 +-
 net/ipv4/ip_fragment.c                        |   6 +-
 net/ipv4/route.c                              |  34 +++---
 net/ipv6/addrconf.c                           |  22 ++--
 net/ipv6/calipso.c                            |   6 +-
 net/ipv6/icmp.c                               |  10 +-
 net/ipv6/ndisc.c                              |   2 +-
 net/ipv6/route.c                              |   8 +-
 net/mptcp/ctrl.c                              |  18 +--
 net/netfilter/ipvs/ip_vs_lblc.c               |   2 +-
 net/netfilter/ipvs/ip_vs_lblcr.c              |   2 +-
 net/netfilter/nf_conntrack_core.c             |   8 +-
 net/netfilter/nf_conntrack_ecache.c           |   2 +-
 net/netfilter/nf_conntrack_expect.c           |   2 +-
 net/netfilter/nf_conntrack_netlink.c          |   2 +-
 net/netfilter/nf_conntrack_proto.c            |   6 +-
 net/netfilter/nf_conntrack_proto_generic.c    |   8 +-
 net/netfilter/nf_conntrack_proto_gre.c        |  20 ++--
 net/netfilter/nf_conntrack_proto_icmp.c       |  12 +-
 net/netfilter/nf_conntrack_proto_icmpv6.c     |  12 +-
 net/netfilter/nf_conntrack_proto_sctp.c       |  13 ++-
 net/netfilter/nf_conntrack_proto_tcp.c        | 110 +++++++++---------
 net/netfilter/nf_conntrack_proto_udp.c        |  24 ++--
 net/netfilter/nf_flow_table_core.c            |   4 +-
 net/netfilter/nf_log_syslog.c                 |   2 +-
 net/rds/recv.c                                |   2 +-
 net/rds/send.c                                |   8 +-
 net/rds/threads.c                             |   4 +-
 net/sctp/associola.c                          |   2 +-
 net/sctp/bind_addr.c                          |   2 +-
 net/sctp/endpointola.c                        |  14 +--
 net/sctp/input.c                              |   2 +-
 net/sctp/sm_make_chunk.c                      |   6 +-
 net/sctp/sm_sideeffect.c                      |   2 +-
 net/sctp/sm_statefuns.c                       |   6 +-
 net/sctp/socket.c                             |  46 ++++----
 net/sctp/sysctl.c                             |  16 +--
 net/sctp/transport.c                          |   6 +-
 net/smc/af_smc.c                              |   8 +-
 net/smc/smc_clc.c                             |   8 +-
 net/smc/smc_core.c                            |   2 +-
 net/smc/smc_tx.c                              |   2 +-
 47 files changed, 259 insertions(+), 246 deletions(-)

-- 
2.43.0

[PATCH net-next 01/14] ipv4: annotate data-races around ip_rt_* sysctls

[Jiayuan Chen]

ip_rt_redirect_number, ip_rt_redirect_load, ip_rt_redirect_silence,
ip_rt_error_cost, ip_rt_error_burst, ip_rt_gc_timeout, ip_rt_min_pmtu,
ip_rt_mtu_expires and ip_rt_min_advmss are read without any lock while
their sysctl handlers update them with WRITE_ONCE(). Add READ_ONCE() to
the readers.

KCSAN reports it when net.ipv4.route.min_adv_mss is changed while TCP
connections are being opened:

  BUG: KCSAN: data-race in do_proc_int_conv / ipv4_default_advmss

  write (marked) to 0xffffffffb0828f70 of 4 bytes by task 715 on cpu 0:
   do_proc_int_conv+0xa9/0xd0
   proc_vec_conv+0x21/0x40
   proc_vec+0x21e/0x6f0
   proc_dointvec+0x16/0x30
   proc_sys_call_handler+0x282/0x3a0
   proc_sys_write+0x13/0x20
   ...

  read to 0xffffffffb0828f70 of 4 bytes by task 719 on cpu 1:
   ipv4_default_advmss+0x98/0x2c0
   __tcp_transmit_skb+0x1df4/0x28f0
   tcp_connect+0x266b/0x3c30
   tcp_v4_connect+0x9a1/0xae0
   __inet_stream_connect+0x1cc/0x760
   inet_stream_connect+0x49/0x80
   ...

  value changed: 0x00000100 -> 0x00000101

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/ipv4/route.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index d0a5f6d77094c..83f49b2d4c36a 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -829,7 +829,7 @@ static void __ip_do_redirect(struct rtable *rt, struct sk_buff *skb, struct flow
 				nhc = FIB_RES_NHC(res);
 				update_or_create_fnhe(nhc, fl4->daddr, new_gw,
 						0, false,
-						jiffies + ip_rt_gc_timeout);
+						jiffies + READ_ONCE(ip_rt_gc_timeout));
 			}
 			if (kill_route)
 				WRITE_ONCE(rt->dst.obsolete, DST_OBSOLETE_KILL);
@@ -930,7 +930,7 @@ void ip_rt_send_redirect(struct sk_buff *skb)
 	/* No redirected packets during ip_rt_redirect_silence;
 	 * reset the algorithm.
 	 */
-	if (time_after(jiffies, peer->rate_last + ip_rt_redirect_silence)) {
+	if (time_after(jiffies, peer->rate_last + READ_ONCE(ip_rt_redirect_silence))) {
 		peer->rate_tokens = 0;
 		peer->n_redirects = 0;
 	}
@@ -938,7 +938,7 @@ void ip_rt_send_redirect(struct sk_buff *skb)
 	/* Too many ignored redirects; do not send anything
 	 * set dst.rate_last to the last seen redirected packet.
 	 */
-	if (peer->n_redirects >= ip_rt_redirect_number) {
+	if (peer->n_redirects >= READ_ONCE(ip_rt_redirect_number)) {
 		peer->rate_last = jiffies;
 		goto out_unlock;
 	}
@@ -946,7 +946,7 @@ void ip_rt_send_redirect(struct sk_buff *skb)
 	/* Check for load limit; set rate_last to the latest sent
 	 * redirect.
 	 */
-	backoff = (u64)ip_rt_redirect_load << min(peer->n_redirects, 31U);
+	backoff = (u64)READ_ONCE(ip_rt_redirect_load) << min(peer->n_redirects, 31U);
 	backoff = min_t(u64, backoff, MAX_JIFFY_OFFSET);
 	if (peer->n_redirects == 0 ||
 	    time_after(jiffies, peer->rate_last + (unsigned long)backoff)) {
@@ -956,7 +956,7 @@ void ip_rt_send_redirect(struct sk_buff *skb)
 		peer->rate_last = jiffies;
 		++peer->n_redirects;
 		if (IS_ENABLED(CONFIG_IP_ROUTE_VERBOSE) && log_martians &&
-		    peer->n_redirects == ip_rt_redirect_number)
+		    peer->n_redirects == READ_ONCE(ip_rt_redirect_number))
 			net_warn_ratelimited("host %pI4/if%d ignores redirects for %pI4 to %pI4\n",
 					     &ip_hdr(skb)->saddr, inet_iif(skb),
 					     &ip_hdr(skb)->daddr, &gw);
@@ -1027,13 +1027,16 @@ static int ip_error(struct sk_buff *skb)
 			       l3mdev_master_ifindex_rcu(skb->dev));
 	send = true;
 	if (peer) {
+		int burst = READ_ONCE(ip_rt_error_burst);
+		int cost = READ_ONCE(ip_rt_error_cost);
+
 		now = jiffies;
 		peer->rate_tokens += now - peer->rate_last;
-		if (peer->rate_tokens > ip_rt_error_burst)
-			peer->rate_tokens = ip_rt_error_burst;
+		if (peer->rate_tokens > burst)
+			peer->rate_tokens = burst;
 		peer->rate_last = now;
-		if (peer->rate_tokens >= ip_rt_error_cost)
-			peer->rate_tokens -= ip_rt_error_cost;
+		if (peer->rate_tokens >= cost)
+			peer->rate_tokens -= cost;
 		else
 			send = false;
 	}
@@ -1063,14 +1066,14 @@ static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32 mtu)
 
 	rcu_read_lock();
 	net = dst_dev_net_rcu(dst);
-	if (mtu < net->ipv4.ip_rt_min_pmtu) {
+	if (mtu < READ_ONCE(net->ipv4.ip_rt_min_pmtu)) {
 		lock = true;
-		mtu = min(old_mtu, net->ipv4.ip_rt_min_pmtu);
+		mtu = min(old_mtu, READ_ONCE(net->ipv4.ip_rt_min_pmtu));
 	}
 
 	if (rt->rt_pmtu == mtu && !lock &&
 	    time_before(jiffies, READ_ONCE(dst->expires) -
-				 net->ipv4.ip_rt_mtu_expires / 2))
+				 READ_ONCE(net->ipv4.ip_rt_mtu_expires) / 2))
 		goto out;
 
 	if (fib_lookup(net, fl4, &res, 0) == 0) {
@@ -1086,14 +1089,15 @@ static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32 mtu)
 				if (!nhc)
 					break;
 				update_or_create_fnhe(nhc, fl4->daddr, 0, mtu, lock,
-						      jiffies + net->ipv4.ip_rt_mtu_expires);
+						      jiffies +
+						      READ_ONCE(net->ipv4.ip_rt_mtu_expires));
 			}
 			goto out;
 		}
 #endif /* CONFIG_IP_ROUTE_MULTIPATH */
 		nhc = FIB_RES_NHC(res);
 		update_or_create_fnhe(nhc, fl4->daddr, 0, mtu, lock,
-				      jiffies + net->ipv4.ip_rt_mtu_expires);
+				      jiffies + READ_ONCE(net->ipv4.ip_rt_mtu_expires));
 	}
 out:
 	rcu_read_unlock();
@@ -1367,7 +1371,7 @@ static unsigned int ipv4_default_advmss(const struct dst_entry *dst)
 	rcu_read_lock();
 	net = dst_dev_net_rcu(dst);
 	advmss = max_t(unsigned int, ipv4_mtu(dst) - header_size,
-				   net->ipv4.ip_rt_min_advmss);
+				   READ_ONCE(net->ipv4.ip_rt_min_advmss));
 	rcu_read_unlock();
 
 	return min(advmss, IPV4_MAX_PMTU - header_size);
-- 
2.43.0

[PATCH net-next 02/14] inet: frags: annotate data-races around fqdir->timeout and fqdir->max_dist

[Jiayuan Chen]

fqdir->timeout and fqdir->max_dist are read without any lock while their
sysctl handlers update them with WRITE_ONCE(). Add READ_ONCE() to the
readers.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/ipv4/inet_fragment.c | 2 +-
 net/ipv4/ip_fragment.c   | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index 0b8d22ffafca6..ba10aec88cc9b 100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -415,7 +415,7 @@ static struct inet_frag_queue *inet_frag_create(struct fqdir *fqdir,
 		inet_frag_putn(q, 2);
 		return NULL;
 	}
-	mod_timer(&q->timer, jiffies + fqdir->timeout);
+	mod_timer(&q->timer, jiffies + READ_ONCE(fqdir->timeout));
 	spin_unlock_bh(&q->lock);
 	return q;
 }
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 36105dd192823..e70ef3fe5e331 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -89,7 +89,7 @@ static void ip4_frag_init(struct inet_frag_queue *q, const void *a)
 
 	q->key.v4 = *key;
 	qp->ecn = 0;
-	if (q->fqdir->max_dist) {
+	if (READ_ONCE(q->fqdir->max_dist)) {
 		rcu_read_lock();
 		p = inet_getpeer_v4(net->ipv4.peers, key->saddr, key->vif);
 		if (p && !refcount_inc_not_zero(&p->refcnt))
@@ -220,7 +220,7 @@ static struct ipq *ip_find(struct net *net, struct iphdr *iph,
 static int ip_frag_too_far(struct ipq *qp)
 {
 	struct inet_peer *peer = qp->peer;
-	unsigned int max = qp->q.fqdir->max_dist;
+	unsigned int max = READ_ONCE(qp->q.fqdir->max_dist);
 	unsigned int start, end;
 
 	int rc;
@@ -242,7 +242,7 @@ static int ip_frag_too_far(struct ipq *qp)
 
 static int ip_frag_reinit(struct ipq *qp)
 {
-	if (!mod_timer_pending(&qp->q.timer, jiffies + qp->q.fqdir->timeout))
+	if (!mod_timer_pending(&qp->q.timer, jiffies + READ_ONCE(qp->q.fqdir->timeout)))
 		return -ETIMEDOUT;
 
 	inet_frag_queue_flush(&qp->q, SKB_DROP_REASON_FRAG_TOO_FAR);
-- 
2.43.0

[PATCH net-next 03/14] ipv6: annotate data-races around addrconf sysctls

[Jiayuan Chen]

The forwarding setting of conf/all, the per-device autoconf, accept_ra,
accept_ra_min_lft, addr_gen_mode, ra_defrtr_metric, disable_ipv6 and mtu6
settings, and the per-netns idgen_retries and idgen_delay, can be read
concurrently with the sysctl handlers that update them with WRITE_ONCE().
Add READ_ONCE() to the readers.

KCSAN reports it when net.ipv6.conf.<dev>.autoconf is changed while
Router Advertisements are received on the device:

  BUG: KCSAN: data-race in addrconf_prefix_rcv / do_proc_int_conv

  write (marked) to 0xffff8d7250116df4 of 4 bytes by task 767 on cpu 2:
   do_proc_int_conv+0xa9/0xd0
   proc_vec_conv+0x21/0x40
   proc_vec+0x21e/0x6f0
   proc_dointvec+0x16/0x30
   proc_sys_call_handler+0x282/0x3a0
   proc_sys_write+0x13/0x20
   ...

  read to 0xffff8d7250116df4 of 4 bytes by interrupt on cpu 1:
   addrconf_prefix_rcv+0x474/0xf00
   ndisc_router_discovery+0xf4f/0x1f80
   ndisc_rcv+0x2a2/0x3c0
   icmpv6_rcv+0xb46/0xd40
   ip6_protocol_deliver_rcu+0x787/0x9a0
   ip6_input_finish+0xb0/0x1b0
   ip6_input+0xa8/0x250
   ip6_mc_input+0x249/0x780
   ipv6_rcv+0x1b1/0x1c0
   __netif_receive_skb_one_core+0x8f/0xe0
   ...

  value changed: 0x00000001 -> 0x00000000

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/ipv6/addrconf.c | 22 ++++++++++++----------
 net/ipv6/ndisc.c    |  2 +-
 net/ipv6/route.c    |  8 ++++----
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d9d13e78bf13b..40f94124641d4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1104,7 +1104,7 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
 		goto out;
 	}
 
-	if (idev->cnf.disable_ipv6) {
+	if (READ_ONCE(idev->cnf.disable_ipv6)) {
 		NL_SET_ERR_MSG_MOD(extack, "IPv6 is disabled on this device");
 		err = -EACCES;
 		goto out;
@@ -2215,7 +2215,7 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
 			.scope = ifp->scope,
 		};
 
-		if (retries > net->ipv6.sysctl.idgen_retries) {
+		if (retries > READ_ONCE(net->ipv6.sysctl.idgen_retries)) {
 			net_info_ratelimited("%s: privacy stable address generation failed because of DAD conflicts!\n",
 					     ifp->idev->dev->name);
 			goto errdad;
@@ -2248,7 +2248,7 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
 		spin_unlock_bh(&ifp2->lock);
 
 		if (start_dad)
-			addrconf_mod_dad_work(ifp2, net->ipv6.sysctl.idgen_delay);
+			addrconf_mod_dad_work(ifp2, READ_ONCE(net->ipv6.sysctl.idgen_delay));
 		in6_ifa_put(ifp2);
 lock_errdad:
 		spin_lock_bh(&ifp->lock);
@@ -2706,8 +2706,10 @@ static void manage_tempaddrs(struct inet6_dev *idev,
 
 static bool is_addr_mode_generate_stable(struct inet6_dev *idev)
 {
-	return idev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_STABLE_PRIVACY ||
-	       idev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
+	u32 mode = READ_ONCE(idev->cnf.addr_gen_mode);
+
+	return mode == IN6_ADDR_GEN_MODE_STABLE_PRIVACY ||
+	       mode == IN6_ADDR_GEN_MODE_RANDOM;
 }
 
 int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
@@ -2735,7 +2737,7 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
 #ifdef CONFIG_IPV6_OPTIMISTIC_DAD
 		if ((READ_ONCE(net->ipv6.devconf_all->optimistic_dad) ||
 		     READ_ONCE(in6_dev->cnf.optimistic_dad)) &&
-		    !net->ipv6.devconf_all->forwarding && sllao)
+		    !READ_ONCE(net->ipv6.devconf_all->forwarding) && sllao)
 			cfg.ifa_flags |= IFA_F_OPTIMISTIC;
 #endif
 
@@ -2857,7 +2859,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 		return;
 	}
 
-	if (valid_lft != 0 && valid_lft < in6_dev->cnf.accept_ra_min_lft)
+	if (valid_lft != 0 && valid_lft < READ_ONCE(in6_dev->cnf.accept_ra_min_lft))
 		goto put;
 
 	/*
@@ -2928,7 +2930,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 	/* Try to figure out our local address for this prefix */
 
 	ignore_autoconf = READ_ONCE(in6_dev->cnf.ra_honor_pio_pflag) && pinfo->preferpd;
-	if (pinfo->autoconf && in6_dev->cnf.autoconf && !ignore_autoconf) {
+	if (pinfo->autoconf && READ_ONCE(in6_dev->cnf.autoconf) && !ignore_autoconf) {
 		struct in6_addr addr;
 		bool tokenized = false, dev_addr_generated = false;
 
@@ -3434,7 +3436,7 @@ static int ipv6_generate_stable_address(struct in6_addr *address,
 
 	if (ipv6_reserved_interfaceid(temp)) {
 		dad_count++;
-		if (dad_count > dev_net(idev->dev)->ipv6.sysctl.idgen_retries)
+		if (dad_count > READ_ONCE(dev_net(idev->dev)->ipv6.sysctl.idgen_retries))
 			return -1;
 		goto retry;
 	}
@@ -3805,7 +3807,7 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
 			 * when the interface up, the changed MTU must be
 			 * reflected in the idev as well as routers.
 			 */
-			if (idev->cnf.mtu6 != dev->mtu &&
+			if (READ_ONCE(idev->cnf.mtu6) != dev->mtu &&
 			    dev->mtu >= IPV6_MIN_MTU) {
 				rt6_mtu_change(dev, dev->mtu);
 				WRITE_ONCE(idev->cnf.mtu6, dev->mtu);
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 0ed1a619372bf..b69e6d66e8b1a 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1375,7 +1375,7 @@ static enum skb_drop_reason ndisc_router_discovery(struct sk_buff *skb)
 		}
 	}
 	/* Set default route metric as specified by user */
-	defrtr_usr_metric = in6_dev->cnf.ra_defrtr_metric;
+	defrtr_usr_metric = READ_ONCE(in6_dev->cnf.ra_defrtr_metric);
 	/* delete the route if lifetime is 0 or if metric needs change */
 	if (rt && (lifetime == 0 || rt->fib6_metric != defrtr_usr_metric)) {
 		ip6_del_rt_reason(net, rt,
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 04098cf32eabe..91a003417697c 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2065,7 +2065,7 @@ static bool rt6_mtu_change_route_allowed(struct inet6_dev *idev,
 	if (dmtu >= mtu)
 		return true;
 
-	if (dmtu == idev->cnf.mtu6)
+	if (dmtu == READ_ONCE(idev->cnf.mtu6))
 		return true;
 
 	return false;
@@ -3666,7 +3666,7 @@ int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh,
 	if (!dev)
 		goto out;
 
-	if (!idev || idev->cnf.disable_ipv6) {
+	if (!idev || READ_ONCE(idev->cnf.disable_ipv6)) {
 		NL_SET_ERR_MSG(extack, "IPv6 is disabled on nexthop device");
 		err = -EACCES;
 		goto out;
@@ -4533,7 +4533,7 @@ static void __rt6_purge_dflt_routers(struct net *net,
 		struct inet6_dev *idev = dev ? __in6_dev_get(dev) : NULL;
 
 		if (rt->fib6_flags & (RTF_DEFAULT | RTF_ADDRCONF) &&
-		    (!idev || idev->cnf.accept_ra != 2) &&
+		    (!idev || READ_ONCE(idev->cnf.accept_ra) != 2) &&
 		    fib6_info_hold_safe(rt)) {
 			rcu_read_unlock();
 			ip6_del_rt(net, rt, false);
@@ -5075,7 +5075,7 @@ static int fib6_nh_mtu_change(struct fib6_nh *nh, void *_arg)
 			return 0;
 
 		if (mtu >= arg->mtu ||
-		    (mtu < arg->mtu && mtu == idev->cnf.mtu6))
+		    (mtu < arg->mtu && mtu == READ_ONCE(idev->cnf.mtu6)))
 			fib6_metric_set(f6i, RTAX_MTU, arg->mtu);
 
 		spin_lock_bh(&rt6_exception_lock);
-- 
2.43.0

[PATCH net-next 04/14] ipv6: annotate data-races around icmpv6 reply sysctls

[Jiayuan Chen]

icmpv6_echo_ignore_all, icmpv6_echo_ignore_multicast,
icmpv6_echo_ignore_anycast, anycast_src_echo_reply and fwmark_reflect are
read without any lock while their sysctl handlers update them with
WRITE_ONCE(). Add READ_ONCE() to the readers.

KCSAN reports it when net.ipv6.icmp.echo_ignore_all is changed while
ICMPv6 echo requests are received:

  BUG: KCSAN: data-race in icmpv6_rcv / proc_dou8vec_minmax

  write (marked) to 0xffffffff8ee2940d of 1 bytes by task 504 on cpu 0:
   proc_dou8vec_minmax+0x1ab/0x1f0
   proc_sys_call_handler+0x282/0x3a0
   proc_sys_write+0x13/0x20
   ...

  read to 0xffffffff8ee2940d of 1 bytes by interrupt on cpu 3:
   icmpv6_rcv+0x99d/0xd40
   ip6_protocol_deliver_rcu+0x787/0x9a0
   ip6_input_finish+0xb0/0x1b0
   ip6_input+0xa8/0x250
   ipv6_rcv+0x14b/0x1c0
   __netif_receive_skb_one_core+0x8f/0xe0
   ...

  value changed: 0x01 -> 0x00

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 include/net/ipv6.h |  2 +-
 net/ipv6/icmp.c    | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 579df1516d251..b77dc5c3ea268 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -199,7 +199,7 @@ struct sk_buff *ip6_frag_next(struct sk_buff *skb,
 			      struct ip6_frag_state *state);
 
 #define IP6_REPLY_MARK(net, mark) \
-	((net)->ipv6.sysctl.fwmark_reflect ? (mark) : 0)
+	(READ_ONCE((net)->ipv6.sysctl.fwmark_reflect) ? (mark) : 0)
 
 #include <net/sock.h>
 
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index a95b0351824f3..3e95b1191c4f2 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -938,17 +938,17 @@ static enum skb_drop_reason icmpv6_echo_reply(struct sk_buff *skb)
 	u8 type;
 
 	if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr) &&
-	    net->ipv6.sysctl.icmpv6_echo_ignore_multicast)
+	    READ_ONCE(net->ipv6.sysctl.icmpv6_echo_ignore_multicast))
 		return reason;
 
 	saddr = &ipv6_hdr(skb)->daddr;
 
 	acast = ipv6_anycast_destination(skb_dst(skb), saddr);
-	if (acast && net->ipv6.sysctl.icmpv6_echo_ignore_anycast)
+	if (acast && READ_ONCE(net->ipv6.sysctl.icmpv6_echo_ignore_anycast))
 		return reason;
 
 	if (!ipv6_unicast_destination(skb) &&
-	    !(net->ipv6.sysctl.anycast_src_echo_reply && acast))
+	    !(READ_ONCE(net->ipv6.sysctl.anycast_src_echo_reply) && acast))
 		saddr = NULL;
 
 	if (icmph->icmp6_type == ICMPV6_EXT_ECHO_REQUEST)
@@ -1152,11 +1152,11 @@ static int icmpv6_rcv(struct sk_buff *skb)
 
 	switch (type) {
 	case ICMPV6_ECHO_REQUEST:
-		if (!net->ipv6.sysctl.icmpv6_echo_ignore_all)
+		if (!READ_ONCE(net->ipv6.sysctl.icmpv6_echo_ignore_all))
 			reason = icmpv6_echo_reply(skb);
 		break;
 	case ICMPV6_EXT_ECHO_REQUEST:
-		if (!net->ipv6.sysctl.icmpv6_echo_ignore_all &&
+		if (!READ_ONCE(net->ipv6.sysctl.icmpv6_echo_ignore_all) &&
 		    READ_ONCE(net->ipv4.sysctl_icmp_echo_enable_probe))
 			reason = icmpv6_echo_reply(skb);
 		break;
-- 
2.43.0

[PATCH net-next 05/14] calipso: annotate data-races around cache sysctls

[Jiayuan Chen]

calipso_cache_enabled and calipso_cache_bucketsize are read without any
lock while their sysctl handlers update them with WRITE_ONCE(). Add
READ_ONCE() to the readers.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/ipv6/calipso.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
index e399a42e30839..5e63212b60163 100644
--- a/net/ipv6/calipso.c
+++ b/net/ipv6/calipso.c
@@ -204,7 +204,7 @@ static int calipso_cache_check(const unsigned char *key,
 	struct calipso_map_cache_entry *prev_entry = NULL;
 	u32 hash;
 
-	if (!calipso_cache_enabled)
+	if (!READ_ONCE(calipso_cache_enabled))
 		return -ENOENT;
 
 	hash = calipso_map_cache_hash(key, key_len);
@@ -269,7 +269,7 @@ static int calipso_cache_add(const unsigned char *calipso_ptr,
 	struct calipso_map_cache_entry *old_entry = NULL;
 	u32 calipso_ptr_len;
 
-	if (!calipso_cache_enabled || calipso_cache_bucketsize <= 0)
+	if (!READ_ONCE(calipso_cache_enabled) || READ_ONCE(calipso_cache_bucketsize) <= 0)
 		return 0;
 
 	calipso_ptr_len = calipso_ptr[1];
@@ -289,7 +289,7 @@ static int calipso_cache_add(const unsigned char *calipso_ptr,
 
 	bkt = entry->hash & (CALIPSO_CACHE_BUCKETS - 1);
 	spin_lock_bh(&calipso_cache[bkt].lock);
-	if (calipso_cache[bkt].size < calipso_cache_bucketsize) {
+	if (calipso_cache[bkt].size < READ_ONCE(calipso_cache_bucketsize)) {
 		list_add(&entry->list, &calipso_cache[bkt].list);
 		calipso_cache[bkt].size += 1;
 	} else {
-- 
2.43.0

[PATCH net-next 06/14] sctp: annotate data-races around sctp sysctls

[Jiayuan Chen]

rto_min, rto_max, rto_initial, hb_interval, sack_timeout,
valid_cookie_life, max_burst, max_retrans_path, max_retrans_association,
max_retrans_init, pf_retrans, ps_retrans, pf_expose, pf_enable, udp_port,
encap_port, probe_interval, addip_enable, addip_noauth, auth_enable,
default_auto_asconf, prsctp_enable, reconf_enable, ecn_enable,
intl_enable, cookie_auth_enable, cookie_preserve_enable, sndbuf_policy,
rcvbuf_policy, rwnd_upd_shift, scope_policy and max_autoclose can be
read concurrently with the sysctl handlers that update them, and the
rto_min, rto_max, udp_port, probe_interval, auth_enable and
cookie_hmac_alg handlers store them with plain writes. Add READ_ONCE() to
the readers and WRITE_ONCE() to those handlers.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/sctp/associola.c     |  2 +-
 net/sctp/bind_addr.c     |  2 +-
 net/sctp/endpointola.c   | 14 ++++++------
 net/sctp/input.c         |  2 +-
 net/sctp/sm_make_chunk.c |  6 +++---
 net/sctp/sm_sideeffect.c |  2 +-
 net/sctp/sm_statefuns.c  |  6 +++---
 net/sctp/socket.c        | 46 +++++++++++++++++++++-------------------
 net/sctp/sysctl.c        | 16 +++++++-------
 net/sctp/transport.c     |  6 +++---
 10 files changed, 52 insertions(+), 50 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 4521be3bd85aa..0a814faaea601 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1454,7 +1454,7 @@ static inline bool sctp_peer_needs_update(struct sctp_association *asoc)
 	case SCTP_STATE_SHUTDOWN_SENT:
 		if ((asoc->rwnd > asoc->a_rwnd) &&
 		    ((asoc->rwnd - asoc->a_rwnd) >= max_t(__u32,
-			   (asoc->base.sk->sk_rcvbuf >> net->sctp.rwnd_upd_shift),
+			   (asoc->base.sk->sk_rcvbuf >> READ_ONCE(net->sctp.rwnd_upd_shift)),
 			   asoc->pathmtu)))
 			return true;
 		break;
diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
index 31737f144c7f0..36cea9ae2ad18 100644
--- a/net/sctp/bind_addr.c
+++ b/net/sctp/bind_addr.c
@@ -530,7 +530,7 @@ int sctp_in_scope(struct net *net, const union sctp_addr *addr,
 	 * Address scoping can be selectively controlled via sysctl
 	 * option
 	 */
-	switch (net->sctp.scope_policy) {
+	switch (READ_ONCE(net->sctp.scope_policy)) {
 	case SCTP_SCOPE_POLICY_DISABLE:
 		return 1;
 	case SCTP_SCOPE_POLICY_ENABLE:
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index a15b599b20b76..49d9c5c0babc1 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -54,8 +54,8 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
 	struct net *net = sock_net(sk);
 	struct sctp_shared_key *null_key;
 
-	ep->asconf_enable = net->sctp.addip_enable;
-	ep->auth_enable = net->sctp.auth_enable;
+	ep->asconf_enable = READ_ONCE(net->sctp.addip_enable);
+	ep->auth_enable = READ_ONCE(net->sctp.auth_enable);
 	if (ep->auth_enable) {
 		if (sctp_auth_init(ep, gfp))
 			goto nomem;
@@ -86,14 +86,14 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
 	INIT_LIST_HEAD(&ep->asocs);
 
 	/* Use SCTP specific send buffer space queues.  */
-	ep->sndbuf_policy = net->sctp.sndbuf_policy;
+	ep->sndbuf_policy = READ_ONCE(net->sctp.sndbuf_policy);
 
 	sk->sk_data_ready = sctp_data_ready;
 	sk->sk_write_space = sctp_write_space;
 	sock_set_flag(sk, SOCK_USE_WRITE_QUEUE);
 
 	/* Get the receive buffer policy for this endpoint */
-	ep->rcvbuf_policy = net->sctp.rcvbuf_policy;
+	ep->rcvbuf_policy = READ_ONCE(net->sctp.rcvbuf_policy);
 
 	/* Generate the cookie authentication key. */
 	gen_cookie_auth_key(&ep->cookie_auth_key);
@@ -109,9 +109,9 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
 	/* Add the null key to the endpoint shared keys list and
 	 * set the hmcas and chunks pointers.
 	 */
-	ep->prsctp_enable = net->sctp.prsctp_enable;
-	ep->reconf_enable = net->sctp.reconf_enable;
-	ep->ecn_enable = net->sctp.ecn_enable;
+	ep->prsctp_enable = READ_ONCE(net->sctp.prsctp_enable);
+	ep->reconf_enable = READ_ONCE(net->sctp.reconf_enable);
+	ep->ecn_enable = READ_ONCE(net->sctp.ecn_enable);
 
 	/* Remember who we are attached to.  */
 	ep->base.sk = sk;
diff --git a/net/sctp/input.c b/net/sctp/input.c
index 9494cfa51106c..ecff82c0d5539 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -1276,7 +1276,7 @@ static struct sctp_association *__sctp_rcv_walk_lookup(struct net *net,
 			break;
 
 		case SCTP_CID_ASCONF:
-			if (have_auth || net->sctp.addip_noauth)
+			if (have_auth || READ_ONCE(net->sctp.addip_noauth))
 				asoc = __sctp_rcv_asconf_lookup(
 						net, ch, laddr,
 						sctp_hdr(skb)->source,
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 90de272842512..43a757d4a69e7 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2038,7 +2038,7 @@ static int sctp_verify_ext_param(struct net *net,
 	 * only if ADD-IP is turned on and we are not backward-compatible
 	 * mode.
 	 */
-	if (net->sctp.addip_noauth)
+	if (READ_ONCE(net->sctp.addip_noauth))
 		return 1;
 
 	if (ep->asconf_enable && !have_auth && have_asconf)
@@ -2425,7 +2425,7 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
 	 * also give us an option to silently ignore the packet, which
 	 * is what we'll do here.
 	 */
-	if (!asoc->base.net->sctp.addip_noauth &&
+	if (!READ_ONCE(asoc->base.net->sctp.addip_noauth) &&
 	    (asoc->peer.asconf_capable && !asoc->peer.auth_capable)) {
 		asoc->peer.addip_disabled_mask |= (SCTP_PARAM_ADD_IP |
 						  SCTP_PARAM_DEL_IP |
@@ -2583,7 +2583,7 @@ static int sctp_process_param(struct sctp_association *asoc,
 		break;
 
 	case SCTP_PARAM_COOKIE_PRESERVATIVE:
-		if (!net->sctp.cookie_preserve_enable)
+		if (!READ_ONCE(net->sctp.cookie_preserve_enable))
 			break;
 
 		stale = ntohl(param.life->lifespan_increment);
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 35f540fb15fc1..01ee6c3dd441a 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -591,7 +591,7 @@ static void sctp_do_8_2_transport_strike(struct sctp_cmd_seq *commands,
 	 * is SCTP_ACTIVE, then mark this transport as Partially Failed,
 	 * see SCTP Quick Failover Draft, section 5.1
 	 */
-	if (asoc->base.net->sctp.pf_enable &&
+	if (READ_ONCE(asoc->base.net->sctp.pf_enable) &&
 	    transport->state == SCTP_ACTIVE &&
 	    transport->error_count < transport->pathmaxrxt &&
 	    transport->error_count > transport->pf_retrans) {
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index b2f609f177fb9..b6b95972eea9a 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -653,7 +653,7 @@ static bool sctp_auth_chunk_verify(struct net *net, struct sctp_chunk *chunk,
 	 */
 
 	/* Make sure that we and the peer are AUTH capable */
-	if (!net->sctp.auth_enable || !asoc->peer.auth_capable)
+	if (!READ_ONCE(net->sctp.auth_enable) || !asoc->peer.auth_capable)
 		return false;
 
 	/* set-up our fake chunk so that we can process it */
@@ -3944,7 +3944,7 @@ enum sctp_disposition sctp_sf_do_asconf(struct net *net,
 	 * described in [I-D.ietf-tsvwg-sctp-auth].
 	 */
 	if (!asoc->peer.asconf_capable ||
-	    (!net->sctp.addip_noauth && !chunk->auth))
+	    (!READ_ONCE(net->sctp.addip_noauth) && !chunk->auth))
 		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 
 	hdr = (struct sctp_addiphdr *)chunk->skb->data;
@@ -4087,7 +4087,7 @@ enum sctp_disposition sctp_sf_do_asconf_ack(struct net *net,
 	 * described in [I-D.ietf-tsvwg-sctp-auth].
 	 */
 	if (!asoc->peer.asconf_capable ||
-	    (!net->sctp.addip_noauth && !asconf_ack->auth))
+	    (!READ_ONCE(net->sctp.addip_noauth) && !asconf_ack->auth))
 		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 
 	addip_hdr = (struct sctp_addiphdr *)asconf_ack->skb->data;
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index b5a57f61ee6d1..715ecc25b2866 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -403,7 +403,7 @@ static void sctp_auto_asconf_init(struct sctp_sock *sp)
 {
 	struct net *net = sock_net(&sp->inet.sk);
 
-	if (net->sctp.default_auto_asconf) {
+	if (READ_ONCE(net->sctp.default_auto_asconf)) {
 		spin_lock_bh(&net->sctp.addr_wq_lock);
 		list_add_tail(&sp->auto_asconf_list, &net->sctp.auto_asconf_splist);
 		spin_unlock_bh(&net->sctp.addr_wq_lock);
@@ -2326,6 +2326,7 @@ static int sctp_setsockopt_autoclose(struct sock *sk, u32 *optval,
 {
 	struct sctp_sock *sp = sctp_sk(sk);
 	struct net *net = sock_net(sk);
+	unsigned long max_autoclose;
 
 	/* Applicable to UDP-style socket only */
 	if (sctp_style(sk, TCP))
@@ -2333,9 +2334,10 @@ static int sctp_setsockopt_autoclose(struct sock *sk, u32 *optval,
 	if (optlen != sizeof(int))
 		return -EINVAL;
 
+	max_autoclose = READ_ONCE(net->sctp.max_autoclose);
 	sp->autoclose = *optval;
-	if (sp->autoclose > net->sctp.max_autoclose)
-		sp->autoclose = net->sctp.max_autoclose;
+	if (sp->autoclose > max_autoclose)
+		sp->autoclose = max_autoclose;
 
 	return 0;
 }
@@ -4296,7 +4298,7 @@ static int sctp_setsockopt_interleaving_supported(struct sock *sk,
 	if (!asoc && p->assoc_id != SCTP_FUTURE_ASSOC && sctp_style(sk, UDP))
 		return -EINVAL;
 
-	if (!sock_net(sk)->sctp.intl_enable || !sp->frag_interleave) {
+	if (!READ_ONCE(sock_net(sk)->sctp.intl_enable) || !sp->frag_interleave) {
 		return -EPERM;
 	}
 
@@ -5098,9 +5100,9 @@ static int sctp_init_sock(struct sock *sk)
 	sp->default_timetolive = 0;
 
 	sp->default_rcv_context = 0;
-	sp->max_burst = net->sctp.max_burst;
+	sp->max_burst = READ_ONCE(net->sctp.max_burst);
 
-	sp->cookie_auth_enable = net->sctp.cookie_auth_enable;
+	sp->cookie_auth_enable = READ_ONCE(net->sctp.cookie_auth_enable);
 
 	/* Initialize default setup parameters. These parameters
 	 * can be modified with the SCTP_INITMSG socket option or
@@ -5108,24 +5110,24 @@ static int sctp_init_sock(struct sock *sk)
 	 */
 	sp->initmsg.sinit_num_ostreams   = sctp_max_outstreams;
 	sp->initmsg.sinit_max_instreams  = sctp_max_instreams;
-	sp->initmsg.sinit_max_attempts   = net->sctp.max_retrans_init;
-	sp->initmsg.sinit_max_init_timeo = net->sctp.rto_max;
+	sp->initmsg.sinit_max_attempts   = READ_ONCE(net->sctp.max_retrans_init);
+	sp->initmsg.sinit_max_init_timeo = READ_ONCE(net->sctp.rto_max);
 
 	/* Initialize default RTO related parameters.  These parameters can
 	 * be modified for with the SCTP_RTOINFO socket option.
 	 */
-	sp->rtoinfo.srto_initial = net->sctp.rto_initial;
-	sp->rtoinfo.srto_max     = net->sctp.rto_max;
-	sp->rtoinfo.srto_min     = net->sctp.rto_min;
+	sp->rtoinfo.srto_initial = READ_ONCE(net->sctp.rto_initial);
+	sp->rtoinfo.srto_max     = READ_ONCE(net->sctp.rto_max);
+	sp->rtoinfo.srto_min     = READ_ONCE(net->sctp.rto_min);
 
 	/* Initialize default association related parameters. These parameters
 	 * can be modified with the SCTP_ASSOCINFO socket option.
 	 */
-	sp->assocparams.sasoc_asocmaxrxt = net->sctp.max_retrans_association;
+	sp->assocparams.sasoc_asocmaxrxt = READ_ONCE(net->sctp.max_retrans_association);
 	sp->assocparams.sasoc_number_peer_destinations = 0;
 	sp->assocparams.sasoc_peer_rwnd = 0;
 	sp->assocparams.sasoc_local_rwnd = 0;
-	sp->assocparams.sasoc_cookie_life = net->sctp.valid_cookie_life;
+	sp->assocparams.sasoc_cookie_life = READ_ONCE(net->sctp.valid_cookie_life);
 
 	/* Initialize default event subscriptions. By default, all the
 	 * options are off.
@@ -5135,15 +5137,15 @@ static int sctp_init_sock(struct sock *sk)
 	/* Default Peer Address Parameters.  These defaults can
 	 * be modified via SCTP_PEER_ADDR_PARAMS
 	 */
-	sp->hbinterval  = net->sctp.hb_interval;
-	sp->udp_port    = htons(net->sctp.udp_port);
-	sp->encap_port  = htons(net->sctp.encap_port);
-	sp->pathmaxrxt  = net->sctp.max_retrans_path;
-	sp->pf_retrans  = net->sctp.pf_retrans;
-	sp->ps_retrans  = net->sctp.ps_retrans;
-	sp->pf_expose   = net->sctp.pf_expose;
+	sp->hbinterval  = READ_ONCE(net->sctp.hb_interval);
+	sp->udp_port    = htons(READ_ONCE(net->sctp.udp_port));
+	sp->encap_port  = htons(READ_ONCE(net->sctp.encap_port));
+	sp->pathmaxrxt  = READ_ONCE(net->sctp.max_retrans_path);
+	sp->pf_retrans  = READ_ONCE(net->sctp.pf_retrans);
+	sp->ps_retrans  = READ_ONCE(net->sctp.ps_retrans);
+	sp->pf_expose   = READ_ONCE(net->sctp.pf_expose);
 	sp->pathmtu     = 0; /* allow default discovery */
-	sp->sackdelay   = net->sctp.sack_timeout;
+	sp->sackdelay   = READ_ONCE(net->sctp.sack_timeout);
 	sp->sackfreq	= 2;
 	sp->param_flags = SPP_HB_ENABLE |
 			  SPP_PMTUD_ENABLE |
@@ -5182,7 +5184,7 @@ static int sctp_init_sock(struct sock *sk)
 	atomic_set(&sp->pd_mode, 0);
 	skb_queue_head_init(&sp->pd_lobby);
 	sp->frag_interleave = 0;
-	sp->probe_interval = net->sctp.probe_interval;
+	sp->probe_interval = READ_ONCE(net->sctp.probe_interval);
 
 	/* Create a per socket endpoint structure.  Even if we
 	 * change the data structure relationships, this may still
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index 2b94c211427de..8a84ae3272d24 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -402,11 +402,11 @@ static int proc_sctp_do_hmac_alg(const struct ctl_table *ctl, int write,
 		if (ret)
 			return ret;
 		if (!strcmp(tmp, "sha256")) {
-			net->sctp.cookie_auth_enable = 1;
+			WRITE_ONCE(net->sctp.cookie_auth_enable, 1);
 			return 0;
 		}
 		if (!strcmp(tmp, "none")) {
-			net->sctp.cookie_auth_enable = 0;
+			WRITE_ONCE(net->sctp.cookie_auth_enable, 0);
 			return 0;
 		}
 		return -EINVAL;
@@ -441,7 +441,7 @@ static int proc_sctp_do_rto_min(const struct ctl_table *ctl, int write,
 		if (new_value > max || new_value < min)
 			return -EINVAL;
 
-		net->sctp.rto_min = new_value;
+		WRITE_ONCE(net->sctp.rto_min, new_value);
 	}
 
 	return ret;
@@ -469,7 +469,7 @@ static int proc_sctp_do_rto_max(const struct ctl_table *ctl, int write,
 		if (new_value > max || new_value < min)
 			return -EINVAL;
 
-		net->sctp.rto_max = new_value;
+		WRITE_ONCE(net->sctp.rto_max, new_value);
 	}
 
 	return ret;
@@ -504,7 +504,7 @@ static int proc_sctp_do_auth(const struct ctl_table *ctl, int write,
 	if (write && ret == 0) {
 		struct sock *sk = net->sctp.ctl_sock;
 
-		net->sctp.auth_enable = new_value;
+		WRITE_ONCE(net->sctp.auth_enable, new_value);
 		/* Update the value in the control socket */
 		lock_sock(sk);
 		sctp_sk(sk)->ep->auth_enable = new_value;
@@ -541,12 +541,12 @@ static int proc_sctp_do_udp_port(const struct ctl_table *ctl, int write,
 			return -EINVAL;
 
 		mutex_lock(&sctp_sysctl_mutex);
-		net->sctp.udp_port = new_value;
+		WRITE_ONCE(net->sctp.udp_port, new_value);
 		sctp_udp_sock_stop(net);
 		if (new_value) {
 			ret = sctp_udp_sock_start(net);
 			if (ret)
-				net->sctp.udp_port = 0;
+				WRITE_ONCE(net->sctp.udp_port, 0);
 		}
 
 		/* Update the value in the control socket */
@@ -580,7 +580,7 @@ static int proc_sctp_do_probe_interval(const struct ctl_table *ctl, int write,
 		if (new_value && new_value < SCTP_PROBE_TIMER_MIN)
 			return -EINVAL;
 
-		net->sctp.probe_interval = new_value;
+		WRITE_ONCE(net->sctp.probe_interval, new_value);
 	}
 
 	return ret;
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 6ea55b9fbde45..78b87e1b7e64b 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -55,7 +55,7 @@ static void sctp_transport_init(struct net *net,
 	 * given destination transport address, set RTO to the protocol
 	 * parameter 'RTO.Initial'.
 	 */
-	peer->rto = msecs_to_jiffies(net->sctp.rto_initial);
+	peer->rto = msecs_to_jiffies(READ_ONCE(net->sctp.rto_initial));
 
 	peer->last_time_heard = 0;
 	peer->last_time_ecne_reduced = jiffies;
@@ -65,8 +65,8 @@ static void sctp_transport_init(struct net *net,
 			    SPP_SACKDELAY_ENABLE;
 
 	/* Initialize the default path max_retrans.  */
-	peer->pathmaxrxt  = net->sctp.max_retrans_path;
-	peer->pf_retrans  = net->sctp.pf_retrans;
+	peer->pathmaxrxt  = READ_ONCE(net->sctp.max_retrans_path);
+	peer->pf_retrans  = READ_ONCE(net->sctp.pf_retrans);
 
 	INIT_LIST_HEAD(&peer->transmitted);
 	INIT_LIST_HEAD(&peer->send_ready);
-- 
2.43.0

[PATCH net-next 07/14] mptcp: annotate data-races around pernet sysctls

[Jiayuan Chen]

enabled, add_addr_timeout, checksum_enabled, allow_join_initial_addr_port,
stale_loss_cnt, close_timeout, pm_type and syn_retrans_before_tcp_fallback
are read by their ctrl.c helpers without any lock while the sysctl
handlers update them with WRITE_ONCE(), and the path_manager handler
stores pm_type with a plain write. Add READ_ONCE() to the readers and
WRITE_ONCE() to that handler.

KCSAN reports it when net.mptcp.enabled is changed while MPTCP sockets
are being created:

  BUG: KCSAN: data-race in mptcp_is_enabled / proc_dou8vec_minmax

  write (marked) to 0xffff8d724248abc9 of 1 bytes by task 733 on cpu 3:
   proc_dou8vec_minmax+0x1ab/0x1f0
   proc_sys_call_handler+0x282/0x3a0
   proc_sys_write+0x13/0x20
   ...

  read to 0xffff8d724248abc9 of 1 bytes by task 735 on cpu 1:
   mptcp_is_enabled+0x33/0x50
   mptcp_init_sock+0x35/0x300
   inet_create+0x4a6/0x8e0
   ...

  value changed: 0x00 -> 0x01

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/mptcp/ctrl.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 63c5747f0f63c..164f79b9deb0e 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -50,39 +50,39 @@ static struct mptcp_pernet *mptcp_get_pernet(const struct net *net)
 
 int mptcp_is_enabled(const struct net *net)
 {
-	return mptcp_get_pernet(net)->mptcp_enabled;
+	return READ_ONCE(mptcp_get_pernet(net)->mptcp_enabled);
 }
 
 unsigned int mptcp_get_add_addr_timeout(const struct net *net)
 {
-	return mptcp_get_pernet(net)->add_addr_timeout;
+	return READ_ONCE(mptcp_get_pernet(net)->add_addr_timeout);
 }
 
 int mptcp_is_checksum_enabled(const struct net *net)
 {
-	return mptcp_get_pernet(net)->checksum_enabled;
+	return READ_ONCE(mptcp_get_pernet(net)->checksum_enabled);
 }
 
 int mptcp_allow_join_id0(const struct net *net)
 {
-	return mptcp_get_pernet(net)->allow_join_initial_addr_port;
+	return READ_ONCE(mptcp_get_pernet(net)->allow_join_initial_addr_port);
 }
 
 unsigned int mptcp_stale_loss_cnt(const struct net *net)
 {
-	return mptcp_get_pernet(net)->stale_loss_cnt;
+	return READ_ONCE(mptcp_get_pernet(net)->stale_loss_cnt);
 }
 
 unsigned int mptcp_close_timeout(const struct sock *sk)
 {
 	if (sock_flag(sk, SOCK_DEAD))
 		return TCP_TIMEWAIT_LEN;
-	return mptcp_get_pernet(sock_net(sk))->close_timeout;
+	return READ_ONCE(mptcp_get_pernet(sock_net(sk))->close_timeout);
 }
 
 int mptcp_get_pm_type(const struct net *net)
 {
-	return mptcp_get_pernet(net)->pm_type;
+	return READ_ONCE(mptcp_get_pernet(net)->pm_type);
 }
 
 const char *mptcp_get_path_manager(const struct net *net)
@@ -230,7 +230,7 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
 				pm_type = MPTCP_PM_TYPE_KERNEL;
 			else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
 				pm_type = MPTCP_PM_TYPE_USERSPACE;
-			pernet->pm_type = pm_type;
+			WRITE_ONCE(pernet->pm_type, pm_type);
 		}
 	}
 
@@ -551,7 +551,7 @@ void mptcp_active_detect_blackhole(struct sock *ssk, bool expired)
 
 	net = sock_net(ssk);
 	timeouts = inet_csk(ssk)->icsk_retransmits;
-	to_max = mptcp_get_pernet(net)->syn_retrans_before_tcp_fallback;
+	to_max = READ_ONCE(mptcp_get_pernet(net)->syn_retrans_before_tcp_fallback);
 
 	if (timeouts == to_max || (timeouts < to_max && expired)) {
 		subflow->mpc_drop = 1;
-- 
2.43.0

[PATCH net-next 08/14] rds: annotate data-races around core rds sysctls

[Jiayuan Chen]

reconnect_min_delay_ms, reconnect_max_delay_ms, max_unacked_packets,
max_unacked_bytes and ping_enable are read without any lock while their
sysctl handlers update them with WRITE_ONCE(). Add READ_ONCE() to the
readers.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/rds/recv.c    | 2 +-
 net/rds/send.c    | 8 ++++----
 net/rds/threads.c | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/rds/recv.c b/net/rds/recv.c
index 00ab60f2d9969..daccbfc8cb8a2 100644
--- a/net/rds/recv.c
+++ b/net/rds/recv.c
@@ -370,7 +370,7 @@ void rds_recv_incoming(struct rds_connection *conn, struct in6_addr *saddr,
 	}
 	cp->cp_next_rx_seq = be64_to_cpu(inc->i_hdr.h_sequence) + 1;
 
-	if (rds_sysctl_ping_enable && inc->i_hdr.h_dport == 0) {
+	if (READ_ONCE(rds_sysctl_ping_enable) && inc->i_hdr.h_dport == 0) {
 		if (inc->i_hdr.h_sport == 0) {
 			rdsdebug("ignore ping with 0 sport from %pI6c\n",
 				 saddr);
diff --git a/net/rds/send.c b/net/rds/send.c
index f38967f65af18..66e75a382bcc7 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -87,8 +87,8 @@ void rds_send_path_reset(struct rds_conn_path *cp)
 
 	cp->cp_conn->c_map_queued = 0;
 
-	cp->cp_unacked_packets = rds_sysctl_max_unacked_packets;
-	cp->cp_unacked_bytes = rds_sysctl_max_unacked_bytes;
+	cp->cp_unacked_packets = READ_ONCE(rds_sysctl_max_unacked_packets);
+	cp->cp_unacked_bytes = READ_ONCE(rds_sysctl_max_unacked_bytes);
 
 	/* Mark messages as retransmissions, and move them to the send q */
 	spin_lock_irqsave(&cp->cp_lock, flags);
@@ -380,9 +380,9 @@ int rds_send_xmit(struct rds_conn_path *cp)
 				set_bit(RDS_MSG_ACK_REQUIRED, &rm->m_flags);
 
 				cp->cp_unacked_packets =
-					rds_sysctl_max_unacked_packets;
+					READ_ONCE(rds_sysctl_max_unacked_packets);
 				cp->cp_unacked_bytes =
-					rds_sysctl_max_unacked_bytes;
+					READ_ONCE(rds_sysctl_max_unacked_bytes);
 				rds_stats_inc(s_send_ack_required);
 			} else {
 				cp->cp_unacked_bytes -= len;
diff --git a/net/rds/threads.c b/net/rds/threads.c
index 639302bab51ef..115fcff0a5b13 100644
--- a/net/rds/threads.c
+++ b/net/rds/threads.c
@@ -137,7 +137,7 @@ void rds_queue_reconnect(struct rds_conn_path *cp)
 
 	set_bit(RDS_RECONNECT_PENDING, &cp->cp_flags);
 	if (cp->cp_reconnect_jiffies == 0) {
-		cp->cp_reconnect_jiffies = rds_sysctl_reconnect_min_jiffies;
+		cp->cp_reconnect_jiffies = READ_ONCE(rds_sysctl_reconnect_min_jiffies);
 		rcu_read_lock();
 		if (!rds_destroy_pending(cp->cp_conn))
 			queue_delayed_work(cp->cp_wq, &cp->cp_conn_w, 0);
@@ -156,7 +156,7 @@ void rds_queue_reconnect(struct rds_conn_path *cp)
 	rcu_read_unlock();
 
 	cp->cp_reconnect_jiffies = min(cp->cp_reconnect_jiffies * 2,
-					rds_sysctl_reconnect_max_jiffies);
+					READ_ONCE(rds_sysctl_reconnect_max_jiffies));
 }
 
 void rds_connect_worker(struct work_struct *work)
-- 
2.43.0

[PATCH net-next 09/14] netfilter: conntrack: annotate data-races around conntrack sysctls

[Jiayuan Chen]

nf_conntrack_acct, nf_conntrack_events, nf_conntrack_timestamp,
nf_conntrack_checksum, nf_conntrack_log_invalid, nf_conntrack_max and
nf_conntrack_expect_max are read without any lock while their sysctl
handlers update them with WRITE_ONCE(), and nf_ct_set_acct() stores
sysctl_acct with a plain write. Add READ_ONCE() to the readers and
WRITE_ONCE() to nf_ct_set_acct().

KCSAN reports it when nf_conntrack_checksum is changed while packets go
through conntrack:

  BUG: KCSAN: data-race in nf_conntrack_icmpv4_error / proc_dou8vec_minmax

  write (marked) to 0xffffffff8ee29b6d of 1 bytes by task 568 on cpu 0:
   proc_dou8vec_minmax+0x1ab/0x1f0
   proc_sys_call_handler+0x282/0x3a0
   proc_sys_write+0x13/0x20
   ...

  read to 0xffffffff8ee29b6d of 1 bytes by interrupt on cpu 2:
   nf_conntrack_icmpv4_error+0xb3/0x240
   nf_conntrack_in+0xa85/0xb20
   ipv4_conntrack_local+0xb0/0x120
   nf_hook_slow+0x7e/0x180
   nf_hook+0x1fa/0x4e0
   __ip_local_out+0x1ac/0x3f0
   ip_push_pending_frames+0x65/0x160
   icmp_push_reply+0x17a/0x200
   __icmp_send+0xb7d/0x1a10
   udp_rcv+0x95c/0x14e0
   ...

  value changed: 0x00 -> 0x01

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 include/net/netfilter/nf_conntrack_acct.h      | 6 +++---
 include/net/netfilter/nf_conntrack_timestamp.h | 2 +-
 net/netfilter/nf_conntrack_core.c              | 6 +++---
 net/netfilter/nf_conntrack_ecache.c            | 2 +-
 net/netfilter/nf_conntrack_expect.c            | 2 +-
 net/netfilter/nf_conntrack_netlink.c           | 2 +-
 net/netfilter/nf_conntrack_proto.c             | 6 +++---
 net/netfilter/nf_conntrack_proto_icmp.c        | 2 +-
 net/netfilter/nf_conntrack_proto_icmpv6.c      | 2 +-
 net/netfilter/nf_conntrack_proto_sctp.c        | 2 +-
 net/netfilter/nf_conntrack_proto_tcp.c         | 2 +-
 net/netfilter/nf_conntrack_proto_udp.c         | 2 +-
 12 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_acct.h b/include/net/netfilter/nf_conntrack_acct.h
index a120685cac932..4e5a46b628831 100644
--- a/include/net/netfilter/nf_conntrack_acct.h
+++ b/include/net/netfilter/nf_conntrack_acct.h
@@ -33,7 +33,7 @@ struct nf_conn_acct *nf_ct_acct_ext_add(struct nf_conn *ct, gfp_t gfp)
 	struct net *net = nf_ct_net(ct);
 	struct nf_conn_acct *acct;
 
-	if (!net->ct.sysctl_acct)
+	if (!READ_ONCE(net->ct.sysctl_acct))
 		return NULL;
 
 	acct = nf_ct_ext_add(ct, NF_CT_EXT_ACCT, gfp);
@@ -51,7 +51,7 @@ struct nf_conn_acct *nf_ct_acct_ext_add(struct nf_conn *ct, gfp_t gfp)
 static inline bool nf_ct_acct_enabled(struct net *net)
 {
 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
-	return net->ct.sysctl_acct != 0;
+	return READ_ONCE(net->ct.sysctl_acct) != 0;
 #else
 	return false;
 #endif
@@ -61,7 +61,7 @@ static inline bool nf_ct_acct_enabled(struct net *net)
 static inline void nf_ct_set_acct(struct net *net, bool enable)
 {
 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
-	net->ct.sysctl_acct = enable;
+	WRITE_ONCE(net->ct.sysctl_acct, enable);
 #endif
 }
 
diff --git a/include/net/netfilter/nf_conntrack_timestamp.h b/include/net/netfilter/nf_conntrack_timestamp.h
index 57138d974a9f7..59751be856ce9 100644
--- a/include/net/netfilter/nf_conntrack_timestamp.h
+++ b/include/net/netfilter/nf_conntrack_timestamp.h
@@ -29,7 +29,7 @@ struct nf_conn_tstamp *nf_ct_tstamp_ext_add(struct nf_conn *ct, gfp_t gfp)
 #ifdef CONFIG_NF_CONNTRACK_TIMESTAMP
 	struct net *net = nf_ct_net(ct);
 
-	if (!net->ct.sysctl_tstamp)
+	if (!READ_ONCE(net->ct.sysctl_tstamp))
 		return NULL;
 
 	return nf_ct_ext_add(ct, NF_CT_EXT_TSTAMP, gfp);
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 931b76aacd934..5c7054753722e 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1508,7 +1508,7 @@ static void gc_worker(struct work_struct *work)
 
 	i = gc_work->next_bucket;
 	if (gc_work->early_drop)
-		nf_conntrack_max95 = nf_conntrack_max / 100u * 95u;
+		nf_conntrack_max95 = READ_ONCE(nf_conntrack_max) / 100u * 95u;
 
 	if (i == 0) {
 		gc_work->avg_timeout = GC_SCAN_INTERVAL_INIT;
@@ -1658,7 +1658,7 @@ __nf_conntrack_alloc(struct net *net,
 	/* We don't want any race condition at early drop stage */
 	ct_count = atomic_inc_return(&cnet->count);
 
-	if (unlikely(ct_count > nf_conntrack_max)) {
+	if (unlikely(ct_count > READ_ONCE(nf_conntrack_max))) {
 		if (!early_drop(net, hash)) {
 			if (!conntrack_gc_work.early_drop)
 				conntrack_gc_work.early_drop = true;
@@ -1793,7 +1793,7 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
 #ifdef CONFIG_NF_CONNTRACK_EVENTS
 	ecache = tmpl ? nf_ct_ecache_find(tmpl) : NULL;
 
-	if ((ecache || net->ct.sysctl_events) &&
+	if ((ecache || READ_ONCE(net->ct.sysctl_events)) &&
 	    !nf_ct_ecache_ext_add(ct, ecache ? ecache->ctmask : 0,
 				  ecache ? ecache->expmask : 0,
 				  GFP_ATOMIC)) {
diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
index fb731ee235d89..2846511c5318c 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -321,7 +321,7 @@ bool nf_ct_ecache_ext_add(struct nf_conn *ct, u16 ctmask, u16 expmask, gfp_t gfp
 	struct net *net = nf_ct_net(ct);
 	struct nf_conntrack_ecache *e;
 
-	switch (net->ct.sysctl_events) {
+	switch (READ_ONCE(net->ct.sysctl_events)) {
 	case 0:
 		 /* assignment via template / ruleset? ignore sysctl. */
 		if (ctmask || expmask)
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index 06242c86e1dc8..43594283bcc76 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -518,7 +518,7 @@ static inline int __nf_ct_expect_check(struct nf_conntrack_expect *expect,
 	}
 
 	cnet = nf_ct_pernet(net);
-	if (cnet->expect_count >= nf_ct_expect_max) {
+	if (cnet->expect_count >= READ_ONCE(nf_ct_expect_max)) {
 		net_warn_ratelimited("nf_conntrack: expectation table full\n");
 		ret = -EMFILE;
 	}
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 1f8d94a052403..0f1d42a10b347 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2591,7 +2591,7 @@ ctnetlink_stat_ct_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
 	if (nla_put_be32(skb, CTA_STATS_GLOBAL_ENTRIES, htonl(nr_conntracks)))
 		goto nla_put_failure;
 
-	if (nla_put_be32(skb, CTA_STATS_GLOBAL_MAX_ENTRIES, htonl(nf_conntrack_max)))
+	if (nla_put_be32(skb, CTA_STATS_GLOBAL_MAX_ENTRIES, htonl(READ_ONCE(nf_conntrack_max))))
 		goto nla_put_failure;
 
 	nlmsg_end(skb, nlh);
diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
index 7a40e4e0e33e6..83dc169f87df5 100644
--- a/net/netfilter/nf_conntrack_proto.c
+++ b/net/netfilter/nf_conntrack_proto.c
@@ -51,12 +51,12 @@ void nf_l4proto_log_invalid(const struct sk_buff *skb,
 			    u8 protonum,
 			    const char *fmt, ...)
 {
+	u8 log_invalid = READ_ONCE(state->net->ct.sysctl_log_invalid);
 	struct net *net = state->net;
 	struct va_format vaf;
 	va_list args;
 
-	if (net->ct.sysctl_log_invalid != protonum &&
-	    net->ct.sysctl_log_invalid != IPPROTO_RAW)
+	if (log_invalid != protonum && log_invalid != IPPROTO_RAW)
 		return;
 
 	va_start(args, fmt);
@@ -86,7 +86,7 @@ void nf_ct_l4proto_log_invalid(const struct sk_buff *skb,
 	lockdep_assert_not_held(&ct->lock);
 
 	net = nf_ct_net(ct);
-	if (likely(net->ct.sysctl_log_invalid == 0))
+	if (likely(READ_ONCE(net->ct.sysctl_log_invalid) == 0))
 		return;
 
 	va_start(args, fmt);
diff --git a/net/netfilter/nf_conntrack_proto_icmp.c b/net/netfilter/nf_conntrack_proto_icmp.c
index 32148a3a8509a..330c6d83d670a 100644
--- a/net/netfilter/nf_conntrack_proto_icmp.c
+++ b/net/netfilter/nf_conntrack_proto_icmp.c
@@ -217,7 +217,7 @@ int nf_conntrack_icmpv4_error(struct nf_conn *tmpl,
 	}
 
 	/* See nf_conntrack_proto_tcp.c */
-	if (state->net->ct.sysctl_checksum &&
+	if (READ_ONCE(state->net->ct.sysctl_checksum) &&
 	    state->hook == NF_INET_PRE_ROUTING &&
 	    nf_ip_checksum(skb, state->hook, dataoff, IPPROTO_ICMP)) {
 		icmp_error_log(skb, state, "bad hw icmp checksum");
diff --git a/net/netfilter/nf_conntrack_proto_icmpv6.c b/net/netfilter/nf_conntrack_proto_icmpv6.c
index e508b3aa370a7..803221781de39 100644
--- a/net/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/netfilter/nf_conntrack_proto_icmpv6.c
@@ -198,7 +198,7 @@ int nf_conntrack_icmpv6_error(struct nf_conn *tmpl,
 	}
 
 	if (state->hook == NF_INET_PRE_ROUTING &&
-	    state->net->ct.sysctl_checksum &&
+	    READ_ONCE(state->net->ct.sysctl_checksum) &&
 	    nf_ip6_checksum(skb, state->hook, dataoff, IPPROTO_ICMPV6)) {
 		icmpv6_error_log(skb, state, "ICMPv6 checksum failed");
 		return -NF_ACCEPT;
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 71cc920f68568..49ec8478746ad 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -304,7 +304,7 @@ static bool sctp_error(struct sk_buff *skb,
 		goto out_invalid;
 	}
 	if (state->hook == NF_INET_PRE_ROUTING &&
-	    state->net->ct.sysctl_checksum &&
+	    READ_ONCE(state->net->ct.sysctl_checksum) &&
 	    skb->ip_summed == CHECKSUM_NONE) {
 		if (skb_ensure_writable(skb, dataoff + sizeof(*sh))) {
 			logmsg = "nf_ct_sctp: failed to read header ";
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index ad6f1986d52a6..b859e409f0796 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -856,7 +856,7 @@ static bool tcp_error(const struct tcphdr *th,
 	 * because the checksum is assumed to be correct.
 	 */
 	/* FIXME: Source route IP option packets --RR */
-	if (state->net->ct.sysctl_checksum &&
+	if (READ_ONCE(state->net->ct.sysctl_checksum) &&
 	    state->hook == NF_INET_PRE_ROUTING &&
 	    nf_checksum(skb, state->hook, dataoff, IPPROTO_TCP, state->pf)) {
 		tcp_error_log(skb, state, "bad checksum");
diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index a7edfefd6fd14..9ea4062752238 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -82,7 +82,7 @@ static bool udp_error(struct sk_buff *skb,
 	 * because the checksum is assumed to be correct.
 	 * FIXME: Source route IP option packets --RR */
 	if (state->hook == NF_INET_PRE_ROUTING &&
-	    state->net->ct.sysctl_checksum &&
+	    READ_ONCE(state->net->ct.sysctl_checksum) &&
 	    nf_checksum(skb, state->hook, dataoff, IPPROTO_UDP, state->pf)) {
 		udp_error_log(skb, state, "bad checksum");
 		return true;
-- 
2.43.0

[PATCH net-next 10/14] netfilter: bridge: annotate data-races around br_netfilter and nf_log sysctls

[Jiayuan Chen]

bridge-nf-call-arptables, bridge-nf-call-iptables,
bridge-nf-call-ip6tables, bridge-nf-filter-vlan-tagged,
bridge-nf-filter-pppoe-tagged, bridge-nf-pass-vlan-input-dev and
nf_log_all_netns are read in the packet path without any lock while their
sysctl handlers update them, and brnf_sysctl_call_tables() normalizes the
value with a plain write. Add READ_ONCE() to the readers and WRITE_ONCE()
to that handler.

KCSAN reports it when bridge-nf-call-iptables is changed while packets
are bridged:

  BUG: KCSAN: data-race in br_nf_pre_routing / do_proc_int_conv

  write (marked) to 0xffff8d7242d8ec50 of 4 bytes by task 824 on cpu 0:
   do_proc_int_conv+0xa9/0xd0
   proc_vec_conv+0x21/0x40
   proc_vec+0x21e/0x6f0
   proc_dointvec+0x16/0x30
   brnf_sysctl_call_tables+0x19/0x70
   proc_sys_call_handler+0x282/0x3a0
   proc_sys_write+0x13/0x20
   ...

  read to 0xffff8d7242d8ec50 of 4 bytes by interrupt on cpu 1:
   br_nf_pre_routing+0x1a9/0x9d0
   br_handle_frame+0x523/0x990
   __netif_receive_skb_core.constprop.0+0x3b2/0x1cf0
   __netif_receive_skb_one_core+0x51/0xe0
   __netif_receive_skb+0x23/0xb0
   process_backlog+0x27a/0x950
   ...

  value changed: 0x00000001 -> 0x00000000

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/bridge/br_netfilter_hooks.c | 20 ++++++++++----------
 net/bridge/netfilter/ebt_log.c  |  2 +-
 net/netfilter/nf_log_syslog.c   |  2 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index e15355ff215a7..8a47fe4e8eb61 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -94,7 +94,7 @@ static inline bool is_vlan_ip(const struct sk_buff *skb, const struct net *net)
 {
 	struct brnf_net *brnet = net_generic(net, brnf_net_id);
 
-	return vlan_proto(skb) == htons(ETH_P_IP) && brnet->filter_vlan_tagged;
+	return vlan_proto(skb) == htons(ETH_P_IP) && READ_ONCE(brnet->filter_vlan_tagged);
 }
 
 static inline bool is_vlan_ipv6(const struct sk_buff *skb,
@@ -103,14 +103,14 @@ static inline bool is_vlan_ipv6(const struct sk_buff *skb,
 	struct brnf_net *brnet = net_generic(net, brnf_net_id);
 
 	return vlan_proto(skb) == htons(ETH_P_IPV6) &&
-	       brnet->filter_vlan_tagged;
+	       READ_ONCE(brnet->filter_vlan_tagged);
 }
 
 static inline bool is_vlan_arp(const struct sk_buff *skb, const struct net *net)
 {
 	struct brnf_net *brnet = net_generic(net, brnf_net_id);
 
-	return vlan_proto(skb) == htons(ETH_P_ARP) && brnet->filter_vlan_tagged;
+	return vlan_proto(skb) == htons(ETH_P_ARP) && READ_ONCE(brnet->filter_vlan_tagged);
 }
 
 static inline __be16 pppoe_proto(const struct sk_buff *skb)
@@ -124,7 +124,7 @@ static inline bool is_pppoe_ip(const struct sk_buff *skb, const struct net *net)
 	struct brnf_net *brnet = net_generic(net, brnf_net_id);
 
 	return skb->protocol == htons(ETH_P_PPP_SES) &&
-	       pppoe_proto(skb) == htons(PPP_IP) && brnet->filter_pppoe_tagged;
+	       pppoe_proto(skb) == htons(PPP_IP) && READ_ONCE(brnet->filter_pppoe_tagged);
 }
 
 static inline bool is_pppoe_ipv6(const struct sk_buff *skb,
@@ -134,7 +134,7 @@ static inline bool is_pppoe_ipv6(const struct sk_buff *skb,
 
 	return skb->protocol == htons(ETH_P_PPP_SES) &&
 	       pppoe_proto(skb) == htons(PPP_IPV6) &&
-	       brnet->filter_pppoe_tagged;
+	       READ_ONCE(brnet->filter_pppoe_tagged);
 }
 
 /* largest possible L2 header, see br_nf_dev_queue_xmit() */
@@ -442,7 +442,7 @@ static struct net_device *brnf_get_logical_dev(struct sk_buff *skb,
 
 	br = bridge_parent(dev);
 
-	if (brnet->pass_vlan_indev == 0 || !skb_vlan_tag_present(skb))
+	if (READ_ONCE(brnet->pass_vlan_indev) == 0 || !skb_vlan_tag_present(skb))
 		return br;
 
 	vlan = __vlan_find_dev_deep_rcu(br, skb->vlan_proto,
@@ -502,7 +502,7 @@ static unsigned int br_nf_pre_routing(void *priv,
 	brnet = net_generic(state->net, brnf_net_id);
 	if (IS_IPV6(skb) || is_vlan_ipv6(skb, state->net) ||
 	    is_pppoe_ipv6(skb, state->net)) {
-		if (!brnet->call_ip6tables &&
+		if (!READ_ONCE(brnet->call_ip6tables) &&
 		    !br_opt_get(br, BROPT_NF_CALL_IP6TABLES))
 			return NF_ACCEPT;
 		if (!ipv6_mod_enabled()) {
@@ -514,7 +514,7 @@ static unsigned int br_nf_pre_routing(void *priv,
 		return br_nf_pre_routing_ipv6(priv, skb, state);
 	}
 
-	if (!brnet->call_iptables && !br_opt_get(br, BROPT_NF_CALL_IPTABLES))
+	if (!READ_ONCE(brnet->call_iptables) && !br_opt_get(br, BROPT_NF_CALL_IPTABLES))
 		return NF_ACCEPT;
 
 	if (!IS_IP(skb) && !is_vlan_ip(skb, state->net) &&
@@ -740,7 +740,7 @@ static unsigned int br_nf_forward_arp(struct sk_buff *skb,
 	br = p->br;
 
 	brnet = net_generic(state->net, brnf_net_id);
-	if (!brnet->call_arptables && !br_opt_get(br, BROPT_NF_CALL_ARPTABLES))
+	if (!READ_ONCE(brnet->call_arptables) && !br_opt_get(br, BROPT_NF_CALL_ARPTABLES))
 		return NF_ACCEPT;
 
 	if (is_vlan_arp(skb, state->net))
@@ -1178,7 +1178,7 @@ int brnf_sysctl_call_tables(const struct ctl_table *ctl, int write,
 	ret = proc_dointvec(ctl, write, buffer, lenp, ppos);
 
 	if (write && *(int *)(ctl->data))
-		*(int *)(ctl->data) = 1;
+		WRITE_ONCE(*(int *)(ctl->data), 1);
 	return ret;
 }
 
diff --git a/net/bridge/netfilter/ebt_log.c b/net/bridge/netfilter/ebt_log.c
index e2eea1daaf8ba..f360414a2814e 100644
--- a/net/bridge/netfilter/ebt_log.c
+++ b/net/bridge/netfilter/ebt_log.c
@@ -79,7 +79,7 @@ ebt_log_packet(struct net *net, u_int8_t pf, unsigned int hooknum,
 	unsigned int bitmask;
 
 	/* FIXME: Disabled from containers until syslog ns is supported */
-	if (!net_eq(net, &init_net) && !sysctl_nf_log_all_netns)
+	if (!net_eq(net, &init_net) && !READ_ONCE(sysctl_nf_log_all_netns))
 		return;
 
 	spin_lock_bh(&ebt_log_lock);
diff --git a/net/netfilter/nf_log_syslog.c b/net/netfilter/nf_log_syslog.c
index c3fd398ffcd77..2907f47c83423 100644
--- a/net/netfilter/nf_log_syslog.c
+++ b/net/netfilter/nf_log_syslog.c
@@ -43,7 +43,7 @@ struct arppayload {
 /* Guard against containers flooding syslog. */
 static bool nf_log_allowed(const struct net *net)
 {
-	return net_eq(net, &init_net) || sysctl_nf_log_all_netns;
+	return net_eq(net, &init_net) || READ_ONCE(sysctl_nf_log_all_netns);
 }
 
 static void nf_log_dump_vlan(struct nf_log_buf *m, const struct sk_buff *skb)
-- 
2.43.0

[PATCH net-next 11/14] ipvs: annotate data-races around lblc_expiration and lblcr_expiration

[Jiayuan Chen]

sysctl_lblc_expiration and sysctl_lblcr_expiration are read without any
lock while their sysctl handlers update them with WRITE_ONCE(). Add
READ_ONCE() to the readers.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/netfilter/ipvs/ip_vs_lblc.c  | 2 +-
 net/netfilter/ipvs/ip_vs_lblcr.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index e1111c3b27211..21bb820e02914 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -249,7 +249,7 @@ static void ip_vs_lblc_flush(struct ip_vs_service *svc)
 static int sysctl_lblc_expiration(struct ip_vs_service *svc)
 {
 #ifdef CONFIG_SYSCTL
-	return svc->ipvs->sysctl_lblc_expiration;
+	return READ_ONCE(svc->ipvs->sysctl_lblc_expiration);
 #else
 	return DEFAULT_EXPIRATION;
 #endif
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index dfb5bf83750db..33d6e30968a95 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -413,7 +413,7 @@ static void ip_vs_lblcr_flush(struct ip_vs_service *svc)
 static int sysctl_lblcr_expiration(struct ip_vs_service *svc)
 {
 #ifdef CONFIG_SYSCTL
-	return svc->ipvs->sysctl_lblcr_expiration;
+	return READ_ONCE(svc->ipvs->sysctl_lblcr_expiration);
 #else
 	return DEFAULT_EXPIRATION;
 #endif
-- 
2.43.0

[PATCH net-next 12/14] net/smc: annotate data-races around smc sysctls

[Jiayuan Chen]

autocorking_size, smcr_buf_type, smcr_max_links_per_lgr,
smcr_max_conns_per_lgr and limit_smc_hs are read without any lock while
their sysctl handlers update them with WRITE_ONCE(), and the SMC netlink
handlers toggle limit_smc_hs with plain writes. Add READ_ONCE() to the
readers and WRITE_ONCE() to those handlers.

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/smc/af_smc.c   | 8 ++++----
 net/smc/smc_clc.c  | 8 ++++----
 net/smc/smc_core.c | 2 +-
 net/smc/smc_tx.c   | 2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index d10244af5bd29..9a657172be8f9 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -88,7 +88,7 @@ int smc_nl_dump_hs_limitation(struct sk_buff *skb, struct netlink_callback *cb)
 		return -ENOMEM;
 
 	if (nla_put_u8(skb, SMC_NLA_HS_LIMITATION_ENABLED,
-		       sock_net(skb->sk)->smc.limit_smc_hs))
+		       READ_ONCE(sock_net(skb->sk)->smc.limit_smc_hs)))
 		goto err;
 
 	genlmsg_end(skb, hdr);
@@ -102,13 +102,13 @@ int smc_nl_dump_hs_limitation(struct sk_buff *skb, struct netlink_callback *cb)
 
 int smc_nl_enable_hs_limitation(struct sk_buff *skb, struct genl_info *info)
 {
-	sock_net(skb->sk)->smc.limit_smc_hs = true;
+	WRITE_ONCE(sock_net(skb->sk)->smc.limit_smc_hs, true);
 	return 0;
 }
 
 int smc_nl_disable_hs_limitation(struct sk_buff *skb, struct genl_info *info)
 {
-	sock_net(skb->sk)->smc.limit_smc_hs = false;
+	WRITE_ONCE(sock_net(skb->sk)->smc.limit_smc_hs, false);
 	return 0;
 }
 
@@ -411,7 +411,7 @@ void smc_sk_init(struct net *net, struct sock *sk, int protocol)
 	spin_lock_init(&smc->conn.send_lock);
 	mutex_init(&smc->clcsock_release_lock);
 	smc_init_saved_callbacks(smc);
-	smc->limit_smc_hs = net->smc.limit_smc_hs;
+	smc->limit_smc_hs = READ_ONCE(net->smc.limit_smc_hs);
 	smc->use_fallback = false; /* assume rdma capability first */
 	smc->fallback_rsn = 0;
 	smc_close_init(smc);
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 014d527d5462d..df0afeef67cbd 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -990,8 +990,8 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini)
 	}
 	if (smcr_indicated(ini->smc_type_v2)) {
 		memcpy(v2_ext->roce, ini->smcrv2.ib_gid_v2, SMC_GID_SIZE);
-		v2_ext->max_conns = net->smc.sysctl_max_conns_per_lgr;
-		v2_ext->max_links = net->smc.sysctl_max_links_per_lgr;
+		v2_ext->max_conns = READ_ONCE(net->smc.sysctl_max_conns_per_lgr);
+		v2_ext->max_links = READ_ONCE(net->smc.sysctl_max_links_per_lgr);
 	}
 
 	pclc_base->hdr.length = htons(plen);
@@ -1272,12 +1272,12 @@ int smc_clc_srv_v2x_features_validate(struct smc_sock *smc,
 
 	if (ini->smcr_version & SMC_V2) {
 		ini->max_conns = min_t(u8, pclc_v2_ext->max_conns,
-				       net->smc.sysctl_max_conns_per_lgr);
+				       READ_ONCE(net->smc.sysctl_max_conns_per_lgr));
 		if (ini->max_conns < SMC_CONN_PER_LGR_MIN)
 			return SMC_CLC_DECL_MAXCONNERR;
 
 		ini->max_links = min_t(u8, pclc_v2_ext->max_links,
-				       net->smc.sysctl_max_links_per_lgr);
+				       READ_ONCE(net->smc.sysctl_max_links_per_lgr));
 		if (ini->max_links < SMC_LINKS_ADD_LNK_MIN)
 			return SMC_CLC_DECL_MAXLINKERR;
 	}
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 6021322253e93..3d338019a1b8b 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -990,7 +990,7 @@ static int smc_lgr_create(struct smc_sock *smc, struct smc_init_info *ini)
 		lgr->net = smc_ib_net(lnk->smcibdev);
 		lgr_list = &smc_lgr_list.list;
 		lgr_lock = &smc_lgr_list.lock;
-		lgr->buf_type = lgr->net->smc.sysctl_smcr_buf_type;
+		lgr->buf_type = READ_ONCE(lgr->net->smc.sysctl_smcr_buf_type);
 		atomic_inc(&lgr_cnt);
 	}
 	smc->conn.lgr = lgr;
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 0f0c6df3bf2ce..30bffe79d06a3 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -148,7 +148,7 @@ static bool smc_should_autocork(struct smc_sock *smc)
 	int corking_size;
 
 	corking_size = min_t(unsigned int, conn->sndbuf_desc->len >> 1,
-			     sock_net(&smc->sk)->smc.sysctl_autocorking_size);
+			     READ_ONCE(sock_net(&smc->sk)->smc.sysctl_autocorking_size));
 
 	if (atomic_read(&conn->cdc_pend_tx_wr) == 0 ||
 	    smc_tx_prepared_sends(conn) > corking_size)
-- 
2.43.0

[PATCH net-next 13/14] netfilter: conntrack: annotate data-races around timeout and tcp sysctls

[Jiayuan Chen]

The nf_conntrack_*_timeout* knobs, nf_flowtable_tcp_timeout,
nf_flowtable_udp_timeout and nf_conntrack_tcp_{loose,be_liberal,
max_retrans,ignore_invalid_rst} are read in the packet path without any
lock while their sysctl handlers update them with WRITE_ONCE(). Add
READ_ONCE() to the readers.

KCSAN reports it when nf_conntrack_udp_timeout is changed while UDP
traffic goes through conntrack:

  BUG: KCSAN: data-race in nf_conntrack_udp_packet / proc_int_u2k_conv_uop

  write (marked) to 0xffffffffb0829bc4 of 4 bytes by task 436 on cpu 0:
   proc_int_u2k_conv_uop+0x92/0xa0
   sysctl_u2k_int_conv_hz+0x21/0x30
   proc_int_conv+0xdf/0x130
   do_proc_int_conv_jiffies+0x1b/0x30
   proc_vec_conv+0x21/0x40
   proc_vec+0x21e/0x6f0
   proc_dointvec_conv+0x21/0x40
   proc_dointvec_jiffies+0x15/0x30
   proc_sys_call_handler+0x282/0x3a0
   proc_sys_write+0x13/0x20
   ...

  read to 0xffffffffb0829bc4 of 4 bytes by task 439 on cpu 3:
   nf_conntrack_udp_packet+0x27b/0x550
   nf_conntrack_in+0x767/0xb20
   ipv4_conntrack_local+0xb0/0x120
   nf_hook_slow+0x7e/0x180
   nf_hook+0x1fa/0x4e0
   __ip_local_out+0x1ac/0x3f0
   ip_send_skb+0x31/0x120
   udp_send_skb+0x4bf/0x640
   udp_sendmsg+0xdec/0x18a0
   ...

  value changed: 0x00001d4c -> 0x00001e46

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/netfilter/nf_conntrack_core.c         |  2 +-
 net/netfilter/nf_conntrack_proto_gre.c    |  4 +--
 net/netfilter/nf_conntrack_proto_icmp.c   |  2 +-
 net/netfilter/nf_conntrack_proto_icmpv6.c |  2 +-
 net/netfilter/nf_conntrack_proto_sctp.c   |  2 +-
 net/netfilter/nf_conntrack_proto_tcp.c    | 37 ++++++++++++-----------
 net/netfilter/nf_conntrack_proto_udp.c    |  6 ++--
 net/netfilter/nf_flow_table_core.c        |  4 +--
 8 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 5c7054753722e..8e332e407529e 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1959,7 +1959,7 @@ static int generic_packet(struct nf_conn *ct, struct sk_buff *skb,
 	if (!timeout)
 		timeout = &nf_generic_pernet(nf_ct_net(ct))->timeout;
 
-	nf_ct_refresh_acct(ct, ctinfo, skb, *timeout);
+	nf_ct_refresh_acct(ct, ctinfo, skb, READ_ONCE(*timeout));
 	return NF_ACCEPT;
 }
 
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index 616ab1e2fc5ed..d8bca1e29797b 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -279,8 +279,8 @@ int nf_conntrack_gre_packet(struct nf_conn *ct,
 
 		/* initialize to sane value.  Ideally a conntrack helper
 		 * (e.g. in case of pptp) is increasing them */
-		ct->proto.gre.stream_timeout = timeouts[GRE_CT_REPLIED];
-		ct->proto.gre.timeout = timeouts[GRE_CT_UNREPLIED];
+		ct->proto.gre.stream_timeout = READ_ONCE(timeouts[GRE_CT_REPLIED]);
+		ct->proto.gre.timeout = READ_ONCE(timeouts[GRE_CT_UNREPLIED]);
 	}
 
 	status = READ_ONCE(ct->status);
diff --git a/net/netfilter/nf_conntrack_proto_icmp.c b/net/netfilter/nf_conntrack_proto_icmp.c
index 330c6d83d670a..0f6ef0443c6fa 100644
--- a/net/netfilter/nf_conntrack_proto_icmp.c
+++ b/net/netfilter/nf_conntrack_proto_icmp.c
@@ -98,7 +98,7 @@ int nf_conntrack_icmp_packet(struct nf_conn *ct,
 	if (!timeout)
 		timeout = &nf_icmp_pernet(nf_ct_net(ct))->timeout;
 
-	nf_ct_refresh_acct(ct, ctinfo, skb, *timeout);
+	nf_ct_refresh_acct(ct, ctinfo, skb, READ_ONCE(*timeout));
 	return NF_ACCEPT;
 }
 
diff --git a/net/netfilter/nf_conntrack_proto_icmpv6.c b/net/netfilter/nf_conntrack_proto_icmpv6.c
index 803221781de39..e4eebf13950cb 100644
--- a/net/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/netfilter/nf_conntrack_proto_icmpv6.c
@@ -118,7 +118,7 @@ int nf_conntrack_icmpv6_packet(struct nf_conn *ct,
 	/* Do not immediately delete the connection after the first
 	   successful reply to avoid excessive conntrackd traffic
 	   and also to handle correctly ICMP echo reply duplicates. */
-	nf_ct_refresh_acct(ct, ctinfo, skb, *timeout);
+	nf_ct_refresh_acct(ct, ctinfo, skb, READ_ONCE(*timeout));
 
 	return NF_ACCEPT;
 }
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 49ec8478746ad..047bf69113bf9 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -523,7 +523,7 @@ int nf_conntrack_sctp_packet(struct nf_conn *ct,
 	if (!timeouts)
 		timeouts = nf_sctp_pernet(nf_ct_net(ct))->timeouts;
 
-	nf_ct_refresh_acct(ct, ctinfo, skb, timeouts[new_state]);
+	nf_ct_refresh_acct(ct, ctinfo, skb, READ_ONCE(timeouts[new_state]));
 
 	return NF_ACCEPT;
 
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index b859e409f0796..0c61a16fca400 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -505,7 +505,7 @@ nf_tcp_store_invalid(const struct nf_conn *ct,
 	const struct nf_tcp_net *tn = nf_tcp_pernet(nf_ct_net(ct));
 	bool be_liberal;
 
-	be_liberal = sender->flags & IP_CT_TCP_FLAG_BE_LIBERAL || tn->tcp_be_liberal;
+	be_liberal = sender->flags & IP_CT_TCP_FLAG_BE_LIBERAL || READ_ONCE(tn->tcp_be_liberal);
 	if (be_liberal)
 		return NFCT_TCP_ACCEPT;
 
@@ -904,7 +904,7 @@ static noinline bool tcp_new(struct nf_conn *ct, const struct sk_buff *skb,
 			ct->proto.tcp.seen[0].td_end;
 
 		tcp_options(skb, dataoff, th, &ct->proto.tcp.seen[0]);
-	} else if (tn->tcp_loose == 0) {
+	} else if (READ_ONCE(tn->tcp_loose) == 0) {
 		/* Don't try to pick up connections. */
 		return false;
 	} else {
@@ -979,7 +979,7 @@ void nf_conntrack_tcp_set_closing(struct nf_conn *ct)
 		timeouts = tn->timeouts;
 	}
 
-	timeout = timeouts[TCP_CONNTRACK_CLOSE];
+	timeout = READ_ONCE(timeouts[TCP_CONNTRACK_CLOSE]);
 	WRITE_ONCE(ct->timeout, timeout + nfct_time_stamp);
 
 	spin_unlock_bh(&ct->lock);
@@ -1010,6 +1010,7 @@ int nf_conntrack_tcp_packet(struct nf_conn *ct,
 	struct nf_tcp_invalid_log log = {};
 	unsigned int index, *timeouts;
 	bool lowered_timeout = false;
+	unsigned int retrans, unack;
 	enum nf_ct_tcp_action res;
 	enum ip_conntrack_dir dir;
 	const struct tcphdr *th;
@@ -1235,7 +1236,7 @@ int nf_conntrack_tcp_packet(struct nf_conn *ct,
 				break;
 
 			if (before(seq, ct->proto.tcp.seen[!dir].td_maxack) &&
-			    !tn->tcp_ignore_invalid_rst) {
+			    !READ_ONCE(tn->tcp_ignore_invalid_rst)) {
 				/* Invalid RST  */
 				spin_unlock_bh(&ct->lock);
 				nf_ct_l4proto_log_invalid(skb, ct, state, "invalid rst");
@@ -1322,22 +1323,24 @@ int nf_conntrack_tcp_packet(struct nf_conn *ct,
 	if (!timeouts)
 		timeouts = tn->timeouts;
 
-	if (ct->proto.tcp.retrans >= tn->tcp_max_retrans &&
-	    timeouts[new_state] > timeouts[TCP_CONNTRACK_RETRANS])
-		timeout = timeouts[TCP_CONNTRACK_RETRANS];
+	timeout = READ_ONCE(timeouts[new_state]);
+	retrans = READ_ONCE(timeouts[TCP_CONNTRACK_RETRANS]);
+	unack = READ_ONCE(timeouts[TCP_CONNTRACK_UNACK]);
+
+	if (ct->proto.tcp.retrans >= READ_ONCE(tn->tcp_max_retrans) &&
+	    timeout > retrans)
+		timeout = retrans;
 	else if (unlikely(index == TCP_RST_SET &&
 			  new_state == TCP_CONNTRACK_ESTABLISHED) &&
-		 timeouts[new_state] > timeouts[TCP_CONNTRACK_UNACK])
-		timeout = timeouts[TCP_CONNTRACK_UNACK];
+		 timeout > unack)
+		timeout = unack;
 	else if ((ct->proto.tcp.seen[0].flags | ct->proto.tcp.seen[1].flags) &
 		 IP_CT_TCP_FLAG_DATA_UNACKNOWLEDGED &&
-		 timeouts[new_state] > timeouts[TCP_CONNTRACK_UNACK])
-		timeout = timeouts[TCP_CONNTRACK_UNACK];
+		 timeout > unack)
+		timeout = unack;
 	else if (ct->proto.tcp.last_win == 0 &&
-		 timeouts[new_state] > timeouts[TCP_CONNTRACK_RETRANS])
-		timeout = timeouts[TCP_CONNTRACK_RETRANS];
-	else
-		timeout = timeouts[new_state];
+		 timeout > retrans)
+		timeout = retrans;
 	spin_unlock_bh(&ct->lock);
 
 	if (new_state != old_state)
@@ -1366,8 +1369,8 @@ int nf_conntrack_tcp_packet(struct nf_conn *ct,
 		 * pickup with loose=1. Avoid large ESTABLISHED timeout.
 		 */
 		if (new_state == TCP_CONNTRACK_ESTABLISHED &&
-		    timeout > timeouts[TCP_CONNTRACK_UNACK])
-			timeout = timeouts[TCP_CONNTRACK_UNACK];
+		    timeout > unack)
+			timeout = unack;
 	} else if (!test_bit(IPS_ASSURED_BIT, &ct->status)
 		   && (old_state == TCP_CONNTRACK_SYN_RECV
 		       || old_state == TCP_CONNTRACK_ESTABLISHED)
diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index 9ea4062752238..99eedaa2b1183 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -116,12 +116,12 @@ int nf_conntrack_udp_packet(struct nf_conn *ct,
 	 * stream. Set Assured.
 	 */
 	if (status & IPS_SEEN_REPLY) {
-		unsigned long extra = timeouts[UDP_CT_UNREPLIED];
+		unsigned long extra = READ_ONCE(timeouts[UDP_CT_UNREPLIED]);
 		bool stream = false;
 
 		/* Still active after two seconds? Extend timeout. */
 		if (time_after(jiffies, ct->proto.udp.stream_ts)) {
-			extra = timeouts[UDP_CT_REPLIED];
+			extra = READ_ONCE(timeouts[UDP_CT_REPLIED]);
 			stream = (status & IPS_ASSURED) == 0;
 		}
 
@@ -135,7 +135,7 @@ int nf_conntrack_udp_packet(struct nf_conn *ct,
 		if (stream && !test_and_set_bit(IPS_ASSURED_BIT, &ct->status))
 			nf_conntrack_event_cache(IPCT_ASSURED, ct);
 	} else {
-		nf_ct_refresh_acct(ct, ctinfo, skb, timeouts[UDP_CT_UNREPLIED]);
+		nf_ct_refresh_acct(ct, ctinfo, skb, READ_ONCE(timeouts[UDP_CT_UNREPLIED]));
 	}
 	return NF_ACCEPT;
 }
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 0d72dc8ae47a1..fa25a641e9c38 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -322,11 +322,11 @@ unsigned long flow_offload_get_timeout(struct flow_offload *flow)
 	if (l4num == IPPROTO_TCP) {
 		struct nf_tcp_net *tn = nf_tcp_pernet(net);
 
-		timeout = tn->offload_timeout;
+		timeout = READ_ONCE(tn->offload_timeout);
 	} else if (l4num == IPPROTO_UDP) {
 		struct nf_udp_net *tn = nf_udp_pernet(net);
 
-		timeout = tn->offload_timeout;
+		timeout = READ_ONCE(tn->offload_timeout);
 	}
 
 	return timeout;
-- 
2.43.0

[PATCH net-next 14/14] netfilter: cttimeout: annotate data-races around the default timeouts

[Jiayuan Chen]

cttimeout_default_set() and cttimeout_default_get() hand the per-netns
default timeout arrays to the l4proto nlattr_to_obj() and obj_to_nlattr()
callbacks, which write and read them with plain accesses while the
nf_conntrack_*_timeout* sysctl handlers update the same fields with
WRITE_ONCE(). Use WRITE_ONCE() and READ_ONCE() in the callbacks.

KCSAN reports both sides when the UDP default timeouts are set over
nfnetlink while UDP traffic goes through conntrack, or while the sysctl
is written:

  BUG: KCSAN: data-race in nf_conntrack_udp_packet / udp_timeout_nlattr_to_obj

  write to 0xffffffffb0829bc4 of 4 bytes by task 457 on cpu 1:
   udp_timeout_nlattr_to_obj+0x3b/0xf0
   ctnl_timeout_parse_policy+0xc0/0x100
   cttimeout_default_set+0x8f/0xc0
   nfnetlink_rcv_msg+0x64c/0xc90
   netlink_rcv_skb+0x9c/0x1b0
   nfnetlink_rcv+0x9b/0x1e0
   netlink_unicast+0x418/0x5c0
   netlink_sendmsg+0x3fc/0x750
   ...

  read to 0xffffffffb0829bc4 of 4 bytes by task 459 on cpu 0:
   nf_conntrack_udp_packet+0x27b/0x550
   nf_conntrack_in+0x767/0xb20
   ipv4_conntrack_local+0xb0/0x120
   nf_hook_slow+0x7e/0x180
   ...

  value changed: 0x00001e46 -> 0x00001d4c

  BUG: KCSAN: data-race in proc_int_u2k_conv_uop / udp_timeout_nlattr_to_obj

  write (marked) to 0xffffffffb0829bc4 of 4 bytes by task 475 on cpu 1:
   proc_int_u2k_conv_uop+0x92/0xa0
   ...
   proc_dointvec_jiffies+0x15/0x30
   proc_sys_call_handler+0x282/0x3a0
   proc_sys_write+0x13/0x20
   ...

  read to 0xffffffffb0829bc4 of 4 bytes by task 474 on cpu 0:
   udp_timeout_nlattr_to_obj+0x2c/0xf0
   ctnl_timeout_parse_policy+0xc0/0x100
   cttimeout_default_set+0x8f/0xc0
   nfnetlink_rcv_msg+0x64c/0xc90
   ...

  value changed: 0x00001d4c -> 0x00001e46

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/netfilter/nf_conntrack_proto_generic.c |  8 +--
 net/netfilter/nf_conntrack_proto_gre.c     | 16 ++---
 net/netfilter/nf_conntrack_proto_icmp.c    |  8 +--
 net/netfilter/nf_conntrack_proto_icmpv6.c  |  8 +--
 net/netfilter/nf_conntrack_proto_sctp.c    |  9 +--
 net/netfilter/nf_conntrack_proto_tcp.c     | 71 +++++++++++-----------
 net/netfilter/nf_conntrack_proto_udp.c     | 16 ++---
 7 files changed, 69 insertions(+), 67 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
index cb260eb3d012c..08b8d44c9f402 100644
--- a/net/netfilter/nf_conntrack_proto_generic.c
+++ b/net/netfilter/nf_conntrack_proto_generic.c
@@ -27,11 +27,11 @@ static int generic_timeout_nlattr_to_obj(struct nlattr *tb[],
 		timeout = &gn->timeout;
 
 	if (tb[CTA_TIMEOUT_GENERIC_TIMEOUT])
-		*timeout =
-		    ntohl(nla_get_be32(tb[CTA_TIMEOUT_GENERIC_TIMEOUT])) * HZ;
+		WRITE_ONCE(*timeout,
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_GENERIC_TIMEOUT])) * HZ);
 	else {
 		/* Set default generic timeout. */
-		*timeout = gn->timeout;
+		WRITE_ONCE(*timeout, READ_ONCE(gn->timeout));
 	}
 
 	return 0;
@@ -42,7 +42,7 @@ generic_timeout_obj_to_nlattr(struct sk_buff *skb, const void *data)
 {
 	const unsigned int *timeout = data;
 
-	if (nla_put_be32(skb, CTA_TIMEOUT_GENERIC_TIMEOUT, htonl(*timeout / HZ)))
+	if (nla_put_be32(skb, CTA_TIMEOUT_GENERIC_TIMEOUT, htonl(READ_ONCE(*timeout) / HZ)))
 		goto nla_put_failure;
 
 	return 0;
diff --git a/net/netfilter/nf_conntrack_proto_gre.c b/net/netfilter/nf_conntrack_proto_gre.c
index d8bca1e29797b..e1b99c5a7c7a2 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -318,16 +318,16 @@ static int gre_timeout_nlattr_to_obj(struct nlattr *tb[],
 	if (!timeouts)
 		timeouts = gre_get_timeouts(net);
 	/* set default timeouts for GRE. */
-	timeouts[GRE_CT_UNREPLIED] = net_gre->timeouts[GRE_CT_UNREPLIED];
-	timeouts[GRE_CT_REPLIED] = net_gre->timeouts[GRE_CT_REPLIED];
+	WRITE_ONCE(timeouts[GRE_CT_UNREPLIED], READ_ONCE(net_gre->timeouts[GRE_CT_UNREPLIED]));
+	WRITE_ONCE(timeouts[GRE_CT_REPLIED], READ_ONCE(net_gre->timeouts[GRE_CT_REPLIED]));
 
 	if (tb[CTA_TIMEOUT_GRE_UNREPLIED]) {
-		timeouts[GRE_CT_UNREPLIED] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_GRE_UNREPLIED])) * HZ;
+		WRITE_ONCE(timeouts[GRE_CT_UNREPLIED],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_GRE_UNREPLIED])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_GRE_REPLIED]) {
-		timeouts[GRE_CT_REPLIED] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_GRE_REPLIED])) * HZ;
+		WRITE_ONCE(timeouts[GRE_CT_REPLIED],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_GRE_REPLIED])) * HZ);
 	}
 	return 0;
 }
@@ -338,9 +338,9 @@ gre_timeout_obj_to_nlattr(struct sk_buff *skb, const void *data)
 	const unsigned int *timeouts = data;
 
 	if (nla_put_be32(skb, CTA_TIMEOUT_GRE_UNREPLIED,
-			 htonl(timeouts[GRE_CT_UNREPLIED] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[GRE_CT_UNREPLIED]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_GRE_REPLIED,
-			 htonl(timeouts[GRE_CT_REPLIED] / HZ)))
+			 htonl(READ_ONCE(timeouts[GRE_CT_REPLIED]) / HZ)))
 		goto nla_put_failure;
 	return 0;
 
diff --git a/net/netfilter/nf_conntrack_proto_icmp.c b/net/netfilter/nf_conntrack_proto_icmp.c
index 0f6ef0443c6fa..0d8499b54f878 100644
--- a/net/netfilter/nf_conntrack_proto_icmp.c
+++ b/net/netfilter/nf_conntrack_proto_icmp.c
@@ -327,11 +327,11 @@ static int icmp_timeout_nlattr_to_obj(struct nlattr *tb[],
 	if (tb[CTA_TIMEOUT_ICMP_TIMEOUT]) {
 		if (!timeout)
 			timeout = &in->timeout;
-		*timeout =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_ICMP_TIMEOUT])) * HZ;
+		WRITE_ONCE(*timeout,
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_ICMP_TIMEOUT])) * HZ);
 	} else if (timeout) {
 		/* Set default ICMP timeout. */
-		*timeout = in->timeout;
+		WRITE_ONCE(*timeout, READ_ONCE(in->timeout));
 	}
 	return 0;
 }
@@ -341,7 +341,7 @@ icmp_timeout_obj_to_nlattr(struct sk_buff *skb, const void *data)
 {
 	const unsigned int *timeout = data;
 
-	if (nla_put_be32(skb, CTA_TIMEOUT_ICMP_TIMEOUT, htonl(*timeout / HZ)))
+	if (nla_put_be32(skb, CTA_TIMEOUT_ICMP_TIMEOUT, htonl(READ_ONCE(*timeout) / HZ)))
 		goto nla_put_failure;
 	return 0;
 
diff --git a/net/netfilter/nf_conntrack_proto_icmpv6.c b/net/netfilter/nf_conntrack_proto_icmpv6.c
index e4eebf13950cb..6ed0f7ba60c3d 100644
--- a/net/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/netfilter/nf_conntrack_proto_icmpv6.c
@@ -305,11 +305,11 @@ static int icmpv6_timeout_nlattr_to_obj(struct nlattr *tb[],
 	if (!timeout)
 		timeout = icmpv6_get_timeouts(net);
 	if (tb[CTA_TIMEOUT_ICMPV6_TIMEOUT]) {
-		*timeout =
-		    ntohl(nla_get_be32(tb[CTA_TIMEOUT_ICMPV6_TIMEOUT])) * HZ;
+		WRITE_ONCE(*timeout,
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_ICMPV6_TIMEOUT])) * HZ);
 	} else {
 		/* Set default ICMPv6 timeout. */
-		*timeout = in->timeout;
+		WRITE_ONCE(*timeout, READ_ONCE(in->timeout));
 	}
 	return 0;
 }
@@ -319,7 +319,7 @@ icmpv6_timeout_obj_to_nlattr(struct sk_buff *skb, const void *data)
 {
 	const unsigned int *timeout = data;
 
-	if (nla_put_be32(skb, CTA_TIMEOUT_ICMPV6_TIMEOUT, htonl(*timeout / HZ)))
+	if (nla_put_be32(skb, CTA_TIMEOUT_ICMPV6_TIMEOUT, htonl(READ_ONCE(*timeout) / HZ)))
 		goto nla_put_failure;
 	return 0;
 
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 047bf69113bf9..ea6dd1ef3df89 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -650,16 +650,17 @@ static int sctp_timeout_nlattr_to_obj(struct nlattr *tb[],
 
 	/* set default SCTP timeouts. */
 	for (i=0; i<SCTP_CONNTRACK_MAX; i++)
-		timeouts[i] = sn->timeouts[i];
+		WRITE_ONCE(timeouts[i], READ_ONCE(sn->timeouts[i]));
 
 	/* there's a 1:1 mapping between attributes and protocol states. */
 	for (i=CTA_TIMEOUT_SCTP_UNSPEC+1; i<CTA_TIMEOUT_SCTP_MAX+1; i++) {
 		if (tb[i]) {
-			timeouts[i] = ntohl(nla_get_be32(tb[i])) * HZ;
+			WRITE_ONCE(timeouts[i], ntohl(nla_get_be32(tb[i])) * HZ);
 		}
 	}
 
-	timeouts[CTA_TIMEOUT_SCTP_UNSPEC] = timeouts[CTA_TIMEOUT_SCTP_CLOSED];
+	WRITE_ONCE(timeouts[CTA_TIMEOUT_SCTP_UNSPEC],
+		   READ_ONCE(timeouts[CTA_TIMEOUT_SCTP_CLOSED]));
 	return 0;
 }
 
@@ -670,7 +671,7 @@ sctp_timeout_obj_to_nlattr(struct sk_buff *skb, const void *data)
 	int i;
 
 	for (i=CTA_TIMEOUT_SCTP_UNSPEC+1; i<CTA_TIMEOUT_SCTP_MAX+1; i++) {
-	        if (nla_put_be32(skb, i, htonl(timeouts[i] / HZ)))
+		if (nla_put_be32(skb, i, htonl(READ_ONCE(timeouts[i]) / HZ)))
 			goto nla_put_failure;
 	}
         return 0;
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index 0c61a16fca400..e1db372b9e530 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -1523,55 +1523,56 @@ static int tcp_timeout_nlattr_to_obj(struct nlattr *tb[],
 		timeouts = tn->timeouts;
 	/* set default TCP timeouts. */
 	for (i=0; i<TCP_CONNTRACK_TIMEOUT_MAX; i++)
-		timeouts[i] = tn->timeouts[i];
+		WRITE_ONCE(timeouts[i], READ_ONCE(tn->timeouts[i]));
 
 	if (tb[CTA_TIMEOUT_TCP_SYN_SENT]) {
-		timeouts[TCP_CONNTRACK_SYN_SENT] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_SYN_SENT]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_SYN_SENT],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_SYN_SENT])) * HZ);
 	}
 
 	if (tb[CTA_TIMEOUT_TCP_SYN_RECV]) {
-		timeouts[TCP_CONNTRACK_SYN_RECV] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_SYN_RECV]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_SYN_RECV],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_SYN_RECV])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_TCP_ESTABLISHED]) {
-		timeouts[TCP_CONNTRACK_ESTABLISHED] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_ESTABLISHED]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_ESTABLISHED],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_ESTABLISHED])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_TCP_FIN_WAIT]) {
-		timeouts[TCP_CONNTRACK_FIN_WAIT] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_FIN_WAIT]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_FIN_WAIT],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_FIN_WAIT])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_TCP_CLOSE_WAIT]) {
-		timeouts[TCP_CONNTRACK_CLOSE_WAIT] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_CLOSE_WAIT]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_CLOSE_WAIT],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_CLOSE_WAIT])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_TCP_LAST_ACK]) {
-		timeouts[TCP_CONNTRACK_LAST_ACK] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_LAST_ACK]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_LAST_ACK],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_LAST_ACK])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_TCP_TIME_WAIT]) {
-		timeouts[TCP_CONNTRACK_TIME_WAIT] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_TIME_WAIT]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_TIME_WAIT],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_TIME_WAIT])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_TCP_CLOSE]) {
-		timeouts[TCP_CONNTRACK_CLOSE] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_CLOSE]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_CLOSE],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_CLOSE])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_TCP_SYN_SENT2]) {
-		timeouts[TCP_CONNTRACK_SYN_SENT2] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_SYN_SENT2]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_SYN_SENT2],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_SYN_SENT2])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_TCP_RETRANS]) {
-		timeouts[TCP_CONNTRACK_RETRANS] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_RETRANS]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_RETRANS],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_RETRANS])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_TCP_UNACK]) {
-		timeouts[TCP_CONNTRACK_UNACK] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_UNACK]))*HZ;
+		WRITE_ONCE(timeouts[TCP_CONNTRACK_UNACK],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_TCP_UNACK])) * HZ);
 	}
 
-	timeouts[CTA_TIMEOUT_TCP_UNSPEC] = timeouts[CTA_TIMEOUT_TCP_SYN_SENT];
+	WRITE_ONCE(timeouts[CTA_TIMEOUT_TCP_UNSPEC],
+		   READ_ONCE(timeouts[CTA_TIMEOUT_TCP_SYN_SENT]));
 	return 0;
 }
 
@@ -1581,27 +1582,27 @@ tcp_timeout_obj_to_nlattr(struct sk_buff *skb, const void *data)
 	const unsigned int *timeouts = data;
 
 	if (nla_put_be32(skb, CTA_TIMEOUT_TCP_SYN_SENT,
-			htonl(timeouts[TCP_CONNTRACK_SYN_SENT] / HZ)) ||
+			htonl(READ_ONCE(timeouts[TCP_CONNTRACK_SYN_SENT]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_TCP_SYN_RECV,
-			 htonl(timeouts[TCP_CONNTRACK_SYN_RECV] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[TCP_CONNTRACK_SYN_RECV]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_TCP_ESTABLISHED,
-			 htonl(timeouts[TCP_CONNTRACK_ESTABLISHED] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[TCP_CONNTRACK_ESTABLISHED]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_TCP_FIN_WAIT,
-			 htonl(timeouts[TCP_CONNTRACK_FIN_WAIT] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[TCP_CONNTRACK_FIN_WAIT]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_TCP_CLOSE_WAIT,
-			 htonl(timeouts[TCP_CONNTRACK_CLOSE_WAIT] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[TCP_CONNTRACK_CLOSE_WAIT]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_TCP_LAST_ACK,
-			 htonl(timeouts[TCP_CONNTRACK_LAST_ACK] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[TCP_CONNTRACK_LAST_ACK]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_TCP_TIME_WAIT,
-			 htonl(timeouts[TCP_CONNTRACK_TIME_WAIT] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[TCP_CONNTRACK_TIME_WAIT]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_TCP_CLOSE,
-			 htonl(timeouts[TCP_CONNTRACK_CLOSE] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[TCP_CONNTRACK_CLOSE]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_TCP_SYN_SENT2,
-			 htonl(timeouts[TCP_CONNTRACK_SYN_SENT2] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[TCP_CONNTRACK_SYN_SENT2]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_TCP_RETRANS,
-			 htonl(timeouts[TCP_CONNTRACK_RETRANS] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[TCP_CONNTRACK_RETRANS]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_TCP_UNACK,
-			 htonl(timeouts[TCP_CONNTRACK_UNACK] / HZ)))
+			 htonl(READ_ONCE(timeouts[TCP_CONNTRACK_UNACK]) / HZ)))
 		goto nla_put_failure;
 	return 0;
 
diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index 99eedaa2b1183..777b21da3bcff 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -155,16 +155,16 @@ static int udp_timeout_nlattr_to_obj(struct nlattr *tb[],
 		timeouts = un->timeouts;
 
 	/* set default timeouts for UDP. */
-	timeouts[UDP_CT_UNREPLIED] = un->timeouts[UDP_CT_UNREPLIED];
-	timeouts[UDP_CT_REPLIED] = un->timeouts[UDP_CT_REPLIED];
+	WRITE_ONCE(timeouts[UDP_CT_UNREPLIED], READ_ONCE(un->timeouts[UDP_CT_UNREPLIED]));
+	WRITE_ONCE(timeouts[UDP_CT_REPLIED], READ_ONCE(un->timeouts[UDP_CT_REPLIED]));
 
 	if (tb[CTA_TIMEOUT_UDP_UNREPLIED]) {
-		timeouts[UDP_CT_UNREPLIED] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_UDP_UNREPLIED])) * HZ;
+		WRITE_ONCE(timeouts[UDP_CT_UNREPLIED],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_UDP_UNREPLIED])) * HZ);
 	}
 	if (tb[CTA_TIMEOUT_UDP_REPLIED]) {
-		timeouts[UDP_CT_REPLIED] =
-			ntohl(nla_get_be32(tb[CTA_TIMEOUT_UDP_REPLIED])) * HZ;
+		WRITE_ONCE(timeouts[UDP_CT_REPLIED],
+			   ntohl(nla_get_be32(tb[CTA_TIMEOUT_UDP_REPLIED])) * HZ);
 	}
 	return 0;
 }
@@ -175,9 +175,9 @@ udp_timeout_obj_to_nlattr(struct sk_buff *skb, const void *data)
 	const unsigned int *timeouts = data;
 
 	if (nla_put_be32(skb, CTA_TIMEOUT_UDP_UNREPLIED,
-			 htonl(timeouts[UDP_CT_UNREPLIED] / HZ)) ||
+			 htonl(READ_ONCE(timeouts[UDP_CT_UNREPLIED]) / HZ)) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_UDP_REPLIED,
-			 htonl(timeouts[UDP_CT_REPLIED] / HZ)))
+			 htonl(READ_ONCE(timeouts[UDP_CT_REPLIED]) / HZ)))
 		goto nla_put_failure;
 	return 0;
 
-- 
2.43.0

Re: [PATCH net-next 00/14] net: annotate data-races around sysctl knobs

[Jason Xing]

On Mon, Nov 9, 2026 at 10:55 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> While testing the recent sysctl range fixes I noticed that a lot of our
> sysctl knobs are read with plain loads while the handlers update them
> with WRITE_ONCE() and no lock, so KCSAN reports them as data races. I
> then used an LLM to go through every ctl_table under net/ and list the
> readers that were left behind. This series annotates them with
> READ_ONCE(), and adds WRITE_ONCE() to a few custom handlers and netlink
> paths that store to the same fields with plain writes. Most of it is a
> plain READ_ONCE() at each read; where a function reads the same knob
> twice to make one decision, it now reads it once into a local, so the
> comparison and the value it acts on cannot come from two different
> writes.
>
> There are no Fixes tags. Done one knob per patch this would be well over
> a hundred commits, so I grouped them by subsystem and feature instead to
> keep the review manageable. If maintainers prefer, I can split it up
> further with a Fixes tag on each one, as several smaller series.

Over the last year, I've been thinking about the annotation stuff
especially around sysctls, which, I suppose, shouldn't actually affect
the real workload but does affect the outcome of the syzbot.

Out of sheer curiosity, my question would be what is the point of
trying to change all of them? Or what kind of benefits can we get from
the big series?

Thanks,
Jason

>
> No functional change. KCSAN does hit these, e.g. toggling
> net.ipv6.icmp.echo_ignore_all while receiving ICMPv6 echo requests:
>
>   BUG: KCSAN: data-race in icmpv6_rcv / proc_dou8vec_minmax
>
>   write (marked) to 0xffffffff8ee2940d of 1 bytes by task 504 on cpu 0:
>    proc_dou8vec_minmax+0x1ab/0x1f0
>    proc_sys_call_handler+0x282/0x3a0
>    proc_sys_write+0x13/0x20
>    ...
>
>   read to 0xffffffff8ee2940d of 1 bytes by interrupt on cpu 3:
>    icmpv6_rcv+0x99d/0xd40
>    ip6_protocol_deliver_rcu+0x787/0x9a0
>    ip6_input_finish+0xb0/0x1b0
>    ip6_input+0xa8/0x250
>    ipv6_rcv+0x14b/0x1c0
>    ...
>
>   value changed: 0x01 -> 0x00
>
> Jiayuan Chen (14):
>   ipv4: annotate data-races around ip_rt_* sysctls
>   inet: frags: annotate data-races around fqdir->timeout and
>     fqdir->max_dist
>   ipv6: annotate data-races around addrconf sysctls
>   ipv6: annotate data-races around icmpv6 reply sysctls
>   calipso: annotate data-races around cache sysctls
>   sctp: annotate data-races around sctp sysctls
>   mptcp: annotate data-races around pernet sysctls
>   rds: annotate data-races around core rds sysctls
>   netfilter: conntrack: annotate data-races around conntrack sysctls
>   netfilter: bridge: annotate data-races around br_netfilter and nf_log
>     sysctls
>   ipvs: annotate data-races around lblc_expiration and lblcr_expiration
>   net/smc: annotate data-races around smc sysctls
>   netfilter: conntrack: annotate data-races around timeout and tcp
>     sysctls
>   netfilter: cttimeout: annotate data-races around the default timeouts
>
>  include/net/ipv6.h                            |   2 +-
>  include/net/netfilter/nf_conntrack_acct.h     |   6 +-
>  .../net/netfilter/nf_conntrack_timestamp.h    |   2 +-
>  net/bridge/br_netfilter_hooks.c               |  20 ++--
>  net/bridge/netfilter/ebt_log.c                |   2 +-
>  net/ipv4/inet_fragment.c                      |   2 +-
>  net/ipv4/ip_fragment.c                        |   6 +-
>  net/ipv4/route.c                              |  34 +++---
>  net/ipv6/addrconf.c                           |  22 ++--
>  net/ipv6/calipso.c                            |   6 +-
>  net/ipv6/icmp.c                               |  10 +-
>  net/ipv6/ndisc.c                              |   2 +-
>  net/ipv6/route.c                              |   8 +-
>  net/mptcp/ctrl.c                              |  18 +--
>  net/netfilter/ipvs/ip_vs_lblc.c               |   2 +-
>  net/netfilter/ipvs/ip_vs_lblcr.c              |   2 +-
>  net/netfilter/nf_conntrack_core.c             |   8 +-
>  net/netfilter/nf_conntrack_ecache.c           |   2 +-
>  net/netfilter/nf_conntrack_expect.c           |   2 +-
>  net/netfilter/nf_conntrack_netlink.c          |   2 +-
>  net/netfilter/nf_conntrack_proto.c            |   6 +-
>  net/netfilter/nf_conntrack_proto_generic.c    |   8 +-
>  net/netfilter/nf_conntrack_proto_gre.c        |  20 ++--
>  net/netfilter/nf_conntrack_proto_icmp.c       |  12 +-
>  net/netfilter/nf_conntrack_proto_icmpv6.c     |  12 +-
>  net/netfilter/nf_conntrack_proto_sctp.c       |  13 ++-
>  net/netfilter/nf_conntrack_proto_tcp.c        | 110 +++++++++---------
>  net/netfilter/nf_conntrack_proto_udp.c        |  24 ++--
>  net/netfilter/nf_flow_table_core.c            |   4 +-
>  net/netfilter/nf_log_syslog.c                 |   2 +-
>  net/rds/recv.c                                |   2 +-
>  net/rds/send.c                                |   8 +-
>  net/rds/threads.c                             |   4 +-
>  net/sctp/associola.c                          |   2 +-
>  net/sctp/bind_addr.c                          |   2 +-
>  net/sctp/endpointola.c                        |  14 +--
>  net/sctp/input.c                              |   2 +-
>  net/sctp/sm_make_chunk.c                      |   6 +-
>  net/sctp/sm_sideeffect.c                      |   2 +-
>  net/sctp/sm_statefuns.c                       |   6 +-
>  net/sctp/socket.c                             |  46 ++++----
>  net/sctp/sysctl.c                             |  16 +--
>  net/sctp/transport.c                          |   6 +-
>  net/smc/af_smc.c                              |   8 +-
>  net/smc/smc_clc.c                             |   8 +-
>  net/smc/smc_core.c                            |   2 +-
>  net/smc/smc_tx.c                              |   2 +-
>  47 files changed, 259 insertions(+), 246 deletions(-)
>
> --
> 2.43.0
>
>

Re: [PATCH net-next 00/14] net: annotate data-races around sysctl knobs

[Jiayuan Chen]

On 11/9/26 11:11 AM, Jason Xing wrote:
> On Mon, Nov 9, 2026 at 10:55 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>> While testing the recent sysctl range fixes I noticed that a lot of our
>> sysctl knobs are read with plain loads while the handlers update them
>> with WRITE_ONCE() and no lock, so KCSAN reports them as data races. I
>> then used an LLM to go through every ctl_table under net/ and list the
>> readers that were left behind. This series annotates them with
>> READ_ONCE(), and adds WRITE_ONCE() to a few custom handlers and netlink
>> paths that store to the same fields with plain writes. Most of it is a
>> plain READ_ONCE() at each read; where a function reads the same knob
>> twice to make one decision, it now reads it once into a local, so the
>> comparison and the value it acts on cannot come from two different
>> writes.
>>
>> There are no Fixes tags. Done one knob per patch this would be well over
>> a hundred commits, so I grouped them by subsystem and feature instead to
>> keep the review manageable. If maintainers prefer, I can split it up
>> further with a Fixes tag on each one, as several smaller series.
> Over the last year, I've been thinking about the annotation stuff
> especially around sysctls, which, I suppose, shouldn't actually affect
> the real workload but does affect the outcome of the syzbot.
>
> Out of sheer curiosity, my question would be what is the point of
> trying to change all of them? Or what kind of benefits can we get from
> the big series?


Three things:

1. A plain load may be emitted twice by the compiler. ip_error() compares
peer->rate_tokens against ip_rt_error_cost and then subtracts it again;
if the sysctl changes in between, the u32 wraps. A few readers have that
shape and now read the knob once into a local.

2. Tearing is not involved here, all the fields are at most word sized.

3. The rest is the memory model: a lockless read of a field written with
WRITE_ONCE() is a data race unless it is marked
(tools/memory-model/Documentation/access-marking.txt). That is what KCSAN
reports, and why most of the series is plain READ_ONCE().


I think the benefit is doing it in one pass.