Bluetooth: Fix RFCOMM session reference counting issue
authorOctavian Purdila <octavian.purdila@intel.com>
Fri, 27 Jan 2012 17:32:39 +0000 (19:32 +0200)
committerJohan Hedberg <johan.hedberg@intel.com>
Wed, 15 Feb 2012 11:09:26 +0000 (13:09 +0200)
There is an imbalance in the rfcomm_session_hold / rfcomm_session_put
operations which causes the following crash:

[  685.010159] BUG: unable to handle kernel paging request at 6b6b6b6b
[  685.010169] IP: [<c149d76d>] rfcomm_process_dlcs+0x1b/0x15e
[  685.010181] *pdpt = 000000002d665001 *pde = 0000000000000000
[  685.010191] Oops: 0000 [#1] PREEMPT SMP
[  685.010247]
[  685.010255] Pid: 947, comm: krfcommd Tainted: G         C  3.0.16-mid8-dirty #44
[  685.010266] EIP: 0060:[<c149d76d>] EFLAGS: 00010246 CPU: 1
[  685.010274] EIP is at rfcomm_process_dlcs+0x1b/0x15e
[  685.010281] EAX: e79f551c EBX: 6b6b6b6b ECX: 00000007 EDX: e79f40b4
[  685.010288] ESI: e79f4060 EDI: ed4e1f70 EBP: ed4e1f68 ESP: ed4e1f50
[  685.010295]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[  685.010303] Process krfcommd (pid: 947, ti=ed4e0000 task=ed43e5e0 task.ti=ed4e0000)
[  685.010308] Stack:
[  685.010312]  ed4e1f68 c149eb53 e5925150 e79f4060 ed500000 ed4e1f70 ed4e1f80 c149ec10
[  685.010331]  00000000 ed43e5e0 00000000 ed4e1f90 ed4e1f9c c149ec87 0000bf54 00000000
[  685.010348]  00000000 ee03bf54 c149ec37 ed4e1fe4 c104fe01 00000000 00000000 00000000
[  685.010367] Call Trace:
[  685.010376]  [<c149eb53>] ? rfcomm_process_rx+0x6e/0x74
[  685.010387]  [<c149ec10>] rfcomm_process_sessions+0xb7/0xde
[  685.010398]  [<c149ec87>] rfcomm_run+0x50/0x6d
[  685.010409]  [<c149ec37>] ? rfcomm_process_sessions+0xde/0xde
[  685.010419]  [<c104fe01>] kthread+0x63/0x68
[  685.010431]  [<c104fd9e>] ? __init_kthread_worker+0x42/0x42
[  685.010442]  [<c14dae82>] kernel_thread_helper+0x6/0xd

This issue has been brought up earlier here:

https://lkml.org/lkml/2011/5/21/127

The issue appears to be the rfcomm_session_put in rfcomm_recv_ua. This
operation doesn't seem be to required as for the non-initiator case we
have the rfcomm_process_rx doing an explicit put and in the initiator
case the last dlc_unlink will drive the reference counter to 0.

There have been several attempts to fix these issue:

6c2718d Bluetooth: Do not call rfcomm_session_put() for RFCOMM UA on closed socket
683d949 Bluetooth: Never deallocate a session when some DLC points to it

but AFAICS they do not fix the issue just make it harder to reproduce.

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
Signed-off-by: Gopala Krishna Murala <gopala.krishna.murala@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
net/bluetooth/rfcomm/core.c

index 501649bf5596d0373d6b6dd3be791dd1345ad33e..8a602388f1e73c185aa24499e9ff636c9358fcee 100644 (file)
@@ -1164,12 +1164,18 @@ static int rfcomm_recv_ua(struct rfcomm_session *s, u8 dlci)
                        break;
 
                case BT_DISCONN:
-                       /* When socket is closed and we are not RFCOMM
-                        * initiator rfcomm_process_rx already calls
-                        * rfcomm_session_put() */
-                       if (s->sock->sk->sk_state != BT_CLOSED)
-                               if (list_empty(&s->dlcs))
-                                       rfcomm_session_put(s);
+                       /* rfcomm_session_put is called later so don't do
+                        * anything here otherwise we will mess up the session
+                        * reference counter:
+                        *
+                        * (a) when we are the initiator dlc_unlink will drive
+                        * the reference counter to 0 (there is no initial put
+                        * after session_add)
+                        *
+                        * (b) when we are not the initiator rfcomm_rx_process
+                        * will explicitly call put to balance the initial hold
+                        * done after session add.
+                        */
                        break;
                }
        }