Re: [PATCH] io_uring: fix NULL pointer dereference for async cancel close

[Pavel Begunkov <asml.silence@gmail.com>]

On 19/01/2021 08:00, Joseph Qi wrote:
> 
> 
> On 1/19/21 10:38 AM, Pavel Begunkov wrote:
>> On 19/01/2021 01:58, Joseph Qi wrote:
>>>> Hmm, I hastened, for files we need IO_WQ_WORK_FILES,
>>>> +IO_WQ_WORK_BLKCG for same reasons. needs_file would make 
>>>> it to grab a struct file, that is wrong.
>>>> Probably worked out because it just grabbed fd=0/stdin.
>>>>
>>>
>>> I think IO_WQ_WORK_FILES can work since it will acquire
>>> files when initialize async cancel request.
>>
>> That the one controlling files in the first place, need_file
>> just happened to grab them submission.
>>
>>> Don't quite understand why we should have IO_WQ_WORK_BLKCG.
>>
>> Because it's set for IORING_OP_CLOSE, and similar situation
>> may happen but with async_cancel from io-wq.
>>
> So how about do switch and restore in io_run_cancel(), seems it can
> take care of direct request, sqthread and io-wq cases.

It will get ugly pretty quickly, + this nesting of io-wq handlers
async_handler() -> io_close() is not great...

I'm more inclined to skip them in io_wqe_cancel_pending_work()
to not execute inline. That may need to do some waiting on the
async_cancel side though to not change the semantics. Can you
try out this direction?


> 
>> Actually, it's even nastier than that, and neither of io_op_def
>> flags would work because for io-wq case you can end up doing
>> close() with different from original files. I'll think how it
>> can be done tomorrow.
>>

-- 
Pavel Begunkov