tcp: Fix a connect() race with timewait sockets

When we find a timewait connection in __inet_hash_connect() and reuse
it for a new connection request, we have a race window, releasing bind
list lock and reacquiring it in __inet_twsk_kill() to remove timewait
socket from list.

Another thread might find the timewait socket we already chose, leading to
list corruption and crashes.

Fix is to remove timewait socket from bind list before releasing the bind lock.

Note: This problem happens if sysctl_tcp_tw_reuse is set.

Reported-by: kapil dakhane <kdakhane@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
index b801ade2295ef8af242b97a07393b63c35ab8195..79f67eae8a7e212f30e5b89e09ede8b4d21a245b 100644 (file)
--- a/include/net/inet_timewait_sock.h
+++ b/include/net/inet_timewait_sock.h
@@ -201,6 +201,9 @@ extern void inet_twsk_put(struct inet_timewait_sock *tw);
 
 extern int inet_twsk_unhash(struct inet_timewait_sock *tw);
 
+extern int inet_twsk_bind_unhash(struct inet_timewait_sock *tw,
+                                struct inet_hashinfo *hashinfo);
+
 extern struct inet_timewait_sock *inet_twsk_alloc(const struct sock *sk,
                                                  const int state);
 

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index c4201b7ece383089e2778402ffe135c285382db4..2b79377b468dd0b06d69b0c6b5842f52ba52db07 100644 (file)
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -502,6 +502,8 @@ ok:
                        inet_sk(sk)->inet_sport = htons(port);
                        twrefcnt += hash(sk, tw);
                }
+               if (tw)
+                       twrefcnt += inet_twsk_bind_unhash(tw, hinfo);
                spin_unlock(&head->lock);
 
                if (tw) {

diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index 0fdf45e4c90c8c8475cfd28feb9830d65ebfc204..bf4b1e2a4305e49d45b1fbaa32a8210febfff0ba 100644 (file)
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -29,12 +29,29 @@ int inet_twsk_unhash(struct inet_timewait_sock *tw)
        return 1;
 }
 
+/*
+ * unhash a timewait socket from bind hash
+ * lock must be hold by caller
+ */
+int inet_twsk_bind_unhash(struct inet_timewait_sock *tw,
+                         struct inet_hashinfo *hashinfo)
+{
+       struct inet_bind_bucket *tb = tw->tw_tb;
+
+       if (!tb)
+               return 0;
+
+       __hlist_del(&tw->tw_bind_node);
+       tw->tw_tb = NULL;
+       inet_bind_bucket_destroy(hashinfo->bind_bucket_cachep, tb);
+       return 1;
+}
+
 /* Must be called with locally disabled BHs. */
 static void __inet_twsk_kill(struct inet_timewait_sock *tw,
                             struct inet_hashinfo *hashinfo)
 {
        struct inet_bind_hashbucket *bhead;
-       struct inet_bind_bucket *tb;
        int refcnt;
        /* Unlink from established hashes. */
        spinlock_t *lock = inet_ehash_lockp(hashinfo, tw->tw_hash);
@@ -46,15 +63,11 @@ static void __inet_twsk_kill(struct inet_timewait_sock *tw,
        /* Disassociate with bind bucket. */
        bhead = &hashinfo->bhash[inet_bhashfn(twsk_net(tw), tw->tw_num,
                        hashinfo->bhash_size)];
+
        spin_lock(&bhead->lock);
-       tb = tw->tw_tb;
-       if (tb) {
-               __hlist_del(&tw->tw_bind_node);
-               tw->tw_tb = NULL;
-               inet_bind_bucket_destroy(hashinfo->bind_bucket_cachep, tb);
-               refcnt++;
-       }
+       refcnt += inet_twsk_bind_unhash(tw, hashinfo);
        spin_unlock(&bhead->lock);
+
 #ifdef SOCK_REFCNT_DEBUG
        if (atomic_read(&tw->tw_refcnt) != 1) {
                printk(KERN_DEBUG "%s timewait_sock %p refcnt=%d\n",