netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 00/11] tcp: add missing annotations
@ 2023-07-19 21:28 Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 01/11] tcp: annotate data-races around tp->tcp_tx_delay Eric Dumazet
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

This series was inspired by one syzbot (KCSAN) report.

do_tcp_getsockopt() does not lock the socket, we need to
annotate most of the reads there (and other places as well).

This is a first round, another series will come later.

Eric Dumazet (11):
  tcp: annotate data-races around tp->tcp_tx_delay
  tcp: annotate data-races around tp->tsoffset
  tcp: annotate data-races around tp->keepalive_time
  tcp: annotate data-races around tp->keepalive_intvl
  tcp: annotate data-races around tp->keepalive_probes
  tcp: annotate data-races around icsk->icsk_syn_retries
  tcp: annotate data-races around tp->linger2
  tcp: annotate data-races around rskq_defer_accept
  tcp: annotate data-races around tp->notsent_lowat
  tcp: annotate data-races around icsk->icsk_user_timeout
  tcp: annotate data-races around fastopenq.max_qlen

 include/linux/tcp.h             |  2 +-
 include/net/tcp.h               | 31 ++++++++++++++----
 net/ipv4/inet_connection_sock.c |  2 +-
 net/ipv4/tcp.c                  | 57 +++++++++++++++++----------------
 net/ipv4/tcp_fastopen.c         |  6 ++--
 net/ipv4/tcp_ipv4.c             |  5 +--
 6 files changed, 63 insertions(+), 40 deletions(-)

-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net 01/11] tcp: annotate data-races around tp->tcp_tx_delay
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 02/11] tcp: annotate data-races around tp->tsoffset Eric Dumazet
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

do_tcp_getsockopt() reads tp->tcp_tx_delay while another cpu
might change its value.

Fixes: a842fe1425cb ("tcp: add optional per socket transmit delay")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index e03e08745308189c9d64509c2cff94da56c86a0c..bd6400e1ae9f8ae595bbe759ff3dfb1bd02765e2 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3674,7 +3674,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 	case TCP_TX_DELAY:
 		if (val)
 			tcp_enable_tx_delay();
-		tp->tcp_tx_delay = val;
+		WRITE_ONCE(tp->tcp_tx_delay, val);
 		break;
 	default:
 		err = -ENOPROTOOPT;
@@ -4154,7 +4154,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		break;
 
 	case TCP_TX_DELAY:
-		val = tp->tcp_tx_delay;
+		val = READ_ONCE(tp->tcp_tx_delay);
 		break;
 
 	case TCP_TIMESTAMP:
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 02/11] tcp: annotate data-races around tp->tsoffset
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 01/11] tcp: annotate data-races around tp->tcp_tx_delay Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 03/11] tcp: annotate data-races around tp->keepalive_time Eric Dumazet
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

do_tcp_getsockopt() reads tp->tsoffset while another cpu
might change its value.

Fixes: 93be6ce0e91b ("tcp: set and get per-socket timestamp")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp.c      | 4 ++--
 net/ipv4/tcp_ipv4.c | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index bd6400e1ae9f8ae595bbe759ff3dfb1bd02765e2..03ae6554c78d1e42894a8e511cef362134660aac 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3656,7 +3656,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 		if (!tp->repair)
 			err = -EPERM;
 		else
-			tp->tsoffset = val - tcp_time_stamp_raw();
+			WRITE_ONCE(tp->tsoffset, val - tcp_time_stamp_raw());
 		break;
 	case TCP_REPAIR_WINDOW:
 		err = tcp_repair_set_window(tp, optval, optlen);
@@ -4158,7 +4158,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		break;
 
 	case TCP_TIMESTAMP:
-		val = tcp_time_stamp_raw() + tp->tsoffset;
+		val = tcp_time_stamp_raw() + READ_ONCE(tp->tsoffset);
 		break;
 	case TCP_NOTSENT_LOWAT:
 		val = tp->notsent_lowat;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index b5c81cf5b86f7cb086c9c9619dec0c088e5d5916..0696420146369a8786f0dbab142e45aa09fbac00 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -307,8 +307,9 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 						  inet->inet_daddr,
 						  inet->inet_sport,
 						  usin->sin_port));
