Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [syzbot] KASAN: use-after-free Write in sco_sock_timeout
To:     Hillf Danton <hdanton@sina.com>
Cc:     syzbot <syzbot+2bef95d3ab4daa10155b@syzkaller.appspotmail.com>,
        linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
        luiz.von.dentz@intel.com, syzkaller-bugs@googlegroups.com
References: <000000000000b2725705ca78de29@google.com>
 <20210827011931.2323-1-hdanton@sina.com>
 <20210829082929.2897-1-hdanton@sina.com>
 <11a30bc1-574f-aa1a-21d6-b886de9fc981@gmail.com>
 <20210830001538.1980-1-hdanton@sina.com>
From:   Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
Message-ID: <33d6d22f-7666-bc08-f626-3821cac9f174@gmail.com>
Date:   Mon, 30 Aug 2021 12:41:48 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <20210830001538.1980-1-hdanton@sina.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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);
>>   
>>