mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: RFCOMM: serialize security confirmation handling
@ 2026-08-22 16:43 Chengfeng Ye
  2026-08-24 16:50 ` patchwork-bot+bluetooth
  0 siblings, 1 reply; 4+ messages in thread
From: Chengfeng Ye @ 2026-08-22 16:43 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz, Kees Cook,
	Jakub Kicinski, Chengfeng Ye, Ali Ahmet Memis, Tim Bird,
	SeungJu Cheon, Gustavo Padovan, Dean Jenkins
  Cc: linux-bluetooth, linux-kernel, stable

rfcomm_security_cfm() looks up a session on session_list and then walks
its DLC list without holding rfcomm_mutex. Since RFCOMM session teardown
uses rfcomm_mutex, krfcommd can close and free the same session and DLCs
concurrently:

  hci_rx_work                    krfcommd
  -----------                    ---------
  rfcomm_session_get()
                                 rfcomm_lock()
                                 rfcomm_session_close()
                                   rfcomm_dlc_unlink()
                                   rfcomm_session_del()
                                     kfree(s)
                                 rfcomm_unlock()
  walk s->dlcs

The callback can then read a freed session list head and touch freed DLCs
while updating their flags or timers.

Serialize the session lookup and DLC traversal in rfcomm_security_cfm()
with rfcomm_mutex. This matches the existing RFCOMM session lifetime
rules and prevents concurrent rfcomm_session_del() / rfcomm_dlc_unlink()
from tearing the objects down while the callback is using them.

KASAN reported:

  BUG: KASAN: slab-use-after-free in rfcomm_security_cfm+0x41c/0x440
  Read of size 8 at addr ffff888111fb3960 by task kworker/u17:1/89
  Workqueue: hci0 hci_rx_work
  Call Trace:
   rfcomm_security_cfm+0x41c/0x440
   hci_encrypt_cfm+0x139/0x590
   hci_encrypt_change_evt+0x37b/0xc40
   hci_event_packet+0x71b/0xb20
   hci_rx_work+0x293/0x730
  Allocated by task 69:
   rfcomm_session_add+0x9e/0x2f0
   rfcomm_run+0x44b/0x41e0
  Freed by task 69:
   kfree+0x131/0x3c0
   rfcomm_session_del+0x188/0x220
   rfcomm_run+0x1985/0x41e0

Fixes: 08c30aca9e698faddebd34f81e1196295f9dc063 ("Bluetooth: Remove RFCOMM session refcnt")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/bluetooth/rfcomm/core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 9cdfea666a2c..5d150e8623d5 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2213,9 +2213,13 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt)
 
 	BT_DBG("conn %p status 0x%02x encrypt 0x%02x", conn, status, encrypt);
 
+	rfcomm_lock();
+
 	s = rfcomm_session_get(&conn->hdev->bdaddr, &conn->dst);
-	if (!s)
+	if (!s) {
+		rfcomm_unlock();
 		return;
+	}
 
 	list_for_each_entry_safe(d, n, &s->dlcs, list) {
 		if (test_and_clear_bit(RFCOMM_SEC_PENDING, &d->flags)) {
@@ -2247,6 +2251,8 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt)
 			set_bit(RFCOMM_AUTH_REJECT, &d->flags);
 	}
 
+	rfcomm_unlock();
+
 	rfcomm_schedule();
 }
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: RFCOMM: serialize security confirmation handling
  2026-08-22 16:43 [PATCH] Bluetooth: RFCOMM: serialize security confirmation handling Chengfeng Ye
@ 2026-08-24 16:50 ` patchwork-bot+bluetooth
  2026-08-24 19:38   ` Pauli Virtanen
  0 siblings, 1 reply; 4+ messages in thread
From: patchwork-bot+bluetooth @ 2026-08-24 16:50 UTC (permalink / raw)
  To: Chengfeng Ye
  Cc: marcel, luiz.dentz, kees, kuba, ali, tim.bird, suunj1331,
	gustavo.padovan, Dean_Jenkins, linux-bluetooth, linux-kernel,
	stable

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Sun, 23 Aug 2026 00:43:41 +0800 you wrote:
> rfcomm_security_cfm() looks up a session on session_list and then walks
> its DLC list without holding rfcomm_mutex. Since RFCOMM session teardown
> uses rfcomm_mutex, krfcommd can close and free the same session and DLCs
> concurrently:
> 
>   hci_rx_work                    krfcommd
> 
> [...]

Here is the summary with links:
  - Bluetooth: RFCOMM: serialize security confirmation handling
    https://git.kernel.org/bluetooth/bluetooth-next/c/40c621391de9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: RFCOMM: serialize security confirmation handling
  2026-08-24 16:50 ` patchwork-bot+bluetooth