-		tp->tsoffset = secure_tcp_ts_off(net, inet->inet_saddr,
-						 inet->inet_daddr);
+		WRITE_ONCE(tp->tsoffset,
+			   secure_tcp_ts_off(net, inet->inet_saddr,
+					     inet->inet_daddr));
 	}
 
 	inet->inet_id = get_random_u16();
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 03/11] tcp: annotate data-races around tp->keepalive_time
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 01/11] tcp: annotate data-races around tp->tcp_tx_delay Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 02/11] tcp: annotate data-races around tp->tsoffset Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 04/11] tcp: annotate data-races around tp->keepalive_intvl Eric Dumazet
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

do_tcp_getsockopt() reads tp->keepalive_time while another cpu
might change its value.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/tcp.h | 7 +++++--
 net/ipv4/tcp.c    | 3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index efaed11e691d52db0fcece85d966954763d3cfcf..ff7372410472246d372402dfdfd6391544be8259 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1515,9 +1515,12 @@ static inline int keepalive_intvl_when(const struct tcp_sock *tp)
 static inline int keepalive_time_when(const struct tcp_sock *tp)
 {
 	struct net *net = sock_net((struct sock *)tp);
+	int val;
 
-	return tp->keepalive_time ? :
-		READ_ONCE(net->ipv4.sysctl_tcp_keepalive_time);
+	/* Paired with WRITE_ONCE() in tcp_sock_set_keepidle_locked() */
+	val = READ_ONCE(tp->keepalive_time);
+
+	return val ? : READ_ONCE(net->ipv4.sysctl_tcp_keepalive_time);
 }
 
 static inline int keepalive_probes(const struct tcp_sock *tp)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 03ae6554c78d1e42894a8e511cef362134660aac..b4f7856dfb1611f02073699ee24d48f1a6fe7b87 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3312,7 +3312,8 @@ int tcp_sock_set_keepidle_locked(struct sock *sk, int val)
 	if (val < 1 || val > MAX_TCP_KEEPIDLE)
 		return -EINVAL;
 
-	tp->keepalive_time = val * HZ;
+	/* Paired with WRITE_ONCE() in keepalive_time_when() */
+	WRITE_ONCE(tp->keepalive_time, val * HZ);
 	if (sock_flag(sk, SOCK_KEEPOPEN) &&
 	    !((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) {
 		u32 elapsed = keepalive_time_elapsed(tp);
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 04/11] tcp: annotate data-races around tp->keepalive_intvl
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
                   ` (2 preceding siblings ...)
  2023-07-19 21:28 ` [PATCH net 03/11] tcp: annotate data-races around tp->keepalive_time Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 05/11] tcp: annotate data-races around tp->keepalive_probes Eric Dumazet
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

do_tcp_getsockopt() reads tp->keepalive_intvl while another cpu
might change its value.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/tcp.h | 9 +++++++--
 net/ipv4/tcp.c    | 4 ++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index ff7372410472246d372402dfdfd6391544be8259..79af16a4028665d51f6ea5f1a4382265b8163309 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1507,9 +1507,14 @@ void tcp_leave_memory_pressure(struct sock *sk);
 static inline int keepalive_intvl_when(const struct tcp_sock *tp)
 {
 	struct net *net = sock_net((struct sock *)tp);
+	int val;
+
+	/* Paired with WRITE_ONCE() in tcp_sock_set_keepintvl()
+	 * and do_tcp_setsockopt().
+	 */
+	val = READ_ONCE(tp->keepalive_intvl);
 
-	return tp->keepalive_intvl ? :
-		READ_ONCE(net->ipv4.sysctl_tcp_keepalive_intvl);
+	return val ? : READ_ONCE(net->ipv4.sysctl_tcp_keepalive_intvl);
 }
 
 static inline int keepalive_time_when(const struct tcp_sock *tp)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b4f7856dfb1611f02073699ee24d48f1a6fe7b87..d55fe014e7c902859243cdb619d94a230e44f708 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3345,7 +3345,7 @@ int tcp_sock_set_keepintvl(struct sock *sk, int val)
 		return -EINVAL;
 
 	lock_sock(sk);
-	tcp_sk(sk)->keepalive_intvl = val * HZ;
+	WRITE_ONCE(tcp_sk(sk)->keepalive_intvl, val * HZ);
 	release_sock(sk);
 	return 0;
 }
