Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: fix UAF read of ->accept_q in bt_accept_poll()
@ 2026-05-04 15:10 Jann Horn
  2026-05-04 15:51 ` bluez.test.bot
  2026-05-05 15:06 ` [PATCH] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 4+ messages in thread
From: Jann Horn @ 2026-05-04 15:10 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz, linux-bluetooth
  Cc: linux-kernel, stable, Jann Horn

Use lock_sock() to guard against bt_accept_poll() racing with concurrent
close(accept()), which can lead to UAF:

task 1           task 2
======           ======
                 __x64_sys_poll
                   __se_sys_poll
                     __do_sys_poll
                       do_sys_poll
                         do_poll
                           do_pollfd
                             vfs_poll
                               sock_poll
                                 bt_sock_poll
                                   bt_accept_poll
                                     [read ->accept_q next pointer]
__x64_sys_accept
  __se_sys_accept
    __do_sys_accept
      __sys_accept4
        __sys_accept4_file
          do_accept
            l2cap_sock_accept
              bt_accept_dequeue
                bt_accept_unlink
                  [removes new socket from ->accept_q]
__x64_sys_close
  __se_sys_close
    __do_sys_close
      fput_close_sync
        __fput
          sock_close
            __sock_release
              l2cap_sock_release
                l2cap_sock_kill
                  sock_put
                    sk_free
                      __sk_free
                        sk_destruct
                          __sk_destruct
                            [frees new socket]
                                     [UAF read of ->sk_state]

This UAF only leads to incorrect reads, it does not corrupt memory; it is a
fairly tight race window; I believe every race attempt requires an
incoming bluetooth connection; and the leaked data is limited.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
 net/bluetooth/af_bluetooth.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 33d053d63407..d24897167838 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -521,13 +521,17 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
 	struct bt_sock *s, *n;
 	struct sock *sk;
 
+	lock_sock(parent);
 	list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
 		sk = (struct sock *)s;
 		if (sk->sk_state == BT_CONNECTED ||
 		    (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags) &&
-		     sk->sk_state == BT_CONNECT2))
+		     sk->sk_state == BT_CONNECT2)) {
+			release_sock(parent);
 			return EPOLLIN | EPOLLRDNORM;
+		}
 	}
+	release_sock(parent);
 
 	return 0;
 }

---
base-commit: 6d35786de28116ecf78797a62b84e6bf3c45aa5a
change-id: 20260504-bluetooth-accept-uaf-fix-df393cbda114

--  
Jann Horn <jannh@google.com>


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

* RE: Bluetooth: fix UAF read of ->accept_q in bt_accept_poll()
  2026-05-04 15:10 [PATCH] Bluetooth: fix UAF read of ->accept_q in bt_accept_poll() Jann Horn
@ 2026-05-04 15:51 ` bluez.test.bot
  2026-05-05 15:06 ` [PATCH] " Luiz Augusto von Dentz
  1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2026-05-04 15:51 UTC (permalink / raw)
  To: linux-bluetooth, jannh

