* [PATCH] Bluetooth: fix UAF read of ->accept_q in bt_accept_poll()
@ 2026-05-04 15:10 Jann Horn
2026-05-05 15:06 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 3+ 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] 3+ 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-05 15:06 ` Luiz Augusto von Dentz
2026-05-06 12:04 ` Jann Horn
0 siblings, 1 reply; 3+ 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] 3+ messages in thread
* Re: [PATCH] Bluetooth: fix UAF read of ->accept_q in bt_accept_poll()
2026-05-05 15:06 ` Luiz Augusto von Dentz
@ 2026-05-06 12:04 ` Jann Horn
0 siblings, 0 replies; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2026-05-06 12:05 UTC | newest]
Thread overview: 3+ 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-05 15:06 ` 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