@@ -3559,7 +3559,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 		if (val < 1 || val > MAX_TCP_KEEPINTVL)
 			err = -EINVAL;
 		else
-			tp->keepalive_intvl = val * HZ;
+			WRITE_ONCE(tp->keepalive_intvl, val * HZ);
 		break;
 	case TCP_KEEPCNT:
 		if (val < 1 || val > MAX_TCP_KEEPCNT)
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 05/11] tcp: annotate data-races around tp->keepalive_probes
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
                   ` (3 preceding siblings ...)
  2023-07-19 21:28 ` [PATCH net 04/11] tcp: annotate data-races around tp->keepalive_intvl Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 06/11] tcp: annotate data-races around icsk->icsk_syn_retries Eric Dumazet
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

do_tcp_getsockopt() reads tp->keepalive_probes while another cpu
might change its value.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/tcp.h | 9 +++++++--
 net/ipv4/tcp.c    | 5 +++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 79af16a4028665d51f6ea5f1a4382265b8163309..855dbe72e431776257037d75e32037b44905453c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1531,9 +1531,14 @@ static inline int keepalive_time_when(const struct tcp_sock *tp)
 static inline int keepalive_probes(const struct tcp_sock *tp)
 {
 	struct net *net = sock_net((struct sock *)tp);
+	int val;
+
+	/* Paired with WRITE_ONCE() in tcp_sock_set_keepcnt()
+	 * and do_tcp_setsockopt().
+	 */
+	val = READ_ONCE(tp->keepalive_probes);
 
-	return tp->keepalive_probes ? :
-		READ_ONCE(net->ipv4.sysctl_tcp_keepalive_probes);
+	return val ? : READ_ONCE(net->ipv4.sysctl_tcp_keepalive_probes);
 }
 
 static inline u32 keepalive_time_elapsed(const struct tcp_sock *tp)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index d55fe014e7c902859243cdb619d94a230e44f708..574fd0da167339512077c36958578fde2b1181e8 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3357,7 +3357,8 @@ int tcp_sock_set_keepcnt(struct sock *sk, int val)
 		return -EINVAL;
 
 	lock_sock(sk);
-	tcp_sk(sk)->keepalive_probes = val;
+	/* Paired with READ_ONCE() in keepalive_probes() */
+	WRITE_ONCE(tcp_sk(sk)->keepalive_probes, val);
 	release_sock(sk);
 	return 0;
 }
@@ -3565,7 +3566,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 		if (val < 1 || val > MAX_TCP_KEEPCNT)
 			err = -EINVAL;
 		else
-			tp->keepalive_probes = val;
+			WRITE_ONCE(tp->keepalive_probes, val);
 		break;
 	case TCP_SYNCNT:
 		if (val < 1 || val > MAX_TCP_SYNCNT)
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 06/11] tcp: annotate data-races around icsk->icsk_syn_retries
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
                   ` (4 preceding siblings ...)
  2023-07-19 21:28 ` [PATCH net 05/11] tcp: annotate data-races around tp->keepalive_probes Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 07/11] tcp: annotate data-races around tp->linger2 Eric Dumazet
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

do_tcp_getsockopt() and reqsk_timer_handler() read
icsk->icsk_syn_retries while another cpu might change its value.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/inet_connection_sock.c | 2 +-
 net/ipv4/tcp.c                  | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 0cc19cfbb67345960ef16bdaf6ec330a6eb397fd..aeebe881668996057d1495c84eee0f0b644b7ad0 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -1019,7 +1019,7 @@ static void reqsk_timer_handler(struct timer_list *t)
 
 	icsk = inet_csk(sk_listener);
 	net = sock_net(sk_listener);
