Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3d271554-9ddc-07ad-3ff8-30aba31f8bf2@kernel.dk>
Date:   Tue, 17 May 2022 06:47:17 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [REPORT] Use-after-free Read in __fdget_raw in v5.10.y
Content-Language: en-US
To:     Lee Jones <lee.jones@linaro.org>
Cc:     Pavel Begunkov <asml.silence@gmail.com>, io-uring@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <YoOJ/T4QRKC+fAZE@google.com>
 <97cba3e1-4ef7-0a17-8456-e0787d6702c6@kernel.dk>
 <YoOT7Cyobsed5IE3@google.com>
 <d503d5ff-4bc5-2bd0-00d3-cd7b0a0724cb@kernel.dk>
 <YoOW2+ov8KF1YcYF@google.com>
From:   Jens Axboe <axboe@kernel.dk>
In-Reply-To: <YoOW2+ov8KF1YcYF@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/17/22 6:36 AM, Lee Jones wrote:
> On Tue, 17 May 2022, Jens Axboe wrote:
> 
>> On 5/17/22 6:24 AM, Lee Jones wrote:
>>> On Tue, 17 May 2022, Jens Axboe wrote:
>>>
>>>> On 5/17/22 5:41 AM, Lee Jones wrote:
>>>>> Good afternoon Jens, Pavel, et al.,
>>>>>
>>>>> Not sure if you are presently aware, but there appears to be a
>>>>> use-after-free issue affecting the io_uring worker driver (fs/io-wq.c)
>>>>> in Stable v5.10.y.
>>>>>
>>>>> The full sysbot report can be seen below [0].
>>>>>
>>>>> The C-reproducer has been placed below that [1].
>>>>>
>>>>> I had great success running this reproducer in an infinite loop.
>>>>>
>>>>> My colleague reverse-bisected the fixing commit to:
>>>>>
>>>>>   commit fb3a1f6c745ccd896afadf6e2d6f073e871d38ba
>>>>>   Author: Jens Axboe <axboe@kernel.dk>
>>>>>   Date:   Fri Feb 26 09:47:20 2021 -0700
>>>>>
>>>>>        io-wq: have manager wait for all workers to exit
>>>>>
>>>>>        Instead of having to wait separately on workers and manager, just have
>>>>>        the manager wait on the workers. We use an atomic_t for the reference
>>>>>        here, as we need to start at 0 and allow increment from that. Since the
>>>>>        number of workers is naturally capped by the allowed nr of processes,
>>>>>        and that uses an int, there is no risk of overflow.
>>>>>
>>>>>        Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>>>
>>>>>     fs/io-wq.c | 30 ++++++++++++++++++++++--------
>>>>>     1 file changed, 22 insertions(+), 8 deletions(-)
>>>>
>>>> Does this fix it:
>>>>
>>>> commit 886d0137f104a440d9dfa1d16efc1db06c9a2c02
>>>> Author: Jens Axboe <axboe@kernel.dk>
>>>> Date:   Fri Mar 5 12:59:30 2021 -0700
>>>>
>>>>     io-wq: fix race in freeing 'wq' and worker access
>>>>
>>>> Looks like it didn't make it into 5.10-stable, but we can certainly
>>>> rectify that.
>>>
>>> Thanks for your quick response Jens.
>>>
>>> This patch doesn't apply cleanly to v5.10.y.
>>
>> This is probably why it never made it into 5.10-stable :-/
> 
> Right.  It doesn't apply at all unfortunately.
> 
>>> I'll have a go at back-porting it.  Please bear with me.
>>
>> Let me know if you into issues with that and I can help out.
> 
> I think the dependency list is too big.
> 
> Too much has changed that was never back-ported.
> 
> Actually the list of patches pertaining to fs/io-wq.c alone isn't so
> bad, I did start to back-port them all but some of the big ones have
> fs/io_uring.c changes incorporated and that list is huge (256 patches
> from v5.10 to the fixing patch mentioned above).

The problem is that 5.12 went to the new worker setup, and this patch
landed after that even though it also applies to the pre-native workers.
Hence the dependency chain isn't really as long as it seems, probably
just a few patches backporting the change references and completions.

I'll take a look this afternoon.

-- 
Jens Axboe