Skip to content

Commit

Permalink
tcp: Fix a connect() race with timewait sockets
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Eric Dumazet authored and David S. Miller committed Dec 9, 2009
1 parent 9327f70 commit 3cdaeda
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
3 changes: 3 additions & 0 deletions include/net/inet_timewait_sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 2 additions & 0 deletions net/ipv4/inet_hashtables.c
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,8 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
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) {
Expand Down
29 changes: 21 additions & 8 deletions net/ipv4/inet_timewait_sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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",
Expand Down

0 comments on commit 3cdaeda

Please sign in to comment.