kernel.git/net/bluetooth/rfcomm/tty.c, branch linux-2.6.25.y

bluetooth: rfcomm_dev_state_change deadlock fix

2008-06-16T20:19:54Z

commit 537d59af73d894750cff14f90fe2b6d77fbab15b in mainline 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 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 Acked-by: Marcel Holtmann Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman

bluetooth : __rfcomm_dlc_close lock fix

2008-04-02T06:59:06Z

Lockdep warning will be trigged while rfcomm connection closing. The locks taken in rfcomm_dev_add: rfcomm_dev_lock --> d->lock In __rfcomm_dlc_close: d->lock --> rfcomm_dev_lock (in rfcomm_dev_state_change) There's two way to fix it, one is in rfcomm_dev_add we first locking d->lock then the rfcomm_dev_lock The other (in this patch), remove the locking of d->lock for rfcomm_dev_state_change because just locking "d->state = BT_CLOSED;" is enough. [ 295.002046] ======================================================= [ 295.002046] [ INFO: possible circular locking dependency detected ] [ 295.002046] 2.6.25-rc7 #1 [ 295.002046] ------------------------------------------------------- [ 295.002046] krfcommd/2705 is trying to acquire lock: [ 295.002046] (rfcomm_dev_lock){-.--}, at: [] rfcomm_dev_state_change+0x6a/0xd0 [rfcomm] [ 295.002046] [ 295.002046] but task is already holding lock: [ 295.002046] (&d->lock){--..}, at: [] __rfcomm_dlc_close+0x43/0xd0 [rfcomm] [ 295.002046] [ 295.002046] which lock already depends on the new lock. [ 295.002046] [ 295.002046] [ 295.002046] the existing dependency chain (in reverse order) is: [ 295.002046] [ 295.002046] -> #1 (&d->lock){--..}: [ 295.002046] [] check_prev_add+0xd3/0x200 [ 295.002046] [] check_prevs_add+0x95/0xe0 [ 295.002046] [] validate_chain+0x23f/0x320 [ 295.002046] [] __lock_acquire+0x1c1/0x760 [ 295.002046] [] lock_acquire+0x79/0xb0 [ 295.002046] [] _spin_lock+0x39/0x80 [ 295.002046] [] rfcomm_dev_add+0x240/0x360 [rfcomm] [ 295.002046] [] rfcomm_create_dev+0x6e/0xe0 [rfcomm] [ 295.002046] [] rfcomm_dev_ioctl+0x33/0x60 [rfcomm] [ 295.002046] [] rfcomm_sock_ioctl+0x2c/0x50 [rfcomm] [ 295.002046] [] sock_ioctl+0x118/0x240 [ 295.002046] [] vfs_ioctl+0x76/0x90 [ 295.002046] [] do_vfs_ioctl+0x56/0x140 [ 295.002046] [] sys_ioctl+0x39/0x60 [ 295.002046] [] syscall_call+0x7/0xb [ 295.002046] [] 0xffffffff [ 295.002046] [ 295.002046] -> #0 (rfcomm_dev_lock){-.--}: [ 295.002046] [] check_prev_add+0x34/0x200 [ 295.002046] [] check_prevs_add+0x95/0xe0 [ 295.002046] [] validate_chain+0x23f/0x320 [ 295.002046] [] __lock_acquire+0x1c1/0x760 [ 295.002046] [] lock_acquire+0x79/0xb0 [ 295.002046] [] _read_lock+0x39/0x80 [ 295.002046] [] rfcomm_dev_state_change+0x6a/0xd0 [rfcomm] [ 295.002046] [] __rfcomm_dlc_close+0x58/0xd0 [rfcomm] [ 295.002046] [] rfcomm_recv_ua+0x6f/0x120 [rfcomm] [ 295.002046] [] rfcomm_recv_frame+0x171/0x1e0 [rfcomm] [ 295.002046] [] rfcomm_run+0xe7/0x550 [rfcomm] [ 295.002046] [] kthread+0x5c/0xa0 [ 295.002046] [] kernel_thread_helper+0x7/0x10 [ 295.002046] [] 0xffffffff [ 295.002046] [ 295.002046] other info that might help us debug this: [ 295.002046] [ 295.002046] 2 locks held by krfcommd/2705: [ 295.002046] #0: (rfcomm_mutex){--..}, at: [] rfcomm_run+0x7b/0x550 [rfcomm] [ 295.002046] #1: (&d->lock){--..}, at: [] __rfcomm_dlc_close+0x43/0xd0 [rfcomm] [ 295.002046] [ 295.002046] stack backtrace: [ 295.002046] Pid: 2705, comm: krfcommd Not tainted 2.6.25-rc7 #1 [ 295.002046] [] ? printk+0x18/0x20 [ 295.002046] [] print_circular_bug_tail+0x6f/0x80 [ 295.002046] [] check_prev_add+0x34/0x200 [ 295.002046] [] check_prevs_add+0x95/0xe0 [ 295.002046] [] validate_chain+0x23f/0x320 [ 295.002046] [] __lock_acquire+0x1c1/0x760 [ 295.002046] [] lock_acquire+0x79/0xb0 [ 295.002046] [] ? rfcomm_dev_state_change+0x6a/0xd0 [rfcomm] [ 295.002046] [] _read_lock+0x39/0x80 [ 295.002046] [] ? rfcomm_dev_state_change+0x6a/0xd0 [rfcomm] [ 295.002046] [] rfcomm_dev_state_change+0x6a/0xd0 [rfcomm] [ 295.002046] [] __rfcomm_dlc_close+0x58/0xd0 [rfcomm] [ 295.002046] [] rfcomm_recv_ua+0x6f/0x120 [rfcomm] [ 295.002046] [] rfcomm_recv_frame+0x171/0x1e0 [rfcomm] [ 295.002046] [] ? trace_hardirqs_on+0xb9/0x130 [ 295.002046] [] ? _spin_unlock_irqrestore+0x39/0x70 [ 295.002046] [] rfcomm_run+0xe7/0x550 [rfcomm] [ 295.002046] [] ? __sched_text_start+0x229/0x4c0 [ 295.002046] [] ? cpu_avg_load_per_task+0x20/0x30 [ 295.002046] [] ? rfcomm_run+0x0/0x550 [rfcomm] [ 295.002046] [] kthread+0x5c/0xa0 [ 295.002046] [] ? kthread+0x0/0xa0 [ 295.002046] [] kernel_thread_helper+0x7/0x10 [ 295.002046] ======================= Signed-off-by: Dave Young Signed-off-by: David S. Miller

