Re: [PATCH] Bluetooth: RFCOMM: serialize security confirmation handling

[Luiz Augusto von Dentz]

Hi Pauli,

On Mon, Aug 24, 2026 at 3:38 PM Pauli Virtanen <pav@xxxxxx> wrote:
>
> Hi,
>
> ma, 2026-08-24 kello 16:50 +0000, patchwork-bot+bluetooth@xxxxxxxxxx
> kirjoitti:
> > Hello:
> >
> > This patch was applied to bluetooth/bluetooth-next.git (master)
> > by Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>:
> >
> > 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)