Both tcp_v4_err() and tcp_v6_err() do the following operations while they do not own the socket lock : fastopen = tp->fastopen_rsk; snd_una = fastopen ? tcp_rsk(fastopen)->snt_isn : tp->snd_una; The problem is that without appropriate barrier, the compiler might reload tp->fastopen_rsk and trigger a NULL deref. request sockets are protected by RCU, we can simply add the missing annotations and barriers to solve the issue.
Unified · Split
| @@ -393,7 +393,7 @@ struct tcp_sock { · 392 unmodified lines | |||
| 393 | /* fastopen_rsk points to request_sock that resulted in this big | 393 | /* fastopen_rsk points to request_sock that resulted in this big |
| 394 | * socket. Used to retransmit SYNACKs etc. | 394 | * socket. Used to retransmit SYNACKs etc. |
| 395 | */ | 395 | */ |
| 396 | struct request_sock *fastopen_rsk; | 396 | struct request_sock __rcu *fastopen_rsk; |
| 397 | u32 *saved_syn; | 397 | u32 *saved_syn; |
| 398 | }; | 398 | }; |
| 399 | 399 | ||
| @@ -447,8 +447,8 @@ static inline struct tcp_timewait_sock *tcp_twsk(const struct sock *sk) · 47 unmodified lines | |||
| 447 | 447 | ||
| 448 | static inline bool tcp_passive_fastopen(const struct sock *sk) | 448 | static inline bool tcp_passive_fastopen(const struct sock *sk) |
| 449 | { | 449 | { |
| 450 | return (sk->sk_state == TCP_SYN_RECV && | 450 | return sk->sk_state == TCP_SYN_RECV && |
| 451 | tcp_sk(sk)->fastopen_rsk != NULL); | 451 | rcu_access_pointer(tcp_sk(sk)->fastopen_rsk) != NULL; |
| 452 | } | 452 | } |
| 453 | 453 | ||
| 454 | static inline void fastopen_queue_tune(struct sock *sk, int backlog) | 454 | static inline void fastopen_queue_tune(struct sock *sk, int backlog) |
| @@ -96,7 +96,7 @@ void reqsk_fastopen_remove(struct sock *sk, struct request_sock *req, · 95 unmodified lines | |||
| 96 | 96 | ||
| 97 | fastopenq = &inet_csk(lsk)->icsk_accept_queue.fastopenq; | 97 | fastopenq = &inet_csk(lsk)->icsk_accept_queue.fastopenq; |
| 98 | 98 | ||
| 99 | tcp_sk(sk)->fastopen_rsk = NULL; | 99 | RCU_INIT_POINTER(tcp_sk(sk)->fastopen_rsk, NULL); |
| 100 | spin_lock_bh(&fastopenq->lock); | 100 | spin_lock_bh(&fastopenq->lock); |
| 101 | fastopenq->qlen--; | 101 | fastopenq->qlen--; |
| 102 | tcp_rsk(req)->tfo_listener = false; | 102 | tcp_rsk(req)->tfo_listener = false; |
| @@ -906,7 +906,7 @@ static void inet_child_forget(struct sock *sk, struct request_sock *req, · 905 unmodified lines | |||
| 906 | percpu_counter_inc(sk->sk_prot->orphan_count); | 906 | percpu_counter_inc(sk->sk_prot->orphan_count); |
| 907 | 907 | ||
| 908 | if (sk->sk_protocol == IPPROTO_TCP && tcp_rsk(req)->tfo_listener) { | 908 | if (sk->sk_protocol == IPPROTO_TCP && tcp_rsk(req)->tfo_listener) { |
| 909 | BUG_ON(tcp_sk(child)->fastopen_rsk != req); | 909 | BUG_ON(rcu_access_pointer(tcp_sk(child)->fastopen_rsk) != req); |
| 910 | BUG_ON(sk != req->rsk_listener); | 910 | BUG_ON(sk != req->rsk_listener); |
| 911 | 911 | ||
| 912 | /* Paranoid, to prevent race condition if | 912 | /* Paranoid, to prevent race condition if |
| @@ -915,7 +915,7 @@ static void inet_child_forget(struct sock *sk, struct request_sock *req, · 2 unmodified lines | |||
| 915 | * Also to satisfy an assertion in | 915 | * Also to satisfy an assertion in |
| 916 | * tcp_v4_destroy_sock(). | 916 | * tcp_v4_destroy_sock(). |
| 917 | */ | 917 | */ |
| 918 | tcp_sk(child)->fastopen_rsk = NULL; | 918 | RCU_INIT_POINTER(tcp_sk(child)->fastopen_rsk, NULL); |
| 919 | } | 919 | } |
| 920 | inet_csk_destroy_sock(child); | 920 | inet_csk_destroy_sock(child); |
| 921 | } | 921 | } |
| @@ -543,7 +543,7 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait) · 542 unmodified lines | |||
| 543 | 543 | ||
| 544 | /* Connected or passive Fast Open socket? */ | 544 | /* Connected or passive Fast Open socket? */ |
| 545 | if (state != TCP_SYN_SENT && | 545 | if (state != TCP_SYN_SENT && |
| 546 | (state != TCP_SYN_RECV || tp->fastopen_rsk)) { | 546 | (state != TCP_SYN_RECV || rcu_access_pointer(tp->fastopen_rsk))) { |
| 547 | int target = sock_rcvlowat(sk, 0, INT_MAX); | 547 | int target = sock_rcvlowat(sk, 0, INT_MAX); |
| 548 | 548 | ||
| 549 | if (tp->urg_seq == tp->copied_seq && | 549 | if (tp->urg_seq == tp->copied_seq && |
| @@ -2487,7 +2487,10 @@ adjudge_to_death: · 1937 unmodified lines | |||
| 2487 | } | 2487 | } |
| 2488 | 2488 | ||
| 2489 | if (sk->sk_state == TCP_CLOSE) { | 2489 | if (sk->sk_state == TCP_CLOSE) { |
| 2490 | struct request_sock *req = tcp_sk(sk)->fastopen_rsk; | 2490 | struct request_sock *req; |
| 2491 | |||
| 2492 | req = rcu_dereference_protected(tcp_sk(sk)->fastopen_rsk, | ||
| 2493 | lockdep_sock_is_held(sk)); | ||
| 2491 | /* We could get here with a non-NULL req if the socket is | 2494 | /* We could get here with a non-NULL req if the socket is |
| 2492 | * aborted (e.g., closed with unread data) before 3WHS | 2495 | * aborted (e.g., closed with unread data) before 3WHS |
| 2493 | * finishes. | 2496 | * finishes. |
| @@ -3831,8 +3834,10 @@ EXPORT_SYMBOL(tcp_md5_hash_key); · 1337 unmodified lines | |||
| 3831 | 3834 | ||
| 3832 | void tcp_done(struct sock *sk) | 3835 | void tcp_done(struct sock *sk) |
| 3833 | { | 3836 | { |
| 3834 | struct request_sock *req = tcp_sk(sk)->fastopen_rsk; | 3837 | struct request_sock *req; |
| 3835 | 3838 | ||
| 3839 | req = rcu_dereference_protected(tcp_sk(sk)->fastopen_rsk, | ||
| 3840 | lockdep_sock_is_held(sk)); | ||
| 3836 | if (sk->sk_state == TCP_SYN_SENT || sk->sk_state == TCP_SYN_RECV) | 3841 | if (sk->sk_state == TCP_SYN_SENT || sk->sk_state == TCP_SYN_RECV) |
| 3837 | TCP_INC_STATS(sock_net(sk), TCP_MIB_ATTEMPTFAILS); | 3842 | TCP_INC_STATS(sock_net(sk), TCP_MIB_ATTEMPTFAILS); |
| 3838 | 3843 | ||
| @@ -253,7 +253,7 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk, · 252 unmodified lines | |||
| 253 | */ | 253 | */ |
| 254 | tp = tcp_sk(child); | 254 | tp = tcp_sk(child); |
| 255 | 255 | ||
| 256 | tp->fastopen_rsk = req; | 256 | rcu_assign_pointer(tp->fastopen_rsk, req); |
| 257 | tcp_rsk(req)->tfo_listener = true; | 257 | tcp_rsk(req)->tfo_listener = true; |
| 258 | 258 | ||
| 259 | /* RFC1323: The window in SYN & SYN/ACK segments is never | 259 | /* RFC1323: The window in SYN & SYN/ACK segments is never |
| @@ -2666,7 +2666,7 @@ static void tcp_process_loss(struct sock *sk, int flag, int num_dupack, · 2665 unmodified lines | |||
| 2666 | struct tcp_sock *tp = tcp_sk(sk); | 2666 | struct tcp_sock *tp = tcp_sk(sk); |
| 2667 | bool recovered = !before(tp->snd_una, tp->high_seq); | 2667 | bool recovered = !before(tp->snd_una, tp->high_seq); |
| 2668 | 2668 | ||
| 2669 | if ((flag & FLAG_SND_UNA_ADVANCED || tp->fastopen_rsk) && | 2669 | if ((flag & FLAG_SND_UNA_ADVANCED || rcu_access_pointer(tp->fastopen_rsk)) && |
| 2670 | tcp_try_undo_loss(sk, false)) | 2670 | tcp_try_undo_loss(sk, false)) |
| 2671 | return; | 2671 | return; |
| 2672 | 2672 | ||
| @@ -2990,7 +2990,7 @@ void tcp_rearm_rto(struct sock *sk) · 317 unmodified lines | |||
| 2990 | /* If the retrans timer is currently being used by Fast Open | 2990 | /* If the retrans timer is currently being used by Fast Open |
| 2991 | * for SYN-ACK retrans purpose, stay put. | 2991 | * for SYN-ACK retrans purpose, stay put. |
| 2992 | */ | 2992 | */ |
| 2993 | if (tp->fastopen_rsk) | 2993 | if (rcu_access_pointer(tp->fastopen_rsk)) |
| 2994 | return; | 2994 | return; |
| 2995 | 2995 | ||
| 2996 | if (!tp->packets_out) { | 2996 | if (!tp->packets_out) { |
| @@ -6087,6 +6087,8 @@ reset_and_undo: · 3090 unmodified lines | |||
| 6087 | 6087 | ||
| 6088 | static void tcp_rcv_synrecv_state_fastopen(struct sock *sk) | 6088 | static void tcp_rcv_synrecv_state_fastopen(struct sock *sk) |
| 6089 | { | 6089 | { |
| 6090 | struct request_sock *req; | ||
| 6091 | |||
| 6090 | tcp_try_undo_loss(sk, false); | 6092 | tcp_try_undo_loss(sk, false); |
| 6091 | 6093 | ||
| 6092 | /* Reset rtx states to prevent spurious retransmits_timed_out() */ | 6094 | /* Reset rtx states to prevent spurious retransmits_timed_out() */ |
| @@ -6096,7 +6098,9 @@ static void tcp_rcv_synrecv_state_fastopen(struct sock *sk) · 3 unmodified lines | |||
| 6096 | /* Once we leave TCP_SYN_RECV or TCP_FIN_WAIT_1, | 6098 | /* Once we leave TCP_SYN_RECV or TCP_FIN_WAIT_1, |
| 6097 | * we no longer need req so release it. | 6099 | * we no longer need req so release it. |
| 6098 | */ | 6100 | */ |
| 6099 | reqsk_fastopen_remove(sk, tcp_sk(sk)->fastopen_rsk, false); | 6101 | req = rcu_dereference_protected(tcp_sk(sk)->fastopen_rsk, |
| 6102 | lockdep_sock_is_held(sk)); | ||
| 6103 | reqsk_fastopen_remove(sk, req, false); | ||
| 6100 | 6104 | ||
| 6101 | /* Re-arm the timer because data may have been sent out. | 6105 | /* Re-arm the timer because data may have been sent out. |
| 6102 | * This is similar to the regular data transmission case | 6106 | * This is similar to the regular data transmission case |
| @@ -6171,7 +6175,8 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb) · 68 unmodified lines | |||
| 6171 | 6175 | ||
| 6172 | tcp_mstamp_refresh(tp); | 6176 | tcp_mstamp_refresh(tp); |
| 6173 | tp->rx_opt.saw_tstamp = 0; | 6177 | tp->rx_opt.saw_tstamp = 0; |
| 6174 | req = tp->fastopen_rsk; | 6178 | req = rcu_dereference_protected(tp->fastopen_rsk, |
| 6179 | lockdep_sock_is_held(sk)); | ||
| 6175 | if (req) { | 6180 | if (req) { |
| 6176 | bool req_stolen; | 6181 | bool req_stolen; |
| 6177 | 6182 | ||
| @@ -478,7 +478,7 @@ int tcp_v4_err(struct sk_buff *icmp_skb, u32 info) · 477 unmodified lines | |||
| 478 | icsk = inet_csk(sk); | 478 | icsk = inet_csk(sk); |
| 479 | tp = tcp_sk(sk); | 479 | tp = tcp_sk(sk); |
| 480 | /* XXX (TFO) - tp->snd_una should be ISN (tcp_create_openreq_child() */ | 480 | /* XXX (TFO) - tp->snd_una should be ISN (tcp_create_openreq_child() */ |
| 481 | fastopen = tp->fastopen_rsk; | 481 | fastopen = rcu_dereference(tp->fastopen_rsk); |
| 482 | snd_una = fastopen ? tcp_rsk(fastopen)->snt_isn : tp->snd_una; | 482 | snd_una = fastopen ? tcp_rsk(fastopen)->snt_isn : tp->snd_una; |
| 483 | if (sk->sk_state != TCP_LISTEN && | 483 | if (sk->sk_state != TCP_LISTEN && |
| 484 | !between(seq, snd_una, tp->snd_nxt)) { | 484 | !between(seq, snd_una, tp->snd_nxt)) { |
| @@ -2121,7 +2121,7 @@ void tcp_v4_destroy_sock(struct sock *sk) · 1636 unmodified lines | |||
| 2121 | if (inet_csk(sk)->icsk_bind_hash) | 2121 | if (inet_csk(sk)->icsk_bind_hash) |
| 2122 | inet_put_port(sk); | 2122 | inet_put_port(sk); |
| 2123 | 2123 | ||
| 2124 | BUG_ON(tp->fastopen_rsk); | 2124 | BUG_ON(rcu_access_pointer(tp->fastopen_rsk)); |
| 2125 | 2125 | ||
| 2126 | /* If socket is aborted during connect operation */ | 2126 | /* If socket is aborted during connect operation */ |
| 2127 | tcp_free_fastopen_req(tp); | 2127 | tcp_free_fastopen_req(tp); |
| @@ -541,7 +541,7 @@ struct sock *tcp_create_openreq_child(const struct sock *sk, · 540 unmodified lines | |||
| 541 | newtp->rx_opt.mss_clamp = req->mss; | 541 | newtp->rx_opt.mss_clamp = req->mss; |
| 542 | tcp_ecn_openreq_child(newtp, req); | 542 | tcp_ecn_openreq_child(newtp, req); |
| 543 | newtp->fastopen_req = NULL; | 543 | newtp->fastopen_req = NULL; |
| 544 | newtp->fastopen_rsk = NULL; | 544 | RCU_INIT_POINTER(newtp->fastopen_rsk, NULL); |
| 545 | 545 | ||
| 546 | __TCP_INC_STATS(sock_net(sk), TCP_MIB_PASSIVEOPENS); | 546 | __TCP_INC_STATS(sock_net(sk), TCP_MIB_PASSIVEOPENS); |
| 547 | 547 | ||
| @@ -2482,7 +2482,7 @@ bool tcp_schedule_loss_probe(struct sock *sk, bool advancing_rto) · 2481 unmodified lines | |||
| 2482 | /* Don't do any loss probe on a Fast Open connection before 3WHS | 2482 | /* Don't do any loss probe on a Fast Open connection before 3WHS |
| 2483 | * finishes. | 2483 | * finishes. |
| 2484 | */ | 2484 | */ |
| 2485 | if (tp->fastopen_rsk) | 2485 | if (rcu_access_pointer(tp->fastopen_rsk)) |
| 2486 | return false; | 2486 | return false; |
| 2487 | 2487 | ||
| 2488 | early_retrans = sock_net(sk)->ipv4.sysctl_tcp_early_retrans; | 2488 | early_retrans = sock_net(sk)->ipv4.sysctl_tcp_early_retrans; |
| @@ -386,15 +386,13 @@ abort: tcp_write_err(sk); · 385 unmodified lines | |||
| 386 | * Timer for Fast Open socket to retransmit SYNACK. Note that the | 386 | * Timer for Fast Open socket to retransmit SYNACK. Note that the |
| 387 | * sk here is the child socket, not the parent (listener) socket. | 387 | * sk here is the child socket, not the parent (listener) socket. |
| 388 | */ | 388 | */ |
| 389 | static void tcp_fastopen_synack_timer(struct sock *sk) | 389 | static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req) |
| 390 | { | 390 | { |
| 391 | struct inet_connection_sock *icsk = inet_csk(sk); | 391 | struct inet_connection_sock *icsk = inet_csk(sk); |
| 392 | int max_retries = icsk->icsk_syn_retries ? : | 392 | int max_retries = icsk->icsk_syn_retries ? : |
| 393 | sock_net(sk)->ipv4.sysctl_tcp_synack_retries + 1; /* add one more retry for fastopen */ | 393 | sock_net(sk)->ipv4.sysctl_tcp_synack_retries + 1; /* add one more retry for fastopen */ |
| 394 | struct tcp_sock *tp = tcp_sk(sk); | 394 | struct tcp_sock *tp = tcp_sk(sk); |
| 395 | struct request_sock *req; | ||
| 396 | 395 | ||
| 397 | req = tcp_sk(sk)->fastopen_rsk; | ||
| 398 | req->rsk_ops->syn_ack_timeout(req); | 396 | req->rsk_ops->syn_ack_timeout(req); |
| 399 | 397 | ||
| 400 | if (req->num_timeout >= max_retries) { | 398 | if (req->num_timeout >= max_retries) { |
| @@ -435,11 +433,14 @@ void tcp_retransmit_timer(struct sock *sk) · 34 unmodified lines | |||
| 435 | struct tcp_sock *tp = tcp_sk(sk); | 433 | struct tcp_sock *tp = tcp_sk(sk); |
| 436 | struct net *net = sock_net(sk); | 434 | struct net *net = sock_net(sk); |
| 437 | struct inet_connection_sock *icsk = inet_csk(sk); | 435 | struct inet_connection_sock *icsk = inet_csk(sk); |
| 436 | struct request_sock *req; | ||
| 438 | 437 | ||
| 439 | if (tp->fastopen_rsk) { | 438 | req = rcu_dereference_protected(tp->fastopen_rsk, |
| 439 | lockdep_sock_is_held(sk)); | ||
| 440 | if (req) { | ||
| 440 | WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV && | 441 | WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV && |
| 441 | sk->sk_state != TCP_FIN_WAIT1); | 442 | sk->sk_state != TCP_FIN_WAIT1); |
| 442 | tcp_fastopen_synack_timer(sk); | 443 | tcp_fastopen_synack_timer(sk, req); |
| 443 | /* Before we receive ACK to our SYN-ACK don't retransmit | 444 | /* Before we receive ACK to our SYN-ACK don't retransmit |
| 444 | * anything else (e.g., data or FIN segments). | 445 | * anything else (e.g., data or FIN segments). |
| 445 | */ | 446 | */ |
| @@ -406,7 +406,7 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt, · 405 unmodified lines | |||
| 406 | 406 | ||
| 407 | tp = tcp_sk(sk); | 407 | tp = tcp_sk(sk); |
| 408 | /* XXX (TFO) - tp->snd_una should be ISN (tcp_create_openreq_child() */ | 408 | /* XXX (TFO) - tp->snd_una should be ISN (tcp_create_openreq_child() */ |
| 409 | fastopen = tp->fastopen_rsk; | 409 | fastopen = rcu_dereference(tp->fastopen_rsk); |
| 410 | snd_una = fastopen ? tcp_rsk(fastopen)->snt_isn : tp->snd_una; | 410 | snd_una = fastopen ? tcp_rsk(fastopen)->snt_isn : tp->snd_una; |
| 411 | if (sk->sk_state != TCP_LISTEN && | 411 | if (sk->sk_state != TCP_LISTEN && |
| 412 | !between(seq, snd_una, tp->snd_nxt)) { | 412 | !between(seq, snd_una, tp->snd_nxt)) { |