-	max_syn_ack_retries = icsk->icsk_syn_retries ? :
+	max_syn_ack_retries = READ_ONCE(icsk->icsk_syn_retries) ? :
 		READ_ONCE(net->ipv4.sysctl_tcp_synack_retries);
 	/* Normally all the openreqs are young and become mature
 	 * (i.e. converted to established socket) for first timeout.
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 574fd0da167339512077c36958578fde2b1181e8..9f74ac16f1c1e53353bd14c6a04e1fa9e3de0c15 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3291,7 +3291,7 @@ int tcp_sock_set_syncnt(struct sock *sk, int val)
 		return -EINVAL;
 
 	lock_sock(sk);
-	inet_csk(sk)->icsk_syn_retries = val;
+	WRITE_ONCE(inet_csk(sk)->icsk_syn_retries, val);
 	release_sock(sk);
 	return 0;
 }
@@ -3572,7 +3572,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 		if (val < 1 || val > MAX_TCP_SYNCNT)
 			err = -EINVAL;
 		else
-			icsk->icsk_syn_retries = val;
+			WRITE_ONCE(icsk->icsk_syn_retries, val);
 		break;
 
 	case TCP_SAVE_SYN:
@@ -3993,7 +3993,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		val = keepalive_probes(tp);
 		break;
 	case TCP_SYNCNT:
-		val = icsk->icsk_syn_retries ? :
+		val = READ_ONCE(icsk->icsk_syn_retries) ? :
 			READ_ONCE(net->ipv4.sysctl_tcp_syn_retries);
 		break;
 	case TCP_LINGER2:
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 07/11] tcp: annotate data-races around tp->linger2
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
                   ` (5 preceding siblings ...)
  2023-07-19 21:28 ` [PATCH net 06/11] tcp: annotate data-races around icsk->icsk_syn_retries Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 08/11] tcp: annotate data-races around rskq_defer_accept Eric Dumazet
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

do_tcp_getsockopt() reads tp->linger2 while another cpu
might change its value.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9f74ac16f1c1e53353bd14c6a04e1fa9e3de0c15..2cf129a0c00bfef813e1f1e12cb247ef8107fa88 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3585,11 +3585,11 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 
 	case TCP_LINGER2:
 		if (val < 0)
-			tp->linger2 = -1;
+			WRITE_ONCE(tp->linger2, -1);
 		else if (val > TCP_FIN_TIMEOUT_MAX / HZ)
-			tp->linger2 = TCP_FIN_TIMEOUT_MAX;
+			WRITE_ONCE(tp->linger2, TCP_FIN_TIMEOUT_MAX);
 		else
-			tp->linger2 = val * HZ;
+			WRITE_ONCE(tp->linger2, val * HZ);
 		break;
 
 	case TCP_DEFER_ACCEPT:
@@ -3997,7 +3997,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 			READ_ONCE(net->ipv4.sysctl_tcp_syn_retries);
 		break;
 	case TCP_LINGER2:
-		val = tp->linger2;
+		val = READ_ONCE(tp->linger2);
 		if (val >= 0)
 			val = (val ? : READ_ONCE(net->ipv4.sysctl_tcp_fin_timeout)) / HZ;
 		break;
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 08/11] tcp: annotate data-races around rskq_defer_accept
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
                   ` (6 preceding siblings ...)
  2023-07-19 21:28 ` [PATCH net 07/11] tcp: annotate data-races around tp->linger2 Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 09/11] tcp: annotate data-races around tp->notsent_lowat Eric Dumazet
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

do_tcp_getsockopt() reads rskq_defer_accept while another cpu
might change its value.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2cf129a0c00bfef813e1f1e12cb247ef8107fa88..5beec71a5c418db65e19eb2a68ffd839d4550efc 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3594,9 +3594,9 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 
 	case TCP_DEFER_ACCEPT:
 		/* Translate value in seconds to number of retransmits */
-		icsk->icsk_accept_queue.rskq_defer_accept =
-			secs_to_retrans(val, TCP_TIMEOUT_INIT / HZ,
-					TCP_RTO_MAX / HZ);
+		WRITE_ONCE(icsk->icsk_accept_queue.rskq_defer_accept,
+			   secs_to_retrans(val, TCP_TIMEOUT_INIT / HZ,
+					   TCP_RTO_MAX / HZ));
 		break;
 
 	case TCP_WINDOW_CLAMP:
