aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid S. Miller <davem@davemloft.net>2008-06-12 19:31:35 -0400
committerDavid S. Miller <davem@davemloft.net>2008-06-12 19:34:35 -0400
commitec0a196626bd12e0ba108d7daa6d95a4fb25c2c5 (patch)
tree68d9c2923765e12853368e8edb27b241142e0c48
parentf23d60de719e639690b2dc5c2d0e4243ff614b7a (diff)
tcp: Revert 'process defer accept as established' changes.
This reverts two changesets, ec3c0982a2dd1e671bad8e9d26c28dcba0039d87 ("[TCP]: TCP_DEFER_ACCEPT updates - process as established") and the follow-on bug fix 9ae27e0adbf471c7a6b80102e38e1d5a346b3b38 ("tcp: Fix slab corruption with ipv6 and tcp6fuzz"). This change causes several problems, first reported by Ingo Molnar as a distcc-over-loopback regression where connections were getting stuck. Ilpo Järvinen first spotted the locking problems. The new function added by this code, tcp_defer_accept_check(), only has the child socket locked, yet it is modifying state of the parent listening socket. Fixing that is non-trivial at best, because we can't simply just grab the parent listening socket lock at this point, because it would create an ABBA deadlock. The normal ordering is parent listening socket --> child socket, but this code path would require the reverse lock ordering. Next is a problem noticed by Vitaliy Gusev, he noted: ---------------------------------------- >--- a/net/ipv4/tcp_timer.c >+++ b/net/ipv4/tcp_timer.c >@@ -481,6 +481,11 @@ static void tcp_keepalive_timer (unsigned long data) > goto death; > } > >+ if (tp->defer_tcp_accept.request && sk->sk_state == TCP_ESTABLISHED) { >+ tcp_send_active_reset(sk, GFP_ATOMIC); >+ goto death; Here socket sk is not attached to listening socket's request queue. tcp_done() will not call inet_csk_destroy_sock() (and tcp_v4_destroy_sock() which should release this sk) as socket is not DEAD. Therefore socket sk will be lost for freeing. ---------------------------------------- Finally, Alexey Kuznetsov argues that there might not even be any real value or advantage to these new semantics even if we fix all of the bugs: ---------------------------------------- Hiding from accept() sockets with only out-of-order data only is the only thing which is impossible with old approach. Is this really so valuable? My opinion: no, this is nothing but a new loophole to consume memory without control. ---------------------------------------- So revert this thing for now. Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--include/linux/tcp.h7
-rw-r--r--include/net/request_sock.h4
-rw-r--r--include/net/tcp.h1
-rw-r--r--net/ipv4/inet_connection_sock.c11
-rw-r--r--net/ipv4/tcp.c18
-rw-r--r--net/ipv4/tcp_input.c45
-rw-r--r--net/ipv4/tcp_ipv4.c8
-rw-r--r--net/ipv4/tcp_minisocks.c32
-rw-r--r--net/ipv4/tcp_timer.c5
9 files changed, 33 insertions, 98 deletions
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 18e62e3d406f..b31b6b74aa28 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -239,11 +239,6 @@ static inline struct tcp_request_sock *tcp_rsk(const struct request_sock *req)
239 return (struct tcp_request_sock *)req; 239 return (struct tcp_request_sock *)req;
240} 240}
241 241
242struct tcp_deferred_accept_info {
243 struct sock *listen_sk;
244 struct request_sock *request;
245};
246
247struct tcp_sock { 242struct tcp_sock {
248 /* inet_connection_sock has to be the first member of tcp_sock */ 243 /* inet_connection_sock has to be the first member of tcp_sock */
249 struct inet_connection_sock inet_conn; 244 struct inet_connection_sock inet_conn;
@@ -379,8 +374,6 @@ struct tcp_sock {
379 unsigned int keepalive_intvl; /* time interval between keep alive probes */ 374 unsigned int keepalive_intvl; /* time interval between keep alive probes */
380 int linger2; 375 int linger2;
381 376
382 struct tcp_deferred_accept_info defer_tcp_accept;
383
384 unsigned long last_synq_overflow; 377 unsigned long last_synq_overflow;
385 378
386 u32 tso_deferred; 379 u32 tso_deferred;
diff --git a/include/net/request_sock.h b/include/net/request_sock.h
index b220b5f624de..0c96e7bed5db 100644
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h
@@ -115,8 +115,8 @@ struct request_sock_queue {
115 struct request_sock *rskq_accept_head; 115 struct request_sock *rskq_accept_head;
116 struct request_sock *rskq_accept_tail; 116 struct request_sock *rskq_accept_tail;
117 rwlock_t syn_wait_lock; 117 rwlock_t syn_wait_lock;
118 u16 rskq_defer_accept; 118 u8 rskq_defer_accept;
119 /* 2 bytes hole, try to pack */ 119 /* 3 bytes hole, try to pack */
120 struct listen_sock *listen_opt; 120 struct listen_sock *listen_opt;
121}; 121};
122 122
diff --git a/include/net/tcp.h b/include/net/tcp.h
index d448310c82c1..cf54034019d9 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -139,7 +139,6 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
139#define MAX_TCP_KEEPINTVL 32767 139#define MAX_TCP_KEEPINTVL 32767
140#define MAX_TCP_KEEPCNT 127 140#define MAX_TCP_KEEPCNT 127
141#define MAX_TCP_SYNCNT 127 141#define MAX_TCP_SYNCNT 127
142#define MAX_TCP_ACCEPT_DEFERRED 65535
143 142
144#define TCP_SYNQ_INTERVAL (HZ/5) /* Period of SYNACK timer */ 143#define TCP_SYNQ_INTERVAL (HZ/5) /* Period of SYNACK timer */
145 144
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 828ea211ff21..045e799d3e1d 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -419,7 +419,8 @@ void inet_csk_reqsk_queue_prune(struct sock *parent,
419 struct inet_connection_sock *icsk = inet_csk(parent); 419 struct inet_connection_sock *icsk = inet_csk(parent);
420 struct request_sock_queue *queue = &icsk->icsk_accept_queue; 420 struct request_sock_queue *queue = &icsk->icsk_accept_queue;
421 struct listen_sock *lopt = queue->listen_opt; 421 struct listen_sock *lopt = queue->listen_opt;
422 int thresh = icsk->icsk_syn_retries ? : sysctl_tcp_synack_retries; 422 int max_retries = icsk->icsk_syn_retries ? : sysctl_tcp_synack_retries;
423 int thresh = max_retries;
423 unsigned long now = jiffies; 424 unsigned long now = jiffies;
424 struct request_sock **reqp, *req; 425 struct request_sock **reqp, *req;
425 int i, budget; 426 int i, budget;
@@ -455,6 +456,9 @@ void inet_csk_reqsk_queue_prune(struct sock *parent,
455 } 456 }
456 } 457 }
457 458
459 if (queue->rskq_defer_accept)
460 max_retries = queue->rskq_defer_accept;
461
458 budget = 2 * (lopt->nr_table_entries / (timeout / interval)); 462 budget = 2 * (lopt->nr_table_entries / (timeout / interval));
459 i = lopt->clock_hand; 463 i = lopt->clock_hand;
460 464
@@ -462,8 +466,9 @@ void inet_csk_reqsk_queue_prune(struct sock *parent,
462 reqp=&lopt->syn_table[i]; 466 reqp=&lopt->syn_table[i];
463 while ((req = *reqp) != NULL) { 467 while ((req = *reqp) != NULL) {
464 if (time_after_eq(now, req->expires)) { 468 if (time_after_eq(now, req->expires)) {
465 if (req->retrans < thresh && 469 if ((req->retrans < (inet_rsk(req)->acked ? max_retries : thresh)) &&
466 !req->rsk_ops->rtx_syn_ack(parent, req)) { 470 (inet_rsk(req)->acked ||
471 !req->rsk_ops->rtx_syn_ack(parent, req))) {
467 unsigned long timeo; 472 unsigned long timeo;
468 473
469 if (req->retrans++ == 0) 474 if (req->retrans++ == 0)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index ab66683b8043..fc54a48fde1e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2112,12 +2112,15 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
2112 break; 2112 break;
2113 2113
2114 case TCP_DEFER_ACCEPT: 2114 case TCP_DEFER_ACCEPT:
2115 if (val < 0) { 2115 icsk->icsk_accept_queue.rskq_defer_accept = 0;
2116 err = -EINVAL; 2116 if (val > 0) {
2117 } else { 2117 /* Translate value in seconds to number of
2118 if (val > MAX_TCP_ACCEPT_DEFERRED) 2118 * retransmits */
2119 val = MAX_TCP_ACCEPT_DEFERRED; 2119 while (icsk->icsk_accept_queue.rskq_defer_accept < 32 &&
2120 icsk->icsk_accept_queue.rskq_defer_accept = val; 2120 val > ((TCP_TIMEOUT_INIT / HZ) <<
2121 icsk->icsk_accept_queue.rskq_defer_accept))
2122 icsk->icsk_accept_queue.rskq_defer_accept++;
2123 icsk->icsk_accept_queue.rskq_defer_accept++;
2121 } 2124 }
2122 break; 2125 break;
2123 2126
@@ -2299,7 +2302,8 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
2299 val = (val ? : sysctl_tcp_fin_timeout) / HZ; 2302 val = (val ? : sysctl_tcp_fin_timeout) / HZ;
2300 break; 2303 break;
2301 case TCP_DEFER_ACCEPT: 2304 case TCP_DEFER_ACCEPT:
2302 val = icsk->icsk_accept_queue.rskq_defer_accept; 2305 val = !icsk->icsk_accept_queue.rskq_defer_accept ? 0 :
2306 ((TCP_TIMEOUT_INIT / HZ) << (icsk->icsk_accept_queue.rskq_defer_accept - 1));
2303 break; 2307 break;
2304 case TCP_WINDOW_CLAMP: 2308 case TCP_WINDOW_CLAMP:
2305 val = tp->window_clamp; 2309 val = tp->window_clamp;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index eba873e9b560..cad73b7dfef0 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4541,49 +4541,6 @@ static void tcp_urg(struct sock *sk, struct sk_buff *skb, struct tcphdr *th)
4541 } 4541 }
4542} 4542}
4543 4543
4544static int tcp_defer_accept_check(struct sock *sk)
4545{
4546 struct tcp_sock *tp = tcp_sk(sk);
4547
4548 if (tp->defer_tcp_accept.request) {
4549 int queued_data = tp->rcv_nxt - tp->copied_seq;
4550 int hasfin = !skb_queue_empty(&sk->sk_receive_queue) ?
4551 tcp_hdr((struct sk_buff *)
4552 sk->sk_receive_queue.prev)->fin : 0;
4553
4554 if (queued_data && hasfin)
4555 queued_data--;
4556
4557 if (queued_data &&
4558 tp->defer_tcp_accept.listen_sk->sk_state == TCP_LISTEN) {
4559 if (sock_flag(sk, SOCK_KEEPOPEN)) {
4560 inet_csk_reset_keepalive_timer(sk,
4561 keepalive_time_when(tp));
4562 } else {
4563 inet_csk_delete_keepalive_timer(sk);
4564 }
4565
4566 inet_csk_reqsk_queue_add(
4567 tp->defer_tcp_accept.listen_sk,
4568 tp->defer_tcp_accept.request,
4569 sk);
4570
4571 tp->defer_tcp_accept.listen_sk->sk_data_ready(
4572 tp->defer_tcp_accept.listen_sk, 0);
4573
4574 sock_put(tp->defer_tcp_accept.listen_sk);
4575 sock_put(sk);
4576 tp->defer_tcp_accept.listen_sk = NULL;
4577 tp->defer_tcp_accept.request = NULL;
4578 } else if (hasfin ||
4579 tp->defer_tcp_accept.listen_sk->sk_state != TCP_LISTEN) {
4580 tcp_reset(sk);
4581 return -1;
4582 }
4583 }
4584 return 0;
4585}
4586
4587static int tcp_copy_to_iovec(struct sock *sk, struct sk_buff *skb, int hlen) 4544static int tcp_copy_to_iovec(struct sock *sk, struct sk_buff *skb, int hlen)
4588{ 4545{
4589 struct tcp_sock *tp = tcp_sk(sk); 4546 struct tcp_sock *tp = tcp_sk(sk);
@@ -4944,8 +4901,6 @@ step5:
4944 4901
4945 tcp_data_snd_check(sk); 4902 tcp_data_snd_check(sk);
4946 tcp_ack_snd_check(sk); 4903 tcp_ack_snd_check(sk);
4947
4948 tcp_defer_accept_check(sk);
4949 return 0; 4904 return 0;
4950 4905
4951csum_error: 4906csum_error:
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 4f8485c67d1a..97a230026e13 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1918,14 +1918,6 @@ int tcp_v4_destroy_sock(struct sock *sk)
1918 sk->sk_sndmsg_page = NULL; 1918 sk->sk_sndmsg_page = NULL;
1919 } 1919 }
1920 1920
1921 if (tp->defer_tcp_accept.request) {
1922 reqsk_free(tp->defer_tcp_accept.request);
1923 sock_put(tp->defer_tcp_accept.listen_sk);
1924 sock_put(sk);
1925 tp->defer_tcp_accept.listen_sk = NULL;
1926 tp->defer_tcp_accept.request = NULL;
1927 }
1928
1929 atomic_dec(&tcp_sockets_allocated); 1921 atomic_dec(&tcp_sockets_allocated);
1930 1922
1931 return 0; 1923 return 0;
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 019c8c16e5cc..8245247a6ceb 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -571,8 +571,10 @@ struct sock *tcp_check_req(struct sock *sk,struct sk_buff *skb,
571 does sequence test, SYN is truncated, and thus we consider 571 does sequence test, SYN is truncated, and thus we consider
572 it a bare ACK. 572 it a bare ACK.
573 573
574 Both ends (listening sockets) accept the new incoming 574 If icsk->icsk_accept_queue.rskq_defer_accept, we silently drop this
575 connection and try to talk to each other. 8-) 575 bare ACK. Otherwise, we create an established connection. Both
576 ends (listening sockets) accept the new incoming connection and try
577 to talk to each other. 8-)
576 578
577 Note: This case is both harmless, and rare. Possibility is about the 579 Note: This case is both harmless, and rare. Possibility is about the
578 same as us discovering intelligent life on another plant tomorrow. 580 same as us discovering intelligent life on another plant tomorrow.
@@ -640,6 +642,13 @@ struct sock *tcp_check_req(struct sock *sk,struct sk_buff *skb,
640 if (!(flg & TCP_FLAG_ACK)) 642 if (!(flg & TCP_FLAG_ACK))
641 return NULL; 643 return NULL;
642 644
645 /* If TCP_DEFER_ACCEPT is set, drop bare ACK. */
646 if (inet_csk(sk)->icsk_accept_queue.rskq_defer_accept &&
647 TCP_SKB_CB(skb)->end_seq == tcp_rsk(req)->rcv_isn + 1) {
648 inet_rsk(req)->acked = 1;
649 return NULL;
650 }
651
643 /* OK, ACK is valid, create big socket and 652 /* OK, ACK is valid, create big socket and
644 * feed this segment to it. It will repeat all 653 * feed this segment to it. It will repeat all
645 * the tests. THIS SEGMENT MUST MOVE SOCKET TO 654 * the tests. THIS SEGMENT MUST MOVE SOCKET TO
@@ -678,24 +687,7 @@ struct sock *tcp_check_req(struct sock *sk,struct sk_buff *skb,
678 inet_csk_reqsk_queue_unlink(sk, req, prev); 687 inet_csk_reqsk_queue_unlink(sk, req, prev);
679 inet_csk_reqsk_queue_removed(sk, req); 688 inet_csk_reqsk_queue_removed(sk, req);
680 689
681 if (inet_csk(sk)->icsk_accept_queue.rskq_defer_accept && 690 inet_csk_reqsk_queue_add(sk, req, child);
682 TCP_SKB_CB(skb)->end_seq == tcp_rsk(req)->rcv_isn + 1) {
683
684 /* the accept queue handling is done is est recv slow
685 * path so lets make sure to start there
686 */
687 tcp_sk(child)->pred_flags = 0;
688 sock_hold(sk);
689 sock_hold(child);
690 tcp_sk(child)->defer_tcp_accept.listen_sk = sk;
691 tcp_sk(child)->defer_tcp_accept.request = req;
692
693 inet_csk_reset_keepalive_timer(child,
694 inet_csk(sk)->icsk_accept_queue.rskq_defer_accept * HZ);
695 } else {
696 inet_csk_reqsk_queue_add(sk, req, child);
697 }
698
699 return child; 691 return child;
700 692
701 listen_overflow: 693 listen_overflow:
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 4de68cf5f2aa..63ed9d6830e7 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -489,11 +489,6 @@ static void tcp_keepalive_timer (unsigned long data)
489 goto death; 489 goto death;
490 } 490 }
491 491
492 if (tp->defer_tcp_accept.request && sk->sk_state == TCP_ESTABLISHED) {
493 tcp_send_active_reset(sk, GFP_ATOMIC);
494 goto death;
495 }
496
497 if (!sock_flag(sk, SOCK_KEEPOPEN) || sk->sk_state == TCP_CLOSE) 492 if (!sock_flag(sk, SOCK_KEEPOPEN) || sk->sk_state == TCP_CLOSE)
498 goto out; 493 goto out;
499 494