[-- Attachment #1: Type: text/plain, Size: 3639 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1089360

---Test result---

Test Summary:
CheckPatch                    PASS      0.63 seconds
GitLint                       FAIL      0.21 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      26.94 seconds
CheckAllWarning               PASS      29.43 seconds
CheckSparse                   PASS      28.45 seconds
BuildKernel32                 PASS      26.62 seconds
TestRunnerSetup               PASS      554.33 seconds
TestRunner_l2cap-tester       FAIL      6.36 seconds
TestRunner_iso-tester         PASS      294.03 seconds
TestRunner_bnep-tester        FAIL      6.60 seconds
TestRunner_mgmt-tester        FAIL      8.86 seconds
TestRunner_rfcomm-tester      PASS      28.81 seconds
TestRunner_sco-tester         PASS      67.96 seconds
TestRunner_ioctl-tester       FAIL      37.29 seconds
TestRunner_mesh-tester        PASS      27.03 seconds
TestRunner_smp-tester         PASS      6.53 seconds
TestRunner_userchan-tester    FAIL      6.70 seconds
TestRunner_6lowpan-tester     PASS      22.87 seconds
IncrementalBuild              PASS      24.55 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: fix UAF read of ->accept_q in bt_accept_poll()

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
61: B2 Line has trailing whitespace: "--  "
##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_bnep-tester - FAIL
Desc: Run bnep-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_ioctl-tester - FAIL
Desc: Run ioctl-tester with test-runner
Output:
Total: 28, Passed: 0 (0.0%), Failed: 11, Not Run: 17

Failed Test Cases
Device List                                          Timed out  -30.834 seconds
Device Info                                          Timed out   -6.182 seconds
Reset Stat                                           Timed out   -6.185 seconds
Set Link Mode - ACCEPT                               Timed out   -6.188 seconds
Set Pkt Type - DM                                    Timed out  -14.375 seconds
Set Pkt Type - DH                                    Timed out  -14.378 seconds
Set Pkt Type - HV                                    Timed out  -14.380 seconds
Set Pkt Type - 2-DH                                  Timed out  -14.383 seconds
Set Pkt Type - 2-DH                                  Timed out  -14.386 seconds
Set Pkt Type - ALL                                   Timed out  -14.388 seconds
Set ACL MTU - 1                                      Timed out  -14.391 seconds
##############################
Test: TestRunner_userchan-tester - FAIL
Desc: Run userchan-tester with test-runner
Output:
No test result found


https://github.com/bluez/bluetooth-next/pull/143

---
Regards,
Linux Bluetooth


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

* Re: [PATCH] Bluetooth: fix UAF read of ->accept_q in bt_accept_poll()
  2026-05-04 15:10 [PATCH] Bluetooth: fix UAF read of ->accept_q in bt_accept_poll() Jann Horn
  2026-05-04 15:51 ` bluez.test.bot
@ 2026-05-05 15:06 ` Luiz Augusto von Dentz
  2026-05-06 12:04   ` Jann Horn
  1 sibling, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2026-05-05 15:06 UTC (permalink / raw)
  To: Jann Horn; +Cc: Marcel Holtmann, linux-bluetooth, linux-kernel, stable

Hi Jann,

On Mon, May 4, 2026 at 11:11 AM Jann Horn <jannh@google.com> wrote:
>
> Use lock_sock() to guard against bt_accept_poll() racing with concurrent
> close(accept()), which can lead to UAF:
>
> task 1           task 2
> ======           ======
>                  __x64_sys_poll
>                    __se_sys_poll
>                      __do_sys_poll
>                        do_sys_poll
>                          do_poll
>                            do_pollfd
>                              vfs_poll
>                                sock_poll
>                                  bt_sock_poll
>                                    bt_accept_poll
>                                      [read ->accept_q next pointer]
> __x64_sys_accept
>   __se_sys_accept
>     __do_sys_accept
>       __sys_accept4
>         __sys_accept4_file
>           do_accept
>             l2cap_sock_accept
>               bt_accept_dequeue
>                 bt_accept_unlink
>                   [removes new socket from ->accept_q]
> __x64_sys_close
>   __se_sys_close
>     __do_sys_close
>       fput_close_sync
>         __fput
>           sock_close
>             __sock_release
>               l2cap_sock_release
>                 l2cap_sock_kill
>                   sock_put
>                     sk_free
>                       __sk_free
>                         sk_destruct
>                           __sk_destruct
>                             [frees new socket]
>                                      [UAF read of ->sk_state]
>
> This UAF only leads to incorrect reads, it does not corrupt memory; it is a
> fairly tight race window; I believe every race attempt requires an
> incoming bluetooth connection; and the leaked data is limited.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
>  net/bluetooth/af_bluetooth.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index 33d053d63407..d24897167838 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -521,13 +521,17 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
>         struct bt_sock *s, *n;
>         struct sock *sk;
>
> +       lock_sock(parent);
>         list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
>                 sk = (struct sock *)s;
>                 if (sk->sk_state == BT_CONNECTED ||
>                     (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags) &&
> -                    sk->sk_state == BT_CONNECT2))
> +                    sk->sk_state == BT_CONNECT2)) {
> +                       release_sock(parent);
>                         return EPOLLIN | EPOLLRDNORM;
> +               }
>         }
> +       release_sock(parent);

There is the following comments though:

https://sashiko.dev/#/patchset/20260504-bluetooth-accept-uaf-fix-v1-1-1ca63c0efadd%40google.com

I'm not really sure if likes for the poll are supposed to be done
lockless, if they are, we cannot use lock_sock here and will likely
need to rework accept_q so it doesn't contain deferred sks, as those
shouldn't be considered ready for acceptance.

>         return 0;
>  }
>
> ---
> base-commit: 6d35786de28116ecf78797a62b84e6bf3c45aa5a
> change-id: 20260504-bluetooth-accept-uaf-fix-df393cbda114
>
> --
> Jann Horn <jannh@google.com>
>


-- 
Luiz Augusto von Dentz

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

* Re: [PATCH] Bluetooth: fix UAF read of ->accept_q in bt_accept_poll()
  2026-05-05 15:06 ` [PATCH] " Luiz Augusto von Dentz
@ 2026-05-06 12:04   ` Jann Horn
  0 siblings, 0 replies; 4+ messages in thread
From: Jann Horn @ 2026-05-06 12:04 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, linux-bluetooth, linux-kernel, stable

On Tue, May 5, 2026 at 5:06 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> On Mon, May 4, 2026 at 11:11 AM Jann Horn <jannh@google.com> wrote:
> >
> > Use lock_sock() to guard against bt_accept_poll() racing with concurrent
> > close(accept()), which can lead to UAF:
> >
> > task 1           task 2
> > ======           ======
> >                  __x64_sys_poll
> >                    __se_sys_poll
> >                      __do_sys_poll
> >                        do_sys_poll
> >                          do_poll
> >                            do_pollfd
> >                              vfs_poll
> >                                sock_poll
> >                                  bt_sock_poll
> >                                    bt_accept_poll
> >                                      [read ->accept_q next pointer]
> > __x64_sys_accept
> >   __se_sys_accept
> >     __do_sys_accept
> >       __sys_accept4
> >         __sys_accept4_file
> >           do_accept
> >             l2cap_sock_accept
> >               bt_accept_dequeue
> >                 bt_accept_unlink
> >                   [removes new socket from ->accept_q]
> > __x64_sys_close
> >   __se_sys_close
> >     __do_sys_close
> >       fput_close_sync
> >         __fput
> >           sock_close
> >             __sock_release
> >               l2cap_sock_release
> >                 l2cap_sock_kill
> >                   sock_put
> >                     sk_free
> >                       __sk_free
> >                         sk_destruct
> >                           __sk_destruct
> >                             [frees new socket]
> >                                      [UAF read of ->sk_state]
> >
> > This UAF only leads to incorrect reads, it does not corrupt memory; it is a
> > fairly tight race window; I believe every race attempt requires an
> > incoming bluetooth connection; and the leaked data is limited.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jann Horn <jannh@google.com>
> > ---
> >  net/bluetooth/af_bluetooth.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> > index 33d053d63407..d24897167838 100644
> > --- a/net/bluetooth/af_bluetooth.c
> > +++ b/net/bluetooth/af_bluetooth.c
> > @@ -521,13 +521,17 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
> >         struct bt_sock *s, *n;
> >         struct sock *sk;
> >
> > +       lock_sock(parent);
> >         list_for_each_entry_safe(s, n, &bt_sk(parent)->accept_q, accept_q) {
> >                 sk = (struct sock *)s;
> >                 if (sk->sk_state == BT_CONNECTED ||
> >                     (test_bit(BT_SK_DEFER_SETUP, &bt_sk(parent)->flags) &&
> > -                    sk->sk_state == BT_CONNECT2))
> > +                    sk->sk_state == BT_CONNECT2)) {
> > +                       release_sock(parent);
> >                         return EPOLLIN | EPOLLRDNORM;
> > +               }
> >         }
> > +       release_sock(parent);
>
> There is the following comments though:
>
> https://sashiko.dev/#/patchset/20260504-bluetooth-accept-uaf-fix-v1-1-1ca63c0efadd%40google.com

Regarding the LLM output on whether lock_sock(parent) is enough: The
locking I'm adding here is the same as what bt_accept_dequeue() uses
for protection; if event handling can also remove accept_q elements
without holding appropriate locks, I think that is a separate (and
bigger) bug.

I see I've just been CC'ed on
<https://lore.kernel.org/all/20260506114338.2873496-1-n05ec@lzu.edu.cn/>,
which seems to be a broader fix; if you want to go with that patch,
this one is superfluous.

> I'm not really sure if likes for the poll are supposed to be done
> lockless, if they are, we cannot use lock_sock here and will likely
> need to rework accept_q so it doesn't contain deferred sks, as those
> shouldn't be considered ready for acceptance.

I don't see why that would be a problem;
Documentation/filesystems/vfs.rst says nothing about wanting lockless
operation, and if you look around at other poll handlers, you'll see
several ->poll() handlers that take sleeping locks:

 - dma_buf_poll() calls dma_resv_lock(), which locks a W/W mutex
 - vb2_fop_poll() sometimes calls mutex_lock_interruptible()
 - virtio_rpmsg_poll() calls mutex_lock()

My understanding is that is is preferable, but not required, for
->poll() handlers to be fast if they're used by high-performance
userspace code, since event loops might hit ->poll() handlers fairly
often (especially if userspace uses an API like poll() or select(); I
think with epoll you only get one or two ->poll() callbacks once the
file descriptor actually becomes ready); I think this probably isn't
really an issue for bluetooth listening sockets.

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

end of thread, other threads:[~2026-05-06 12:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-04 15:10 [PATCH] Bluetooth: fix UAF read of ->accept_q in bt_accept_poll() Jann Horn
2026-05-04 15:51 ` bluez.test.bot
2026-05-05 15:06 ` [PATCH] " Luiz Augusto von Dentz
2026-05-06 12:04   ` Jann Horn

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