What really happens is:
app calls connect()
socket is set to SS_CONNECTING
-> signal
ICMP happens in between
connect is restarted
sees sk->err returns and clears error but does _not_ set SS_UNCONNECTED
application retries connect
inet_wait_for_connect (because the socket is still SYN_SENT)
-> sleeps forever because no wake up happens and no error
is set because it is already cleared.
Here is the better fix which always clears SS_CONNECTING, I'm still
waiting for Jason's feedback on this one. Seems the "fix" to move
the tcp_set_state(TCP_CLOSE) out of the icmp error handler was worse
medicine than the original problem.
I also commented some other problems for later fixes:
Now still to find the pkt_too_big to self and copied < acked problems
Jason reported. Alexey, do you have a theory for the pkt_too_big to
self?
-Andi
--- linux/net/ipv4/af_inet.c-o Fri Mar 12 22:14:56 1999
+++ linux/net/ipv4/af_inet.c Sat Mar 13 11:11:17 1999
@@ -53,6 +53,7 @@
* David S. Miller : New socket lookup architecture.
* Some other random speedups.
* Cyrus Durgin : Cleaned up file for kmod hacks.
+ * Andi Kleen : Fix inet_stream_connect TCP race.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -613,14 +614,17 @@
}
if(sock->state == SS_CONNECTING) {
+ /* Note: tcp_connected contains SYN_RECV, which may cause
+ bogus results here. -AK */
if(tcp_connected(sk->state)) {
sock->state = SS_CONNECTED;
return 0;
}
- if(sk->protocol == IPPROTO_TCP && (flags & O_NONBLOCK)) {
- if(sk->err)
- return sock_error(sk);
- return -EALREADY;
+ if(sk->protocol == IPPROTO_TCP) {
+ if (sk->zapped || sk->err)
+ goto sock_error;
+ if (flags & O_NONBLOCK)
+ return -EALREADY;
}
} else {
/* We may need to bind the socket. */
@@ -629,15 +633,17 @@
if (sk->prot->connect == NULL)
return(-EOPNOTSUPP);
err = sk->prot->connect(sk, uaddr, addr_len);
+ /* Note: there is a theoretical race here when an wake up
+ occurred before inet_wait_for_connect is entered. In 2.3
+ the wait queue setup should be moved before the low level
+ connect call. -AK*/
if (err < 0)
return(err);
sock->state = SS_CONNECTING;
}
- if (sk->state > TCP_FIN_WAIT2 && sock->state == SS_CONNECTING) {
- sock->state = SS_UNCONNECTED;
- return sock_error(sk);
- }
+ if (sk->state > TCP_FIN_WAIT2 && sock->state == SS_CONNECTING)
+ goto sock_error;
if (sk->state != TCP_ESTABLISHED && (flags & O_NONBLOCK))
return (-EINPROGRESS);
@@ -649,17 +655,19 @@
}
sock->state = SS_CONNECTED;
- if ((sk->state != TCP_ESTABLISHED) && sk->err) {
- /* This is ugly but needed to fix a race in the ICMP error handler */
- if (sk->protocol == IPPROTO_TCP && sk->zapped) {
- lock_sock(sk);
- tcp_set_state(sk, TCP_CLOSE);
- release_sock(sk);
- }
- sock->state = SS_UNCONNECTED;
- return sock_error(sk);
- }
+ if ((sk->state != TCP_ESTABLISHED) && sk->err)
+ goto sock_error;
return(0);
+
+sock_error:
+ /* This is ugly but needed to fix a race in the ICMP error handler */
+ if (sk->protocol == IPPROTO_TCP && sk->zapped) {
+ lock_sock(sk);
+ tcp_set_state(sk, TCP_CLOSE);
+ release_sock(sk);
+ }
+ sock->state = SS_UNCONNECTED;
+ return sock_error(sk);
}
/*
-- This is like TV. I don't like TV.- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/