diff options
author | Dave Young <hidave.darkstar@gmail.com> | 2008-06-02 02:50:52 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2008-06-03 17:27:17 -0400 |
commit | 537d59af73d894750cff14f90fe2b6d77fbab15b (patch) | |
tree | bff5d6efbffbf6685cac5698efe7e3192e391047 /net/bluetooth | |
parent | c3b25b32e8bef526cca748e1ba023c6bdd705a99 (diff) |
bluetooth: rfcomm_dev_state_change deadlock fix
There's logic in __rfcomm_dlc_close:
rfcomm_dlc_lock(d);
d->state = BT_CLOSED;
d->state_changed(d, err);
rfcomm_dlc_unlock(d);
In rfcomm_dev_state_change, it's possible that rfcomm_dev_put try to
take the dlc lock, then we will deadlock.
Here fixed it by unlock dlc before rfcomm_dev_get in
rfcomm_dev_state_change.
why not unlock just before rfcomm_dev_put? it's because there's
another problem. rfcomm_dev_get/rfcomm_dev_del will take
rfcomm_dev_lock, but in rfcomm_dev_add the lock order is :
rfcomm_dev_lock --> dlc lock
so I unlock dlc before the taken of rfcomm_dev_lock.
Actually it's a regression caused by commit
1905f6c736cb618e07eca0c96e60e3c024023428 ("bluetooth :
__rfcomm_dlc_close lock fix"), the dlc state_change could be two
callbacks : rfcomm_sk_state_change and rfcomm_dev_state_change. I
missed the rfcomm_sk_state_change that time.
Thanks Arjan van de Ven <arjan@linux.intel.com> for the effort in
commit 4c8411f8c115def968820a4df6658ccfd55d7f1a ("bluetooth: fix
locking bug in the rfcomm socket cleanup handling") but he missed the
rfcomm_dev_state_change lock issue.
Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/bluetooth')
-rw-r--r-- | net/bluetooth/rfcomm/tty.c | 13 |
1 files changed, 12 insertions, 1 deletions
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c index c3f749abb2d0..c9191871c1e0 100644 --- a/net/bluetooth/rfcomm/tty.c +++ b/net/bluetooth/rfcomm/tty.c | |||
@@ -566,11 +566,22 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err) | |||
566 | if (dlc->state == BT_CLOSED) { | 566 | if (dlc->state == BT_CLOSED) { |
567 | if (!dev->tty) { | 567 | if (!dev->tty) { |
568 | if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) { | 568 | if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) { |
569 | if (rfcomm_dev_get(dev->id) == NULL) | 569 | /* Drop DLC lock here to avoid deadlock |
570 | * 1. rfcomm_dev_get will take rfcomm_dev_lock | ||
571 | * but in rfcomm_dev_add there's lock order: | ||
572 | * rfcomm_dev_lock -> dlc lock | ||
573 | * 2. rfcomm_dev_put will deadlock if it's | ||
574 | * the last reference | ||
575 | */ | ||
576 | rfcomm_dlc_unlock(dlc); | ||
577 | if (rfcomm_dev_get(dev->id) == NULL) { | ||
578 | rfcomm_dlc_lock(dlc); | ||
570 | return; | 579 | return; |
580 | } | ||
571 | 581 | ||
572 | rfcomm_dev_del(dev); | 582 | rfcomm_dev_del(dev); |
573 | rfcomm_dev_put(dev); | 583 | rfcomm_dev_put(dev); |
584 | rfcomm_dlc_lock(dlc); | ||
574 | } | 585 | } |
575 | } else | 586 | } else |
576 | tty_hangup(dev->tty); | 587 | tty_hangup(dev->tty); |