kernel.git/net/bluetooth/rfcomm, 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: fix locking bug in the rfcomm socket cleanup handling

2008-06-16T20:19:53Z

[ Upstream commit: 7dccf1f4e1696c79bff064c3770867cc53cbc71c ] in net/bluetooth/rfcomm/sock.c, rfcomm_sk_state_change() does the following operation: if (parent && sock_flag(sk, SOCK_ZAPPED)) { /* We have to drop DLC lock here, otherwise * rfcomm_sock_destruct() will dead lock. */ rfcomm_dlc_unlock(d); rfcomm_sock_kill(sk); rfcomm_dlc_lock(d); } } which is fine, since rfcomm_sock_kill() will call sk_free() which will call rfcomm_sock_destruct() which takes the rfcomm_dlc_lock()... so far so good. HOWEVER, this assumes that the rfcomm_sk_state_change() function always gets called with the rfcomm_dlc_lock() taken. This is the case for all but one case, and in that case where we don't have the lock, we do a double unlock followed by an attempt to take the lock, which due to underflow isn't going anywhere fast. This patch fixes this by moving the stragling case inside the lock, like the other usages of the same call are doing in this code. This was found with the help of the www.kerneloops.org project, where this deadlock was observed 51 times at this point in time: http://www.kerneloops.org/search.php?search=rfcomm_sock_destruct Signed-off-by: Arjan van de Ven Acked-by: Marcel Holtmann Signed-off-by: David S. Miller Signed-off-by: Chris Wright

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: replace deprecated RW_LOCK_UNLOCKED macros

2008-03-28T23:17:38Z

The older RW_LOCK_UNLOCKED macros defeat lockdep state tracing so replace them with the newer __RW_LOCK_UNLOCKED macros. Signed-off-by: Robert P. J. Day Acked-by: Marcel Holtmann Signed-off-by: Andrew Morton Signed-off-by: David S. Miller

remove final fastcall users

2008-02-14T00:21:18Z

fastcall always expands to empty, remove it. Signed-off-by: Harvey Harrison Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds

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

[NET]: Convert init_timer into setup_timer

2008-01-28T22:53:35Z

Many-many code in the kernel initialized the timer->function and timer->data together with calling init_timer(timer). There is already a helper for this. Use it for networking code. The patch is HUGE, but makes the code 130 lines shorter (98 insertions(+), 228 deletions(-)). Signed-off-by: Pavel Emelyanov Acked-by: Arnaldo Carvalho de Melo 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

[NET]: Forget the zero_it argument of sk_alloc()

2007-11-01T07:39:31Z

Finally, the zero_it argument can be completely removed from the callers and from the function prototype. Besides, fix the checkpatch.pl warnings about using the assignments inside if-s. This patch is rather big, and it is a part of the previous one. I splitted it wishing to make the patches more readable. Hope this particular split helped. Signed-off-by: Pavel Emelyanov Signed-off-by: David S. Miller