* [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