@ 2026-08-24 19:38   ` Pauli Virtanen
  2026-08-24 20:24     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 4+ messages in thread
From: Pauli Virtanen @ 2026-08-24 19:38 UTC (permalink / raw)
  To: patchwork-bot+bluetooth, Chengfeng Ye
  Cc: marcel, luiz.dentz, kuba, linux-bluetooth, linux-kernel, stable

Hi,

ma, 2026-08-24 kello 16:50 +0000, patchwork-bot+bluetooth@kernel.org
kirjoitti:
> Hello:
> 
> This patch was applied to bluetooth/bluetooth-next.git (master)
> by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
> 
> On Sun, 23 Aug 2026 00:43:41 +0800 you wrote:
> > rfcomm_security_cfm() looks up a session on session_list and then walks
> > its DLC list without holding rfcomm_mutex. Since RFCOMM session teardown
> > uses rfcomm_mutex, krfcommd can close and free the same session and DLCs
> > concurrently:
> > 
> >   hci_rx_work                    krfcommd
> > 
> > [...]
> 
> Here is the summary with links:
>   - Bluetooth: RFCOMM: serialize security confirmation handling
>     https://git.kernel.org/bluetooth/bluetooth-next/c/40c621391de9
> 
> You are awesome, thank you!

I'm a bit too late now, but Sashiko report had some valid concerns
about lock ordering in this one:
https://sashiko.dev/#/patchset/20260822164341.3750491-1-nicoyip.dev%40gmail.com

Running both BlueZ l2cap-tester and rfcomm-tester (so lockdep sees more
paths) in same boot you get lockdep splat.

Not immediately clear that a real deadlock could be triggered, but
lockdep thinks the ordering is now inconsistent.


======================================================
WARNING: possible circular locking dependency detected
7.2.0-rc6-01463-gfe3897b4ab57 #994 Not tainted
------------------------------------------------------
rfcomm-tester/364 is trying to acquire lock:
ffff8880027d40b0 (&hdev->lock){+.+.}-{4:4}, at: l2cap_chan_connect (net/bluetooth/l2cap_core.c:7271)

but task is already holding lock:
ffffffff99499f58 (rfcomm_mutex){+.+.}-{4:4}, at: rfcomm_dlc_open (net/bluetooth/rfcomm/core.c:426)

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #2 (rfcomm_mutex){+.+.}-{4:4}:
__mutex_lock (kernel/locking/mutex.c:646)
rfcomm_security_cfm (net/bluetooth/rfcomm/core.c:2222)
hci_auth_complete_evt (./include/net/bluetooth/hci_core.h:2246 net/bluetooth/hci_event.c:3554)
hci_event_packet (net/bluetooth/hci_event.c:7887)
hci_rx_work (net/bluetooth/hci_core.c:4041)
process_scheduled_works (kernel/workqueue.c:3322)
worker_thread (kernel/workqueue.c:3486)
kthread (kernel/kthread.c:436)
ret_from_fork (arch/x86/kernel/process.c:158)
ret_from_fork_asm (arch/x86/entry/entry_64.S:245)

-> #1 (hci_cb_list_lock){+.+.}-{4:4}:
__mutex_lock (kernel/locking/mutex.c:646)
hci_remote_ext_features_evt (./include/net/bluetooth/hci_core.h:2207 net/bluetooth/hci_event.c:5030)
hci_event_packet (net/bluetooth/hci_event.c:7887)
hci_rx_work (net/bluetooth/hci_core.c:4041)
process_scheduled_works (kernel/workqueue.c:3322)
worker_thread (kernel/workqueue.c:3486)
kthread (kernel/kthread.c:436)
ret_from_fork (arch/x86/kernel/process.c:158)
ret_from_fork_asm (arch/x86/entry/entry_64.S:245)

-> #0 (&hdev->lock){+.+.}-{4:4}:
__lock_acquire (kernel/locking/lockdep.c:3165)
lock_acquire (kernel/locking/lockdep.c:5868)
__mutex_lock (kernel/locking/mutex.c:646)
l2cap_chan_connect (net/bluetooth/l2cap_core.c:7271)
l2cap_sock_connect (net/bluetooth/l2cap_sock.c:255)
kernel_connect (net/socket.c:3726)
rfcomm_dlc_open (net/bluetooth/rfcomm/core.c:808)
rfcomm_sock_connect (net/bluetooth/rfcomm/sock.c:421)
__sys_connect (net/socket.c:2135)
__x64_sys_connect (net/socket.c:2160 net/socket.c:2157 net/socket.c:2157)
do_syscall_64 (arch/x86/entry/syscall_64.c:63)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)

other info that might help us debug this:

Chain exists of:
&hdev->lock --> hci_cb_list_lock --> rfcomm_mutex

Possible unsafe locking scenario:

CPU0                    CPU1
----                    ----
lock(rfcomm_mutex);
                        lock(hci_cb_list_lock);
                        lock(rfcomm_mutex);
lock(&hdev->lock);

*** DEADLOCK ***

1 lock held by rfcomm-tester/364:
#0: ffffffff99499f58 (rfcomm_mutex){+.+.}-{4:4}, at: rfcomm_dlc_open (net/bluetooth/rfcomm/core.c:426)

stack backtrace:
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-10.fc44 06/10/2025
Call Trace:
<TASK>
print_circular_bug (kernel/locking/lockdep.c:2043)
check_noncircular (kernel/locking/lockdep.c:2175)
__lock_acquire (kernel/locking/lockdep.c:3165)
? stack_trace_save (kernel/stacktrace.c:122)
? add_lock_to_list (./include/linux/find.h:391 kernel/locking/lockdep.c:1403 kernel/locking/lockdep.c:1434)
? __lock_acquire (kernel/locking/lockdep.c:191 kernel/locking/lockdep.c:3912 kernel/locking/lockdep.c:5237)
lock_acquire (kernel/locking/lockdep.c:5868)
? l2cap_chan_connect (net/bluetooth/l2cap_core.c:7271)
__mutex_lock (kernel/locking/mutex.c:646)
? l2cap_chan_connect (net/bluetooth/l2cap_core.c:7271)
? lock_acquire (kernel/locking/lockdep.c:5868)
? hci_get_route (net/bluetooth/hci_conn.c:1331)
? lock_release (kernel/locking/lockdep.c:5350)
? hci_get_route (net/bluetooth/hci_conn.c:1379)
? l2cap_chan_connect (net/bluetooth/l2cap_core.c:7271)
l2cap_chan_connect (net/bluetooth/l2cap_core.c:7271)
? l2cap_sock_connect (net/bluetooth/l2cap_sock.c:193)
l2cap_sock_connect (net/bluetooth/l2cap_sock.c:255)
kernel_connect (net/socket.c:3726)
rfcomm_dlc_open (net/bluetooth/rfcomm/core.c:808)
rfcomm_sock_connect (net/bluetooth/rfcomm/sock.c:421)
? __pfx_rfcomm_sock_connect (net/bluetooth/rfcomm/sock.c:?)
__sys_connect (net/socket.c:2135)
__x64_sys_connect (net/socket.c:2160 net/socket.c:2157 net/socket.c:2157)
do_syscall_64 (arch/x86/entry/syscall_64.c:63)
? entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
? trace_hardirqs_off (./include/trace/events/preemptirq.h:36)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
RIP: 0033:0x7f2b9c05854e
Code: 4d 89 d8 e8 b4 bd 00 00 4c 8b 5d f8 41 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 11 c9 c3 0f 1f 80 00 00 00 00 48 8b 45 10 0f 05 <c9> c3 83 e2 39 83 fa 08 75 e7 e8 03 ff ff ff 0f 1f 00 f3 0f 1e fa
All code
========
   0:	4d 89 d8             	mov    %r11,%r8
   3:	e8 b4 bd 00 00       	call   0xbdbc
   8:	4c 8b 5d f8          	mov    -0x8(%rbp),%r11
   c:	41 8b 93 08 03 00 00 	mov    0x308(%r11),%edx
  13:	59                   	pop    %rcx
  14:	5e                   	pop    %rsi
  15:	48 83 f8 fc          	cmp    $0xfffffffffffffffc,%rax
  19:	74 11                	je     0x2c
  1b:	c9                   	leave
  1c:	c3                   	ret
  1d:	0f 1f 80 00 00 00 00 	nopl   0x0(%rax)
  24:	48 8b 45 10          	mov    0x10(%rbp),%rax
  28:	0f 05                	syscall
  2a:*	c9                   	leave		<-- trapping instruction
  2b:	c3                   	ret
  2c:	83 e2 39             	and    $0x39,%edx
  2f:	83 fa 08             	cmp    $0x8,%edx
  32:	75 e7                	jne    0x1b
  34:	e8 03 ff ff ff       	call   0xffffffffffffff3c
  39:	0f 1f 00             	nopl   (%rax)
  3c:	f3 0f 1e fa          	endbr64

Code starting with the faulting instruction
===========================================
   0:	c9                   	leave
   1:	c3                   	ret
   2:	83 e2 39             	and    $0x39,%edx
   5:	83 fa 08             	cmp    $0x8,%edx
   8:	75 e7                	jne    0xfffffffffffffff1
   a:	e8 03 ff ff ff       	call   0xffffffffffffff12
   f:	0f 1f 00             	nopl   (%rax)
  12:	f3 0f 1e fa          	endbr64
RSP: 002b:00007fff41018e50 EFLAGS: 00000202 ORIG_RAX: 000000000000002a
RAX: ffffffffffffffda RBX: 00000000004fbee0 RCX: 00007f2b9c05854e
RDX: 000000000000000a RSI: 00007b2b99ad7e60 RDI: 000000000000000d
RBP: 00007fff41018e60 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000202 R12: 00007b2b99ad7e40
R13: 00007b2b99ad7e60 R14: 000000000000000d R15: 00007b8b9ade0140
</TASK>


-- 
Pauli Virtanen

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: RFCOMM: serialize security confirmation handling
  2026-08-24 19:38   ` Pauli Virtanen
