Re: [syzbot] KASAN: use-after-free Write in sco_sock_timeout

[Desmond Cheong Zhi Xi]

On 30/8/21 8:15 am, Hillf Danton wrote:
> On Mon, 30 Aug 2021 02:34:11 +0800 Desmond Cheong Zhi Xi wrote:
> > Ok I went back to make a more thorough audit. Even without calling
> > cancel_delayed_work_sync, sco_sock_timeout should not cause a UAF.
> >
> > I believe the real issue is that we can allocate a connection twice in
> > sco_connect. This means that the first connection gets lost and we're
> > unable to clean it up properly.
> >
> > Thoughts on this?
>
> LGTM regardless of the uaf reported.
>
> Hillf

Thanks for taking a look. It passed the Syzbot reproducer too, so I 
think the root cause should have been addressed.

I'll write up a patch to summarize what we found out. May I include you 
as a Co-developed-by: author?

> > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git e3f30ab28ac8
> >
> > --- a/net/bluetooth/sco.c
> > +++ b/net/bluetooth/sco.c
> > @@ -578,9 +578,6 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
> >   	    addr->sa_family != AF_BLUETOOTH)
> >   		return -EINVAL;
> >   
> > -	if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND)
> > -		return -EBADFD;
> > -
> >   	if (sk->sk_type != SOCK_SEQPACKET)
> >   		return -EINVAL;
> >   
> > @@ -591,6 +588,13 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
> >   
> >   	lock_sock(sk);
> >   
> > +	if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND) {
> > +		hci_dev_unlock(hdev);
> > +		hci_dev_put(hdev);
> > +		err = -EBADFD;
> > +		goto done;
> > +	}
> > +
> >   	/* Set destination address and psm */
> >   	bacpy(&sco_pi(sk)->dst, &sa->sco_bdaddr);
> >