| From 5391a87751a164b3194864126f3b016038abc9fe Mon Sep 17 00:00:00 2001 |
| From: Tuong Lien <tuong.t.lien@dektech.com.au> |
| Date: Mon, 10 Feb 2020 15:35:44 +0700 |
| Subject: tipc: fix successful connect() but timed out |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Tuong Lien <tuong.t.lien@dektech.com.au> |
| |
| commit 5391a87751a164b3194864126f3b016038abc9fe upstream. |
| |
| In commit 9546a0b7ce00 ("tipc: fix wrong connect() return code"), we |
| fixed the issue with the 'connect()' that returns zero even though the |
| connecting has failed by waiting for the connection to be 'ESTABLISHED' |
| really. However, the approach has one drawback in conjunction with our |
| 'lightweight' connection setup mechanism that the following scenario |
| can happen: |
| |
| (server) (client) |
| |
| +- accept()| | wait_for_conn() |
| | | |connect() -------+ |
| | |<-------[SYN]---------| > sleeping |
| | | *CONNECTING | |
| |--------->*ESTABLISHED | | |
| |--------[ACK]-------->*ESTABLISHED > wakeup() |
| send()|--------[DATA]------->|\ > wakeup() |
| send()|--------[DATA]------->| | > wakeup() |
| . . . . |-> recvq . |
| . . . . | . |
| send()|--------[DATA]------->|/ > wakeup() |
| close()|--------[FIN]-------->*DISCONNECTING | |
| *DISCONNECTING | | |
| | ~~~~~~~~~~~~~~~~~~> schedule() |
| | wait again |
| . |
| . |
| | ETIMEDOUT |
| |
| Upon the receipt of the server 'ACK', the client becomes 'ESTABLISHED' |
| and the 'wait_for_conn()' process is woken up but not run. Meanwhile, |
| the server starts to send a number of data following by a 'close()' |
| shortly without waiting any response from the client, which then forces |
| the client socket to be 'DISCONNECTING' immediately. When the wait |
| process is switched to be running, it continues to wait until the timer |
| expires because of the unexpected socket state. The client 'connect()' |
| will finally get ‘-ETIMEDOUT’ and force to release the socket whereas |
| there remains the messages in its receive queue. |
| |
| Obviously the issue would not happen if the server had some delay prior |
| to its 'close()' (or the number of 'DATA' messages is large enough), |
| but any kind of delay would make the connection setup/shutdown "heavy". |
| We solve this by simply allowing the 'connect()' returns zero in this |
| particular case. The socket is already 'DISCONNECTING', so any further |
| write will get '-EPIPE' but the socket is still able to read the |
| messages existing in its receive queue. |
| |
| Note: This solution doesn't break the previous one as it deals with a |
| different situation that the socket state is 'DISCONNECTING' but has no |
| error (i.e. sk->sk_err = 0). |
| |
| Fixes: 9546a0b7ce00 ("tipc: fix wrong connect() return code") |
| Acked-by: Ying Xue <ying.xue@windriver.com> |
| Acked-by: Jon Maloy <jon.maloy@ericsson.com> |
| Signed-off-by: Tuong Lien <tuong.t.lien@dektech.com.au> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| net/tipc/socket.c | 2 ++ |
| 1 file changed, 2 insertions(+) |
| |
| --- a/net/tipc/socket.c |
| +++ b/net/tipc/socket.c |
| @@ -2441,6 +2441,8 @@ static int tipc_wait_for_connect(struct |
| return -ETIMEDOUT; |
| if (signal_pending(current)) |
| return sock_intr_errno(*timeo_p); |
| + if (sk->sk_state == TIPC_DISCONNECTING) |
| + break; |
| |
| add_wait_queue(sk_sleep(sk), &wait); |
| done = sk_wait_event(sk, timeo_p, tipc_sk_connected(sk), |