On Thu, May 07, 2020 at 01:05:23PM -0600, Jens Axboe wrote: > On 5/7/20 1:01 PM, Al Viro wrote: > > On Thu, May 07, 2020 at 08:57:25PM +0200, Max Kellermann wrote: > >> If an operation's flag `needs_file` is set, the function > >> io_req_set_file() calls io_file_get() to obtain a `struct file*`. > >> > >> This fails for `O_PATH` file descriptors, because those have no > >> `struct file*` > > > > O_PATH descriptors most certainly *do* have that. What the hell > > are you talking about? > > Yeah, hence I was interested in the test case. Since this is > bypassing that part, was assuming we'd have some logic error > that attempted a file grab for a case where we shouldn't. Just in case - you do realize that you should either resolve the descriptor yourself (and use the resulting struct file *, without letting anyone even look at the descriptor) *or* pass the descriptor as-is and don't even look at the descriptor table? Once more, with feeling: Descriptor tables are inherently sharable objects. You can't resolve a descriptor twice and assume you'll get the same thing both times. You can't insert something into descriptor table and assume that the same slot will be holding the same struct file reference after the descriptor table has been unlocked. Again, resolving the descriptor more than once in course of syscall is almost always a serious bug; there are very few exceptions and none of the mentioned in that patch are anywhere near those. IOW, that patch will either immediately break things on O_PATH (if you are really passing struct file *) or it's probably correct, but the reason is entirely different - it's that you are passing descriptor, which gets resolved by whatever you are calling, in which case io_uring has no business resolving it. And if that's the case, you are limited to real descriptors - your descriptor table lookalikes won't be of any use.