@ 2026-08-24 20:24     ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2026-08-24 20:24 UTC (permalink / raw)
  To: Pauli Virtanen
  Cc: patchwork-bot+bluetooth, Chengfeng Ye, marcel, kuba,
	linux-bluetooth, linux-kernel, stable

Hi Pauli,

On Mon, Aug 24, 2026 at 3:38 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi,
>
> ma, 2026-08-24 kello 16:50 +0000, patchwork-bot+bluetooth@kernel.org
> kirjoitti:
> > Hello:
> >
> > This patch was applied to bluetooth/bluetooth-next.git (master)
> > by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
> >
> > On Sun, 23 Aug 2026 00:43:41 +0800 you wrote:
> > > rfcomm_security_cfm() looks up a session on session_list and then walks
> > > its DLC list without holding rfcomm_mutex. Since RFCOMM session teardown
> > > uses rfcomm_mutex, krfcommd can close and free the same session and DLCs
> > > concurrently:
> > >
> > >   hci_rx_work                    krfcommd
> > >
> > > [...]
> >
> > Here is the summary with links:
> >   - Bluetooth: RFCOMM: serialize security confirmation handling
> >     https://git.kernel.org/bluetooth/bluetooth-next/c/40c621391de9
> >
> > You are awesome, thank you!
>
> I'm a bit too late now, but Sashiko report had some valid concerns
> about lock ordering in this one:
> https://sashiko.dev/#/patchset/20260822164341.3750491-1-nicoyip.dev%40gmail.com
>
> Running both BlueZ l2cap-tester and rfcomm-tester (so lockdep sees more
> paths) in same boot you get lockdep splat.

