Commit 537d59af73d894750cff14f90fe2b6d77fbab15b
Committed by
David S. Miller
1 parent
c3b25b32e8
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>
Showing 1 changed file with 12 additions and 1 deletions Side-by-side Diff
net/bluetooth/rfcomm/tty.c
... | ... | @@ -566,11 +566,22 @@ |
566 | 566 | if (dlc->state == BT_CLOSED) { |
567 | 567 | if (!dev->tty) { |
568 | 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 | 579 | return; |
580 | + } | |
571 | 581 | |
572 | 582 | rfcomm_dev_del(dev); |
573 | 583 | rfcomm_dev_put(dev); |
584 | + rfcomm_dlc_lock(dlc); | |
574 | 585 | } |
575 | 586 | } else |
576 | 587 | tty_hangup(dev->tty); |