aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHerbert Xu <herbert@gondor.apana.org.au>2006-05-05 17:09:13 -0700
committerDavid S. Miller <davem@davemloft.net>2006-05-05 17:09:13 -0700
commit134af34632a7b3b0a98a79a2e56bf9cc927e0eac (patch)
treeb54012edae78a294723fba01d684cd41b8cd6e97
parent1c29fc4989bc2a3838b2837adc12b8aeb0feeede (diff)
[DCCP]: Fix sock_orphan dead lock
Calling sock_orphan inside bh_lock_sock in dccp_close can lead to dead locks. For example, the inet_diag code holds sk_callback_lock without disabling BH. If an inbound packet arrives during that admittedly tiny window, it will cause a dead lock on bh_lock_sock. Another possible path would be through sock_wfree if the network device driver frees the tx skb in process context with BH enabled. We can fix this by moving sock_orphan out of bh_lock_sock. The tricky bit is to work out when we need to destroy the socket ourselves and when it has already been destroyed by someone else. By moving sock_orphan before the release_sock we can solve this problem. This is because as long as we own the socket lock its state cannot change. So we simply record the socket state before the release_sock and then check the state again after we regain the socket lock. If the socket state has transitioned to DCCP_CLOSED in the time being, we know that the socket has been destroyed. Otherwise the socket is still ours to keep. This problem was discoverd by Ingo Molnar using his lock validator. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--net/dccp/proto.c13
1 files changed, 10 insertions, 3 deletions
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index 1ff7328b0e1..2e0ee8355c4 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -848,6 +848,7 @@ static int dccp_close_state(struct sock *sk)
void dccp_close(struct sock *sk, long timeout)
{
struct sk_buff *skb;
+ int state;
lock_sock(sk);
@@ -882,6 +883,11 @@ void dccp_close(struct sock *sk, long timeout)
sk_stream_wait_close(sk, timeout);
adjudge_to_death:
+ state = sk->sk_state;
+ sock_hold(sk);
+ sock_orphan(sk);
+ atomic_inc(sk->sk_prot->orphan_count);
+
/*
* It is the last release_sock in its life. It will remove backlog.
*/
@@ -894,8 +900,9 @@ adjudge_to_death:
bh_lock_sock(sk);
BUG_TRAP(!sock_owned_by_user(sk));
- sock_hold(sk);
- sock_orphan(sk);
+ /* Have we already been destroyed by a softirq or backlog? */
+ if (state != DCCP_CLOSED && sk->sk_state == DCCP_CLOSED)
+ goto out;
/*
* The last release_sock may have processed the CLOSE or RESET
@@ -915,12 +922,12 @@ adjudge_to_death:
#endif
}
- atomic_inc(sk->sk_prot->orphan_count);
if (sk->sk_state == DCCP_CLOSED)
inet_csk_destroy_sock(sk);
/* Otherwise, socket is reprieved until protocol close. */
+out:
bh_unlock_sock(sk);
local_bh_enable();
sock_put(sk);