I didn't know about this trick, so I wonder if we should do something
about it. Testing with just rfcomm-tester should show this, or perhaps
we need to change the CI to run all tests under the same test-runner
instance so lockdep learns about all the code paths.

> Not immediately clear that a real deadlock could be triggered, but
> lockdep thinks the ordering is now inconsistent.
>
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 7.2.0-rc6-01463-gfe3897b4ab57 #994 Not tainted
> ------------------------------------------------------
> rfcomm-tester/364 is trying to acquire lock:
> ffff8880027d40b0 (&hdev->lock){+.+.}-{4:4}, at: l2cap_chan_connect (net/bluetooth/l2cap_core.c:7271)
>
> but task is already holding lock:
> ffffffff99499f58 (rfcomm_mutex){+.+.}-{4:4}, at: rfcomm_dlc_open (net/bluetooth/rfcomm/core.c:426)
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #2 (rfcomm_mutex){+.+.}-{4:4}:
> __mutex_lock (kernel/locking/mutex.c:646)
> rfcomm_security_cfm (net/bluetooth/rfcomm/core.c:2222)
> hci_auth_complete_evt (./include/net/bluetooth/hci_core.h:2246 net/bluetooth/hci_event.c:3554)
> hci_event_packet (net/bluetooth/hci_event.c:7887)
> hci_rx_work (net/bluetooth/hci_core.c:4041)
> process_scheduled_works (kernel/workqueue.c:3322)
> worker_thread (kernel/workqueue.c:3486)
> kthread (kernel/kthread.c:436)
> ret_from_fork (arch/x86/kernel/process.c:158)
> ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
>
> -> #1 (hci_cb_list_lock){+.+.}-{4:4}:
> __mutex_lock (kernel/locking/mutex.c:646)
> hci_remote_ext_features_evt (./include/net/bluetooth/hci_core.h:2207 net/bluetooth/hci_event.c:5030)
> hci_event_packet (net/bluetooth/hci_event.c:7887)
> hci_rx_work (net/bluetooth/hci_core.c:4041)
> process_scheduled_works (kernel/workqueue.c:3322)
> worker_thread (kernel/workqueue.c:3486)
> kthread (kernel/kthread.c:436)
> ret_from_fork (arch/x86/kernel/process.c:158)
> ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
>
> -> #0 (&hdev->lock){+.+.}-{4:4}:
> __lock_acquire (kernel/locking/lockdep.c:3165)
> lock_acquire (kernel/locking/lockdep.c:5868)
> __mutex_lock (kernel/locking/mutex.c:646)
> l2cap_chan_connect (net/bluetooth/l2cap_core.c:7271)
> l2cap_sock_connect (net/bluetooth/l2cap_sock.c:255)
> kernel_connect (net/socket.c:3726)

