Re: [PATCH v2] io_uring: fix dead-hung for non-iter fixed rw

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

On 11/25/2019 5:37 AM, Jens Axboe wrote:
> On 11/24/19 10:52 AM, Pavel Begunkov wrote:
>> On 24/11/2019 20:10, Jens Axboe wrote:
>>> On 11/24/19 1:58 AM, Pavel Begunkov wrote:
>>>> Read/write requests to devices without implemented read/write_iter
>>>> using fixed buffers causes general protection fault, which totally
>>>> hangs a machine.
>>>>
>>>> io_import_fixed() initialises iov_iter with bvec, but loop_rw_iter()
>>>> accesses it as iovec, so dereferencing random address.
>>>>
>>>> kmap() page by page in this case
>>>
>>> This looks good to me, much cleaner/simpler. I've added a few pipe fixed
>>> buffer tests to liburing as well. Didn't crash for me, but obvious
>>> garbage coming out. I've flagged this for stable as well.
>>>
>> The problem I have is that __user pointer is meant to be checked
>> for not being a kernel address. I suspect, it could fail in some
>> device, which double checks the pointer after vfs (e.g. using access_ok()).
>> Am I wrong? Not a fault at least...
>>
>> #define access_ok(...) __range_not_ok(addr, user_addr_max());
> 
> They are user pages! So this should be totally fine. The only difference
> is that we have them pre-mapped. But it's not like we're pretending
> these kernel pages are user pages, and hence access_ok() should be
> totally happy with them.
> 
Good, if you say so, but I'll take the liberty of asking a little bit
more :)

Yes, they are user pages, but that's rather about used virtual
addresses. Having virtual address space separation (e.g. [0-n): user's
virtual address space part, [n-m): kernel's one), I'd expect __user ptr
to be checked to be bounded by [0-n). E.g. copy_to_user() obviously
shouldn't write into kernel's addresses. And I also assume that kmap()
maps into [n-m), at least because the kernel may want to allocate and
kmap() pages, and use them internally.

And that's why I thought it may fail.
E.g. vfs_read_sys((__user void*)kmap()) _should_ fail.
Where is my mistake?

>> BTW, is there anybody testing it for non x86-64 arch?
> 
> Would be nice, I've mostly failed at getting other archs interested
> enough to actually make hardware available. Which seems pretty lame, but
> only so much I can do there. There _shouldn't_ be anything arch
> specific, but it would be great to have archs with eg weaker ordering as
> part of the regular test arsenal.
> 
Yeah, that's the point. It probably needs some use in Android to turn
the things over.

-- 
Pavel Begunkov