Re: [PATCH] Bluetooth: fix UAF read of ->accept_q in bt_accept_poll()

[Luiz Augusto von Dentz]

Hi Jann,

On Mon, May 4, 2026 at 11:11 AM Jann Horn <jannh@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Jann Horn <jannh@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>
>


-- 
Luiz Augusto von Dentz