Re: [PATCH 5.11] io_uring: NULL files dereference by SQPOLL

[Jens Axboe <axboe@kernel.dk>]

On 11/7/20 3:47 PM, Pavel Begunkov wrote:
> On 07/11/2020 22:28, Jens Axboe wrote:
>> On 11/7/20 2:54 PM, Pavel Begunkov wrote:
>>> On 07/11/2020 21:18, Pavel Begunkov wrote:
>>>> On 07/11/2020 21:16, Pavel Begunkov wrote:
>>>>> SQPOLL task may find sqo_task->files == NULL, so
>>>>> __io_sq_thread_acquire_files() would left it unset and so all the
>>>>> following fails, e.g. attempts to submit. Fail if sqo_task doesn't have
>>>>> files.
>>>>
>>>> Josef, could you try this one?
>>>
>>> Hmm, as you said it happens often... IIUC there is a drawback with
>>> SQPOLL -- after the creator process/thread exits most of subsequent
>>> requests will start failing.
>>> I'd say from application correctness POV such tasks should exit
>>> only after their SQPOLL io_urings got killed.
>>
>> I don't think there's anything wrong with that - if you submit requests
>> and exit before they have completed, then you by definition are not
>> caring about the result of them.
> 
> Other threads may use it as well thinking that this is fine as
> they share mm, files, etc.
> 
> 1. task1 create io_uring
> 2. clone(CLONE_FILES|CLONE_VM|...) -> task2
> 3. task1 exits
> 4. task2 continues to use io_uring

Sure, but I think this is getting pretty contrived. Yes, if the task
that created the ring (and whose credentials are being used) exits,
then the ring is effectively dead if you're using SQPOLL. If you're
using threads, the threads go away too.

-- 
Jens Axboe