@@ -4002,8 +4002,9 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 			val = (val ? : READ_ONCE(net->ipv4.sysctl_tcp_fin_timeout)) / HZ;
 		break;
 	case TCP_DEFER_ACCEPT:
-		val = retrans_to_secs(icsk->icsk_accept_queue.rskq_defer_accept,
-				      TCP_TIMEOUT_INIT / HZ, TCP_RTO_MAX / HZ);
+		val = READ_ONCE(icsk->icsk_accept_queue.rskq_defer_accept);
+		val = retrans_to_secs(val, TCP_TIMEOUT_INIT / HZ,
+				      TCP_RTO_MAX / HZ);
 		break;
 	case TCP_WINDOW_CLAMP:
 		val = tp->window_clamp;
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 09/11] tcp: annotate data-races around tp->notsent_lowat
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
                   ` (7 preceding siblings ...)
  2023-07-19 21:28 ` [PATCH net 08/11] tcp: annotate data-races around rskq_defer_accept Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 10/11] tcp: annotate data-races around icsk->icsk_user_timeout Eric Dumazet
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

tp->notsent_lowat can be read locklessly from do_tcp_getsockopt()
and tcp_poll().

Fixes: c9bee3b7fdec ("tcp: TCP_NOTSENT_LOWAT socket option")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/tcp.h | 6 +++++-
 net/ipv4/tcp.c    | 4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 855dbe72e431776257037d75e32037b44905453c..a32d1963cb75ff81c164b3021a848d3d29816642 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2059,7 +2059,11 @@ void __tcp_v4_send_check(struct sk_buff *skb, __be32 saddr, __be32 daddr);
 static inline u32 tcp_notsent_lowat(const struct tcp_sock *tp)
 {
 	struct net *net = sock_net((struct sock *)tp);
-	return tp->notsent_lowat ?: READ_ONCE(net->ipv4.sysctl_tcp_notsent_lowat);
+	u32 val;
+
+	val = READ_ONCE(tp->notsent_lowat);
+
+	return val ?: READ_ONCE(net->ipv4.sysctl_tcp_notsent_lowat);
 }
 
 bool tcp_stream_memory_free(const struct sock *sk, int wake);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5beec71a5c418db65e19eb2a68ffd839d4550efc..2b2241e9b492726562a6b5055cf8c168e5fed799 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3664,7 +3664,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 		err = tcp_repair_set_window(tp, optval, optlen);
 		break;
 	case TCP_NOTSENT_LOWAT:
-		tp->notsent_lowat = val;
+		WRITE_ONCE(tp->notsent_lowat, val);
 		sk->sk_write_space(sk);
 		break;
 	case TCP_INQ:
@@ -4164,7 +4164,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		val = tcp_time_stamp_raw() + READ_ONCE(tp->tsoffset);
 		break;
 	case TCP_NOTSENT_LOWAT:
-		val = tp->notsent_lowat;
+		val = READ_ONCE(tp->notsent_lowat);
 		break;
 	case TCP_INQ:
 		val = tp->recvmsg_inq;
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 10/11] tcp: annotate data-races around icsk->icsk_user_timeout
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
                   ` (8 preceding siblings ...)
  2023-07-19 21:28 ` [PATCH net 09/11] tcp: annotate data-races around tp->notsent_lowat Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-19 21:28 ` [PATCH net 11/11] tcp: annotate data-races around fastopenq.max_qlen Eric Dumazet
  2023-07-20 19:50 ` [PATCH net 00/11] tcp: add missing annotations patchwork-bot+netdevbpf
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

This field can be read locklessly from do_tcp_getsockopt()

Fixes: dca43c75e7e5 ("tcp: Add TCP_USER_TIMEOUT socket option.")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2b2241e9b492726562a6b5055cf8c168e5fed799..3e137e9a18f552a02d8c74e1af34ba2356e4d8ed 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3300,7 +3300,7 @@ EXPORT_SYMBOL(tcp_sock_set_syncnt);
 void tcp_sock_set_user_timeout(struct sock *sk, u32 val)
 {
 	lock_sock(sk);
-	inet_csk(sk)->icsk_user_timeout = val;
+	WRITE_ONCE(inet_csk(sk)->icsk_user_timeout, val);
 	release_sock(sk);
 }
 EXPORT_SYMBOL(tcp_sock_set_user_timeout);
