aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDave Young <hidave.darkstar@gmail.com>2008-06-02 02:50:52 -0400
committerDavid S. Miller <davem@davemloft.net>2008-06-03 17:27:17 -0400
commit537d59af73d894750cff14f90fe2b6d77fbab15b (patch)
treebff5d6efbffbf6685cac5698efe7e3192e391047
parentc3b25b32e8bef526cca748e1ba023c6bdd705a99 (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>
-rw-r--r--net/bluetooth/rfcomm/tty.c13
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);