bluetooth rfcomm tty: destroy before tty_close()

2008-02-05T11:12:06Z

rfcomm dev could be deleted in tty_hangup, so we must not call rfcomm_dev_del again to prevent from destroying rfcomm dev before tty close. Signed-off-by: Dave Young Signed-off-by: Andrew Morton Signed-off-by: David S. Miller

[BLUETOOTH]: Move children of connection device to NULL before connection down.

2008-01-23T11:11:39Z

The rfcomm tty device will possibly retain even when conn is down, and sysfs doesn't support zombie device moving, so this patch move the tty device before conn device is destroyed. For the bug refered please see : http://lkml.org/lkml/2007/12/28/87 Signed-off-by: Dave Young Signed-off-by: David S. Miller

[BLUETOOTH]: rfcomm tty BUG_ON() code fix

2008-01-11T06:22:52Z

1) In tty.c the BUG_ON at line 115 will never be called, because the the before list_del_init in this same function. 115 BUG_ON(!list_empty(&dev->list)); So move the list_del_init to rfcomm_dev_del 2) The rfcomm_dev_del could be called from diffrent path (rfcomm_tty_hangup/rfcomm_dev_state_change/rfcomm_release_dev), So add another BUG_ON when the rfcomm_dev_del is called more than one time. Signed-off-by: Dave Young Signed-off-by: David S. Miller

[Bluetooth] Add address and channel attribute to RFCOMM TTY device

2007-10-22T09:59:47Z

Export the remote device address and channel of RFCOMM TTY device via sysfs attributes. This allows udev to create better naming rules for configured RFCOMM devices. Signed-off-by: Marcel Holtmann

[IRDA]: Fix rfcomm use-after-free

2007-07-31T09:28:05Z

Adrian Bunk wrote: > Commit 8de0a15483b357d0f0b821330ec84d1660cadc4e added the following > use-after-free in net/bluetooth/rfcomm/tty.c: > > <-- snip --> > > ... > static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc) > { > ... > if (IS_ERR(dev->tty_dev)) { > list_del(&dev->list); > kfree(dev); > return PTR_ERR(dev->tty_dev); > } > ... > > <-- snip --> > > Spotted by the Coverity checker. really good catch. I fully overlooked that one. The attached patch should fix it. Signed-off-by: Marcel Holtmann Signed-off-by: David S. Miller

[Bluetooth] Keep rfcomm_dev on the list until it is freed

2007-07-11T05:06:51Z

This patch changes the RFCOMM TTY release process so that the TTY is kept on the list until it is really freed. A new device flag is used to keep track of released TTYs. Signed-off-by: Ville Tervo Signed-off-by: Marcel Holtmann

[Bluetooth] Hangup TTY before releasing rfcomm_dev

2007-07-11T05:01:26Z

The core problem is that RFCOMM socket layer ioctl can release rfcomm_dev struct while RFCOMM TTY layer is still actively using it. Calling tty_vhangup() is needed for a synchronous hangup before rfcomm_dev is freed. Addresses the oops at http://bugzilla.kernel.org/show_bug.cgi?id=7509 Acked-by: Alan Cox Signed-off-by: Marcel Holtmann

[Bluetooth] Check that device is in rfcomm_dev_list before deleting

2007-05-04T22:36:10Z

If RFCOMM_RELEASE_ONHUP flag is on and rfcomm_release_dev is called before connection is closed, rfcomm_dev is deleted twice from the rfcomm_dev_list and refcount is messed up. This patch adds a check before deleting device that the device actually is listed. Signed-off-by: Ville Tervo Signed-off-by: Marcel Holtmann