Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-08 Thread Jens Axboe
On 5/8/20 9:29 AM, Hillf Danton wrote: > Dunno if what's missing makes grumpy. > > --- a/fs/io_uring.c > +++ b/fs/io_uring.c > @@ -3439,6 +3439,11 @@ static void io_close_finish(struct io_wq > static int io_close(struct io_kiocb *req, bool force_nonblock) > { > int ret; > + struct fd

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Jens Axboe
On 5/7/20 8:28 PM, Jens Axboe wrote: > On 5/7/20 5:31 PM, Al Viro wrote: >> On Thu, May 07, 2020 at 05:03:17PM -0600, Jens Axboe wrote: >>> On 5/7/20 4:44 PM, Al Viro wrote: On Thu, May 07, 2020 at 04:25:24PM -0600, Jens Axboe wrote: > static int io_close(struct io_kiocb *req, bool

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Jens Axboe
On 5/7/20 5:31 PM, Al Viro wrote: > On Thu, May 07, 2020 at 05:03:17PM -0600, Jens Axboe wrote: >> On 5/7/20 4:44 PM, Al Viro wrote: >>> On Thu, May 07, 2020 at 04:25:24PM -0600, Jens Axboe wrote: >>> static int io_close(struct io_kiocb *req, bool force_nonblock) { + struct

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Al Viro
On Thu, May 07, 2020 at 05:03:17PM -0600, Jens Axboe wrote: > On 5/7/20 4:44 PM, Al Viro wrote: > > On Thu, May 07, 2020 at 04:25:24PM -0600, Jens Axboe wrote: > > > >> static int io_close(struct io_kiocb *req, bool force_nonblock) > >> { > >> + struct files_struct *files = current->files; >

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Jens Axboe
On 5/7/20 4:44 PM, Al Viro wrote: > On Thu, May 07, 2020 at 04:25:24PM -0600, Jens Axboe wrote: > >> static int io_close(struct io_kiocb *req, bool force_nonblock) >> { >> +struct files_struct *files = current->files; >> int ret; >> >> req->close.put_file = NULL; >> -ret =

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Al Viro
On Thu, May 07, 2020 at 04:25:24PM -0600, Jens Axboe wrote: > static int io_close(struct io_kiocb *req, bool force_nonblock) > { > + struct files_struct *files = current->files; > int ret; > > req->close.put_file = NULL; > - ret = __close_fd_get_file(req->close.fd,

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Jens Axboe
On 5/7/20 4:06 PM, Al Viro wrote: > On Thu, May 07, 2020 at 02:53:30PM -0600, Jens Axboe wrote: > >> I think the patch is correct as-is, I took a good look at how we're >> currently handling it. None of those three ops should fiddle with >> the fd at all, and all of them do forbid the use of

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Al Viro
On Thu, May 07, 2020 at 02:53:30PM -0600, Jens Axboe wrote: > I think the patch is correct as-is, I took a good look at how we're > currently handling it. None of those three ops should fiddle with > the fd at all, and all of them do forbid the use of fixed files (the > descriptor table

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Jens Axboe
On 5/7/20 1:29 PM, Al Viro wrote: > 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

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Max Kellermann
On 2020/05/07 21:29, Al Viro wrote: > Again, resolving the descriptor more than once in course of syscall > is almost always a serious bug; .. and that is what Linux currently does for those three operation, and yes, it's buggy. The generic preparation code looks up the fd, but later in the

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Al Viro
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*`. >

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Max Kellermann
On 2020/05/07 21:05, 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

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Jens Axboe
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

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Max Kellermann
On 2020/05/07 21:01, 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

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Al Viro
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

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Max Kellermann
On 2020/05/07 20:58, Jens Axboe wrote: > Do you happen to have a liburing test addition for this as well? No, I'll write one tomorrow. GitHub PR or email preferred? Max

Re: [PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Jens Axboe
On 5/7/20 12:57 PM, 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*`, causing io_req_set_file() to throw

[PATCH] fs/io_uring: fix O_PATH fds in openat, openat2, statx

2020-05-07 Thread Max Kellermann
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*`, causing io_req_set_file() to throw `-EBADF`. This breaks the operations `openat`, `openat2`