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
