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