Skip to content

Commit 7998c0a

Browse files
kuba-mookuba-moo
committed
Merge branch 'tcp-add-missing-annotations'
Eric Dumazet says: ==================== tcp: add missing annotations 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. ==================== Link: https://lore.kernel.org/r/20230719212857.3943972-1-edumazet@google.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents ac52864 + 70f360d commit 7998c0a

File tree

6 files changed

+63
-40
lines changed

6 files changed

+63
-40
lines changed

‎include/linux/tcp.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ static inline void fastopen_queue_tune(struct sock *sk, int backlog)
513513
struct request_sock_queue *queue = &inet_csk(sk)->icsk_accept_queue;
514514
int somaxconn = READ_ONCE(sock_net(sk)->core.sysctl_somaxconn);
515515

516-
queue->fastopenq.max_qlen = min_t(unsigned int, backlog, somaxconn);
516+
WRITE_ONCE(queue->fastopenq.max_qlen, min_t(unsigned int, backlog, somaxconn));
517517
}
518518

519519
static inline void tcp_move_syn(struct tcp_sock *tp,

‎include/net/tcp.h‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,25 +1509,38 @@ void tcp_leave_memory_pressure(struct sock *sk);
15091509
static inline int keepalive_intvl_when(const struct tcp_sock *tp)
15101510
{
15111511
struct net *net = sock_net((struct sock *)tp);
1512+
int val;
15121513

1513-
return tp->keepalive_intvl ? :
1514-
READ_ONCE(net->ipv4.sysctl_tcp_keepalive_intvl);
1514+
/* Paired with WRITE_ONCE() in tcp_sock_set_keepintvl()
1515+
* and do_tcp_setsockopt().
1516+
*/
1517+
val = READ_ONCE(tp->keepalive_intvl);
1518+
1519+
return val ? : READ_ONCE(net->ipv4.sysctl_tcp_keepalive_intvl);
15151520
}
15161521

15171522
static inline int keepalive_time_when(const struct tcp_sock *tp)
15181523
{
15191524
struct net *net = sock_net((struct sock *)tp);
1525+
int val;
15201526

1521-
return tp->keepalive_time ? :
1522-
READ_ONCE(net->ipv4.sysctl_tcp_keepalive_time);
1527+
/* Paired with WRITE_ONCE() in tcp_sock_set_keepidle_locked() */
1528+
val = READ_ONCE(tp->keepalive_time);
1529+
1530+
return val ? : READ_ONCE(net->ipv4.sysctl_tcp_keepalive_time);
15231531
}
15241532

15251533
static inline int keepalive_probes(const struct tcp_sock *tp)
15261534
{
15271535
struct net *net = sock_net((struct sock *)tp);
1536+
int val;
15281537

1529-
return tp->keepalive_probes ? :
1530-
READ_ONCE(net->ipv4.sysctl_tcp_keepalive_probes);
1538+
/* Paired with WRITE_ONCE() in tcp_sock_set_keepcnt()
1539+
* and do_tcp_setsockopt().
1540+
*/
1541+
val = READ_ONCE(tp->keepalive_probes);
1542+
1543+
return val ? : READ_ONCE(net->ipv4.sysctl_tcp_keepalive_probes);
15311544
}
15321545

15331546
static inline u32 keepalive_time_elapsed(const struct tcp_sock *tp)
@@ -2048,7 +2061,11 @@ void __tcp_v4_send_check(struct sk_buff *skb, __be32 saddr, __be32 daddr);
20482061
static inline u32 tcp_notsent_lowat(const struct tcp_sock *tp)
20492062
{
20502063
struct net *net = sock_net((struct sock *)tp);
2051-
return tp->notsent_lowat ?: READ_ONCE(net->ipv4.sysctl_tcp_notsent_lowat);
2064+
u32 val;
2065+
2066+
val = READ_ONCE(tp->notsent_lowat);
2067+
2068+
return val ?: READ_ONCE(net->ipv4.sysctl_tcp_notsent_lowat);
20522069
}
20532070

20542071
bool tcp_stream_memory_free(const struct sock *sk, int wake);

‎net/ipv4/inet_connection_sock.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,7 @@ static void reqsk_timer_handler(struct timer_list *t)
10191019

10201020
icsk = inet_csk(sk_listener);
10211021
net = sock_net(sk_listener);
1022-
max_syn_ack_retries = icsk->icsk_syn_retries ? :
1022+
max_syn_ack_retries = READ_ONCE(icsk->icsk_syn_retries) ? :
10231023
READ_ONCE(net->ipv4.sysctl_tcp_synack_retries);
10241024
/* Normally all the openreqs are young and become mature
10251025
* (i.e. converted to established socket) for first timeout.

‎net/ipv4/tcp.c‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3291,7 +3291,7 @@ int tcp_sock_set_syncnt(struct sock *sk, int val)
32913291
return -EINVAL;
32923292

32933293
lock_sock(sk);
3294-
inet_csk(sk)->icsk_syn_retries = val;
3294+
WRITE_ONCE(inet_csk(sk)->icsk_syn_retries, val);
32953295
release_sock(sk);
32963296
return 0;
32973297
}
@@ -3300,7 +3300,7 @@ EXPORT_SYMBOL(tcp_sock_set_syncnt);
33003300
void tcp_sock_set_user_timeout(struct sock *sk, u32 val)
33013301
{
33023302
lock_sock(sk);
3303-
inet_csk(sk)->icsk_user_timeout = val;
3303+
WRITE_ONCE(inet_csk(sk)->icsk_user_timeout, val);
33043304
release_sock(sk);
33053305
}
33063306
EXPORT_SYMBOL(tcp_sock_set_user_timeout);
@@ -3312,7 +3312,8 @@ int tcp_sock_set_keepidle_locked(struct sock *sk, int val)
33123312
if (val < 1 || val > MAX_TCP_KEEPIDLE)
33133313
return -EINVAL;
33143314

3315-
tp->keepalive_time = val * HZ;
3315+
/* Paired with WRITE_ONCE() in keepalive_time_when() */
3316+
WRITE_ONCE(tp->keepalive_time, val * HZ);
33163317
if (sock_flag(sk, SOCK_KEEPOPEN) &&
33173318
!((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) {
33183319
u32 elapsed = keepalive_time_elapsed(tp);
@@ -3344,7 +3345,7 @@ int tcp_sock_set_keepintvl(struct sock *sk, int val)
33443345
return -EINVAL;
33453346

33463347
lock_sock(sk);
3347-
tcp_sk(sk)->keepalive_intvl = val * HZ;
3348+
WRITE_ONCE(tcp_sk(sk)->keepalive_intvl, val * HZ);
33483349
release_sock(sk);
33493350
return 0;
33503351
}
@@ -3356,7 +3357,8 @@ int tcp_sock_set_keepcnt(struct sock *sk, int val)
33563357
return -EINVAL;
33573358

33583359
lock_sock(sk);
3359-
tcp_sk(sk)->keepalive_probes = val;
3360+
/* Paired with READ_ONCE() in keepalive_probes() */
3361+
WRITE_ONCE(tcp_sk(sk)->keepalive_probes, val);
33603362
release_sock(sk);
33613363
return 0;
33623364
}
@@ -3558,19 +3560,19 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
35583560
if (val < 1 || val > MAX_TCP_KEEPINTVL)
35593561
err = -EINVAL;
35603562
else
3561-
tp->keepalive_intvl = val * HZ;
3563+
WRITE_ONCE(tp->keepalive_intvl, val * HZ);
35623564
break;
35633565
case TCP_KEEPCNT:
35643566
if (val < 1 || val > MAX_TCP_KEEPCNT)
35653567
err = -EINVAL;
35663568
else
3567-
tp->keepalive_probes = val;
3569+
WRITE_ONCE(tp->keepalive_probes, val);
35683570
break;
35693571
case TCP_SYNCNT:
35703572
if (val < 1 || val > MAX_TCP_SYNCNT)
35713573
err = -EINVAL;
35723574
else
3573-
icsk->icsk_syn_retries = val;
3575+
WRITE_ONCE(icsk->icsk_syn_retries, val);
35743576
break;
35753577

35763578
case TCP_SAVE_SYN:
@@ -3583,18 +3585,18 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
35833585

35843586
case TCP_LINGER2:
35853587
if (val < 0)
3586-
tp->linger2 = -1;
3588+
WRITE_ONCE(tp->linger2, -1);
35873589
else if (val > TCP_FIN_TIMEOUT_MAX / HZ)
3588-
tp->linger2 = TCP_FIN_TIMEOUT_MAX;
3590+
WRITE_ONCE(tp->linger2, TCP_FIN_TIMEOUT_MAX);
35893591
else
3590-
tp->linger2 = val * HZ;
3592+
WRITE_ONCE(tp->linger2, val * HZ);
35913593
break;
35923594

35933595
case TCP_DEFER_ACCEPT:
35943596
/* Translate value in seconds to number of retransmits */
3595-
icsk->icsk_accept_queue.rskq_defer_accept =
3596-
secs_to_retrans(val, TCP_TIMEOUT_INIT / HZ,
3597-
TCP_RTO_MAX / HZ);
3597+
WRITE_ONCE(icsk->icsk_accept_queue.rskq_defer_accept,
3598+
secs_to_retrans(val, TCP_TIMEOUT_INIT / HZ,
3599+
TCP_RTO_MAX / HZ));
35983600
break;
35993601

36003602
case TCP_WINDOW_CLAMP:
@@ -3618,7 +3620,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
36183620
if (val < 0)
36193621
err = -EINVAL;
36203622
else
3621-
icsk->icsk_user_timeout = val;
3623+
WRITE_ONCE(icsk->icsk_user_timeout, val);
36223624
break;
36233625

36243626
case TCP_FASTOPEN:
@@ -3656,13 +3658,13 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
36563658
if (!tp->repair)
36573659
err = -EPERM;
36583660
else
3659-
tp->tsoffset = val - tcp_time_stamp_raw();
3661+
WRITE_ONCE(tp->tsoffset, val - tcp_time_stamp_raw());
36603662
break;
36613663
case TCP_REPAIR_WINDOW:
36623664
err = tcp_repair_set_window(tp, optval, optlen);
36633665
break;
36643666
case TCP_NOTSENT_LOWAT:
3665-
tp->notsent_lowat = val;
3667+
WRITE_ONCE(tp->notsent_lowat, val);
36663668
sk->sk_write_space(sk);
36673669
break;
36683670
case TCP_INQ:
@@ -3674,7 +3676,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
36743676
case TCP_TX_DELAY:
36753677
if (val)
36763678
tcp_enable_tx_delay();
3677-
tp->tcp_tx_delay = val;
3679+
WRITE_ONCE(tp->tcp_tx_delay, val);
36783680
break;
36793681
default:
36803682
err = -ENOPROTOOPT;
@@ -3991,17 +3993,18 @@ int do_tcp_getsockopt(struct sock *sk, int level,
39913993
val = keepalive_probes(tp);
39923994
break;
39933995
case TCP_SYNCNT:
3994-
val = icsk->icsk_syn_retries ? :
3996+
val = READ_ONCE(icsk->icsk_syn_retries) ? :
39953997
READ_ONCE(net->ipv4.sysctl_tcp_syn_retries);
39963998
break;
39973999
case TCP_LINGER2:
3998-
val = tp->linger2;
4000+
val = READ_ONCE(tp->linger2);
39994001
if (val >= 0)
40004002
val = (val ? : READ_ONCE(net->ipv4.sysctl_tcp_fin_timeout)) / HZ;
40014003
break;
40024004
case TCP_DEFER_ACCEPT:
4003-
val = retrans_to_secs(icsk->icsk_accept_queue.rskq_defer_accept,
4004-
TCP_TIMEOUT_INIT / HZ, TCP_RTO_MAX / HZ);
4005+
val = READ_ONCE(icsk->icsk_accept_queue.rskq_defer_accept);
4006+
val = retrans_to_secs(val, TCP_TIMEOUT_INIT / HZ,
4007+
TCP_RTO_MAX / HZ);
40054008
break;
40064009
case TCP_WINDOW_CLAMP:
40074010
val = tp->window_clamp;
@@ -4138,11 +4141,11 @@ int do_tcp_getsockopt(struct sock *sk, int level,
41384141
break;
41394142

41404143
case TCP_USER_TIMEOUT:
4141-
val = icsk->icsk_user_timeout;
4144+
val = READ_ONCE(icsk->icsk_user_timeout);
41424145
break;
41434146

41444147
case TCP_FASTOPEN:
4145-
val = icsk->icsk_accept_queue.fastopenq.max_qlen;
4148+
val = READ_ONCE(icsk->icsk_accept_queue.fastopenq.max_qlen);
41464149
break;
41474150

41484151
case TCP_FASTOPEN_CONNECT:
@@ -4154,14 +4157,14 @@ int do_tcp_getsockopt(struct sock *sk, int level,
41544157
break;
41554158

41564159
case TCP_TX_DELAY:
4157-
val = tp->tcp_tx_delay;
4160+
val = READ_ONCE(tp->tcp_tx_delay);
41584161
break;
41594162

41604163
case TCP_TIMESTAMP:
4161-
val = tcp_time_stamp_raw() + tp->tsoffset;
4164+
val = tcp_time_stamp_raw() + READ_ONCE(tp->tsoffset);
41624165
break;
41634166
case TCP_NOTSENT_LOWAT:
4164-
val = tp->notsent_lowat;
4167+
val = READ_ONCE(tp->notsent_lowat);
41654168
break;
41664169
case TCP_INQ:
41674170
val = tp->recvmsg_inq;

‎net/ipv4/tcp_fastopen.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
296296
static bool tcp_fastopen_queue_check(struct sock *sk)
297297
{
298298
struct fastopen_queue *fastopenq;
299+
int max_qlen;
299300

300301
/* Make sure the listener has enabled fastopen, and we don't
301302
* exceed the max # of pending TFO requests allowed before trying
@@ -308,10 +309,11 @@ static bool tcp_fastopen_queue_check(struct sock *sk)
308309
* temporarily vs a server not supporting Fast Open at all.
309310
*/
310311
fastopenq = &inet_csk(sk)->icsk_accept_queue.fastopenq;
311-
if (fastopenq->max_qlen == 0)
312+
max_qlen = READ_ONCE(fastopenq->max_qlen);
313+
if (max_qlen == 0)
312314
return false;
313315

314-
if (fastopenq->qlen >= fastopenq->max_qlen) {
316+
if (fastopenq->qlen >= max_qlen) {
315317
struct request_sock *req1;
316318
spin_lock(&fastopenq->lock);
317319
req1 = fastopenq->rskq_rst_head;

‎net/ipv4/tcp_ipv4.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,9 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
307307
inet->inet_daddr,
308308
inet->inet_sport,
309309
usin->sin_port));
310-
tp->tsoffset = secure_tcp_ts_off(net, inet->inet_saddr,
311-
inet->inet_daddr);
310+
WRITE_ONCE(tp->tsoffset,
311+
secure_tcp_ts_off(net, inet->inet_saddr,
312+
inet->inet_daddr));
312313
}
313314

314315
inet->inet_id = get_random_u16();

0 commit comments

Comments
 (0)