Interesting, I though the kernel_connect would actually behave as a
syscall and use it own thread so it wouldn't result into
l2cap_chan_connect, in that case it is probably useless to use it in
place of l2cap_chan_connect directly, so we may take the opportunity
and clean this up.

> rfcomm_dlc_open (net/bluetooth/rfcomm/core.c:808)
> rfcomm_sock_connect (net/bluetooth/rfcomm/sock.c:421)
> __sys_connect (net/socket.c:2135)
> __x64_sys_connect (net/socket.c:2160 net/socket.c:2157 net/socket.c:2157)
> do_syscall_64 (arch/x86/entry/syscall_64.c:63)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
>
> other info that might help us debug this:
>
> Chain exists of:
> &hdev->lock --> hci_cb_list_lock --> rfcomm_mutex
>
> Possible unsafe locking scenario:
>
> CPU0                    CPU1
> ----                    ----
> lock(rfcomm_mutex);
>                         lock(hci_cb_list_lock);
>                         lock(rfcomm_mutex);
> lock(&hdev->lock);
>
> *** DEADLOCK ***
>
> 1 lock held by rfcomm-tester/364:
> #0: ffffffff99499f58 (rfcomm_mutex){+.+.}-{4:4}, at: rfcomm_dlc_open (net/bluetooth/rfcomm/core.c:426)

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-24 20:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-22 16:43 [PATCH] Bluetooth: RFCOMM: serialize security confirmation handling Chengfeng Ye
2026-08-24 16:50 ` patchwork-bot+bluetooth
2026-08-24 19:38   ` Pauli Virtanen
2026-08-24 20:24     ` Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®