linux-kernel - Re: KASAN: use-after-free Read in __io_uring_files

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <422ab1c5-b347-66a2-66e1-a3fb6b28f6e7@gmail.com>
Date:   Fri, 9 Oct 2020 15:48:57 +0300
From:   Pavel Begunkov <asml.silence@...il.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     syzbot <syzbot+77efce558b2b9e6b6405@...kaller.appspotmail.com>,
        axboe@...nel.dk, io-uring@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        syzkaller-bugs@...glegroups.com, viro@...iv.linux.org.uk
Subject: Re: KASAN: use-after-free Read in __io_uring_files_cancel

On 09/10/2020 15:35, Matthew Wilcox wrote:
> On Fri, Oct 09, 2020 at 03:28:54PM +0300, Pavel Begunkov wrote:
>> On 09/10/2020 15:12, Matthew Wilcox wrote:
>>>> It seems this fails on "node->shift" in xas_next_entry(), that would
>>>> mean that the node itself was freed while we're iterating on it.
>>>>
>>>> __io_uring_files_cancel() iterates with xas_next_entry() and creates
>>>> XA_STATE once by hand, but it also removes entries in the loop with
>>>> io_uring_del_task_file() -> xas_store(&xas, NULL); without updating
>>>> the iterating XA_STATE. Could it be the problem? See a diff below
>>>
>>> No, the problem is that the lock is dropped after calling
>>> xas_next_entry(), and at any point after calling xas_next_entry(),
>>> the node that it's pointing to can be freed.
>>
>> Only the task itself clears/removes entries, others can only insert.
>> So, could it be freed even though there are no parallel erases?
> 
> Not with today's implementation, but that's something that might
> change in the future.  I agree it's probably the task itself that's
> deleting the entry and causing the node to be deleted.

I see, then it looks like I narrowed it down right. But your
approach is cleaner anyway.

-- 
Pavel Begunkov