Skip to content

Commit

Permalink
bluetooth: rfcomm_dev_state_change deadlock fix
Browse files Browse the repository at this point in the history
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
1905f6c ("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 4c8411f ("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>
  • Loading branch information
Dave Young authored and David S. Miller committed Jun 3, 2008
1 parent c3b25b3 commit 537d59a
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion net/bluetooth/rfcomm/tty.c
Original file line number Diff line number Diff line change
Expand Up @@ -566,11 +566,22 @@ static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err)
if (dlc->state == BT_CLOSED) {
if (!dev->tty) {
if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) {
if (rfcomm_dev_get(dev->id) == NULL)
/* Drop DLC lock here to avoid deadlock
* 1. rfcomm_dev_get will take rfcomm_dev_lock
* but in rfcomm_dev_add there's lock order:
* rfcomm_dev_lock -> dlc lock
* 2. rfcomm_dev_put will deadlock if it's
* the last reference
*/
rfcomm_dlc_unlock(dlc);
if (rfcomm_dev_get(dev->id) == NULL) {
rfcomm_dlc_lock(dlc);
return;
}

rfcomm_dev_del(dev);
rfcomm_dev_put(dev);
rfcomm_dlc_lock(dlc);
}
} else
tty_hangup(dev->tty);
Expand Down

0 comments on commit 537d59a

Please sign in to comment.