Re: [PATCH 1/2 5.10.y] Bluetooth: SCO: Fix UAF on sco_sock_timeout

On Tue, Aug 4, 2026 at 6:13 PM Sasha Levin <[email protected]> wrote:
>
> On Tue, Aug 04, 2026 at 09:44:05PM +0000, Todd Kjos wrote:
> > @@ -192,7 +200,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> >
> >       /* Kill socket */
> >       sco_conn_lock(conn);
> > -     sk = conn->sk;
> > +     sk = sco_sock_hold(conn);
> >       sco_conn_unlock(conn);
> >
> >       if (sk) {
>
> I think that this hunk leaks a struct sock reference on 5.10.

Yes, you are right.

>
> Unlike upstream and the 5.15/6.1 backports, where the sock_hold(sk) sits
> just before sco_conn_unlock() and the upstream hunk deletes it, 5.10 keeps
> its sock_hold(sk) inside the condition:
>
>         sco_conn_lock(conn);
>         sk = conn->sk;
>         sco_conn_unlock(conn);
>
>         if (sk) {
>                 sock_hold(sk);

This reference is not needed in the new version, since in we take the
reference above.

>                 bh_lock_sock(sk);
>                 sco_sock_clear_timer(sk);
>                 sco_chan_del(sk, err);
>                 bh_unlock_sock(sk);
>                 sock_put(sk);
>         }
>
> So after this patch the function takes two references (one in
> sco_sock_hold(), one from the surviving sock_hold(sk)) and drops only one.
> That is a permanent struct sock leak on every SCO teardown, reachable by
> any user with repeated connect/disconnect.
>
> While respinning, please also drop the Change-Id: trailers from both
> patches.

Will do. Sorry about that.

>
> --
> Thanks,
> Sasha