Re: openat ignores changes to RLIMIT_NOFILE?

[Dmitry Kadashev <dkadashev@gmail.com>]

Hi Jens,

Yes, with the patch it works perfectly, thank you.

-- 
Dmitry


On Fri, Mar 20, 2020 at 8:23 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 3/19/20 6:12 AM, Dmitry Kadashev wrote:
> > Hi,
> >
> > It seems that openat calls issued via io_uring ignore changes to
> > RLIMIT_NOFILE. Maybe a wrong limit is checked. A short reproducer is
> > attached, it sets RLIMIT_NOFILE to a very low value and the sync
> > openat() call fails with "Too many open files", but io_uring one
> > succeeds. The resulting FD is completely usable, I've tried writing to
> > it successfully.
> >
> > To be clear, originally I've encountered another side of this problem:
> > we increase the limit in our code, and io_uring's openat started to
> > fail after a while under load, while the sync calls executed on a
> > thread pool were working as expected. It's just easier to demo with
> > small limit.
> >
> > Kernel 5.6-rc2, 5.6-rc6.
> >
> > Hope it's the right place to report an issue like this.
>
> Can you try the below patch?
>
>
> diff --git a/fs/file.c b/fs/file.c
> index a364e1a9b7e8..c8a4e4c86e55 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -540,9 +540,14 @@ static int alloc_fd(unsigned start, unsigned flags)
>         return __alloc_fd(current->files, start, rlimit(RLIMIT_NOFILE), flags);
>  }
>
> +int __get_unused_fd_flags(unsigned flags, unsigned long nofile)
> +{
> +       return __alloc_fd(current->files, 0, nofile, flags);
> +}
> +
>  int get_unused_fd_flags(unsigned flags)
>  {
> -       return __alloc_fd(current->files, 0, rlimit(RLIMIT_NOFILE), flags);
> +       return __get_unused_fd_flags(flags, rlimit(RLIMIT_NOFILE));
>  }
>  EXPORT_SYMBOL(get_unused_fd_flags);
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index c06082bb039a..be5705ff33b4 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -398,6 +398,7 @@ struct io_open {
>         struct filename                 *filename;
>         struct statx __user             *buffer;
>         struct open_how                 how;
> +       unsigned long                   nofile;
>  };
>
>  struct io_files_update {
> @@ -2578,6 +2579,7 @@ static int io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>                 return ret;
>         }
>
> +       req->open.nofile = rlimit(RLIMIT_NOFILE);
>         req->flags |= REQ_F_NEED_CLEANUP;
>         return 0;
>  }
> @@ -2619,6 +2621,7 @@ static int io_openat2_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>                 return ret;
>         }
>
> +       req->open.nofile = rlimit(RLIMIT_NOFILE);
>         req->flags |= REQ_F_NEED_CLEANUP;
>         return 0;
>  }
> @@ -2637,7 +2640,7 @@ static int io_openat2(struct io_kiocb *req, struct io_kiocb **nxt,
>         if (ret)
>                 goto err;
>
> -       ret = get_unused_fd_flags(req->open.how.flags);
> +       ret = __get_unused_fd_flags(req->open.how.flags, req->open.nofile);
>         if (ret < 0)
>                 goto err;
>
> diff --git a/include/linux/file.h b/include/linux/file.h
> index c6c7b24ea9f7..142d102f285e 100644
> --- a/include/linux/file.h
> +++ b/include/linux/file.h
> @@ -85,6 +85,7 @@ extern int f_dupfd(unsigned int from, struct file *file, unsigned flags);
>  extern int replace_fd(unsigned fd, struct file *file, unsigned flags);
>  extern void set_close_on_exec(unsigned int fd, int flag);
>  extern bool get_close_on_exec(unsigned int fd);
> +extern int __get_unused_fd_flags(unsigned flags, unsigned long nofile);
>  extern int get_unused_fd_flags(unsigned flags);
>  extern void put_unused_fd(unsigned int fd);
>
> --
> Jens Axboe
>