@@ -3620,7 +3620,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 		if (val < 0)
 			err = -EINVAL;
 		else
-			icsk->icsk_user_timeout = val;
+			WRITE_ONCE(icsk->icsk_user_timeout, val);
 		break;
 
 	case TCP_FASTOPEN:
@@ -4141,7 +4141,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		break;
 
 	case TCP_USER_TIMEOUT:
-		val = icsk->icsk_user_timeout;
+		val = READ_ONCE(icsk->icsk_user_timeout);
 		break;
 
 	case TCP_FASTOPEN:
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net 11/11] tcp: annotate data-races around fastopenq.max_qlen
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
                   ` (9 preceding siblings ...)
  2023-07-19 21:28 ` [PATCH net 10/11] tcp: annotate data-races around icsk->icsk_user_timeout Eric Dumazet
@ 2023-07-19 21:28 ` Eric Dumazet
  2023-07-20 19:50 ` [PATCH net 00/11] tcp: add missing annotations patchwork-bot+netdevbpf
  11 siblings, 0 replies; 13+ messages in thread
From: Eric Dumazet @ 2023-07-19 21:28 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet

This field can be read locklessly.

Fixes: 1536e2857bd3 ("tcp: Add a TCP_FASTOPEN socket option to get a max backlog on its listner")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/tcp.h     | 2 +-
 net/ipv4/tcp.c          | 2 +-
 net/ipv4/tcp_fastopen.c | 6 ++++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index b4c08ac86983568a9511258708724da15d0b999e..91a37c99ba6651c075d1547c5545700be3e5593c 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -513,7 +513,7 @@ static inline void fastopen_queue_tune(struct sock *sk, int backlog)
 	struct request_sock_queue *queue = &inet_csk(sk)->icsk_accept_queue;
 	int somaxconn = READ_ONCE(sock_net(sk)->core.sysctl_somaxconn);
 
-	queue->fastopenq.max_qlen = min_t(unsigned int, backlog, somaxconn);
+	WRITE_ONCE(queue->fastopenq.max_qlen, min_t(unsigned int, backlog, somaxconn));
 }
 
 static inline void tcp_move_syn(struct tcp_sock *tp,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 3e137e9a18f552a02d8c74e1af34ba2356e4d8ed..8ed52e1e3c99a334a47964d8fd05c720a8f683f9 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4145,7 +4145,7 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		break;
 
 	case TCP_FASTOPEN:
-		val = icsk->icsk_accept_queue.fastopenq.max_qlen;
+		val = READ_ONCE(icsk->icsk_accept_queue.fastopenq.max_qlen);
 		break;
 
 	case TCP_FASTOPEN_CONNECT:
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 45cc7f1ca29618e3ac1066cb49e7d6dc90e1c64d..85e4953f118215ba7100931dccb37ad871c5dfd2 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -296,6 +296,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
 static bool tcp_fastopen_queue_check(struct sock *sk)
 {
 	struct fastopen_queue *fastopenq;
+	int max_qlen;
 
 	/* Make sure the listener has enabled fastopen, and we don't
 	 * exceed the max # of pending TFO requests allowed before trying
@@ -308,10 +309,11 @@ static bool tcp_fastopen_queue_check(struct sock *sk)
 	 * temporarily vs a server not supporting Fast Open at all.
 	 */
 	fastopenq = &inet_csk(sk)->icsk_accept_queue.fastopenq;
-	if (fastopenq->max_qlen == 0)
+	max_qlen = READ_ONCE(fastopenq->max_qlen);
+	if (max_qlen == 0)
 		return false;
 
-	if (fastopenq->qlen >= fastopenq->max_qlen) {
+	if (fastopenq->qlen >= max_qlen) {
 		struct request_sock *req1;
 		spin_lock(&fastopenq->lock);
 		req1 = fastopenq->rskq_rst_head;
-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH net 00/11] tcp: add missing annotations
  2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
                   ` (10 preceding siblings ...)
  2023-07-19 21:28 ` [PATCH net 11/11] tcp: annotate data-races around fastopenq.max_qlen Eric Dumazet
@ 2023-07-20 19:50 ` patchwork-bot+netdevbpf
  11 siblings, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-07-20 19:50 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, kuba, pabeni, netdev, eric.dumazet

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 19 Jul 2023 21:28:46 +0000 you wrote:
> This series was inspired by one syzbot (KCSAN) report.
> 
> do_tcp_getsockopt() does not lock the socket, we need to
> annotate most of the reads there (and other places as well).
> 
> This is a first round, another series will come later.
> 
> [...]

Here is the summary with links:
  - [net,01/11] tcp: annotate data-races around tp->tcp_tx_delay
    https://git.kernel.org/netdev/net/c/348b81b68b13
  - [net,02/11] tcp: annotate data-races around tp->tsoffset
    https://git.kernel.org/netdev/net/c/dd23c9f1e8d5
  - [net,03/11] tcp: annotate data-races around tp->keepalive_time
    https://git.kernel.org/netdev/net/c/4164245c76ff
  - [net,04/11] tcp: annotate data-races around tp->keepalive_intvl
    https://git.kernel.org/netdev/net/c/5ecf9d4f52ff
  - [net,05/11] tcp: annotate data-races around tp->keepalive_probes
    https://git.kernel.org/netdev/net/c/6e5e1de616bf
  - [net,06/11] tcp: annotate data-races around icsk->icsk_syn_retries
    https://git.kernel.org/netdev/net/c/3a037f0f3c4b
  - [net,07/11] tcp: annotate data-races around tp->linger2
    https://git.kernel.org/netdev/net/c/9df5335ca974
  - [net,08/11] tcp: annotate data-races around rskq_defer_accept
    https://git.kernel.org/netdev/net/c/ae488c74422f
  - [net,09/11] tcp: annotate data-races around tp->notsent_lowat
    https://git.kernel.org/netdev/net/c/1aeb87bc1440
  - [net,10/11] tcp: annotate data-races around icsk->icsk_user_timeout
    https://git.kernel.org/netdev/net/c/26023e91e12c
  - [net,11/11] tcp: annotate data-races around fastopenq.max_qlen
    https://git.kernel.org/netdev/net/c/70f360dd7042

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-07-20 19:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-19 21:28 [PATCH net 00/11] tcp: add missing annotations Eric Dumazet
2023-07-19 21:28 ` [PATCH net 01/11] tcp: annotate data-races around tp->tcp_tx_delay Eric Dumazet
2023-07-19 21:28 ` [PATCH net 02/11] tcp: annotate data-races around tp->tsoffset Eric Dumazet
2023-07-19 21:28 ` [PATCH net 03/11] tcp: annotate data-races around tp->keepalive_time Eric Dumazet
2023-07-19 21:28 ` [PATCH net 04/11] tcp: annotate data-races around tp->keepalive_intvl Eric Dumazet
2023-07-19 21:28 ` [PATCH net 05/11] tcp: annotate data-races around tp->keepalive_probes Eric Dumazet
2023-07-19 21:28 ` [PATCH net 06/11] tcp: annotate data-races around icsk->icsk_syn_retries Eric Dumazet
2023-07-19 21:28 ` [PATCH net 07/11] tcp: annotate data-races around tp->linger2 Eric Dumazet
2023-07-19 21:28 ` [PATCH net 08/11] tcp: annotate data-races around rskq_defer_accept Eric Dumazet
2023-07-19 21:28 ` [PATCH net 09/11] tcp: annotate data-races around tp->notsent_lowat Eric Dumazet
2023-07-19 21:28 ` [PATCH net 10/11] tcp: annotate data-races around icsk->icsk_user_timeout Eric Dumazet
2023-07-19 21:28 ` [PATCH net 11/11] tcp: annotate data-races around fastopenq.max_qlen Eric Dumazet
2023-07-20 19:50 ` [PATCH net 00/11] tcp: add missing annotations patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).