openat ignores changes to RLIMIT_NOFILE?

openat ignores changes to RLIMIT_NOFILE?

[Dmitry Kadashev]

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

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.

Thanks.

-- 
Dmitry

[-- Attachment #2: test-io_uring-openat-rlimit.c --]
[-- Type: text/x-csrc, Size: 2135 bytes --]

#include <liburing.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <errno.h>
#include <sys/resource.h>
#include <unistd.h>

#define DIE(...) do {\
		fprintf(stderr, __VA_ARGS__);\
		abort();\
	} while(0);

static const int RSIZE = 2;
static const int OPEN_FLAGS = O_RDWR | O_CREAT;
static const mode_t OPEN_MODE = S_IRUSR | S_IWUSR;

void setup_rlimit()
{
	struct rlimit rlim;
	rlim.rlim_cur = rlim.rlim_max = 5; // 3 stdio ones, 1 for uring, 1 for dirfd
	if (setrlimit(RLIMIT_NOFILE, &rlim) == -1) {
		DIE("setrlimit nofile: %s\n", strerror(errno));
	}
}

void open_sync(int dfd, const char* fn)
{
	int fd = openat(dfd, fn, OPEN_FLAGS, OPEN_MODE);
	if (fd < 0) {
		fprintf(stderr, "sync open failed: %s\n", strerror(errno));
	}
	else {
		fprintf(stderr, "sync open succeeded\n");
		close(fd);
	}
}

void open_io_uring(struct io_uring *ring, int dfd, const char* fn)
{
	struct io_uring_sqe *sqe;
	sqe = io_uring_get_sqe(ring);
	if (!sqe) {
		fprintf(stderr, "failed to get sqe\n");
		return;
	}
	io_uring_prep_openat(sqe, dfd, fn, OPEN_FLAGS, OPEN_MODE);
	int ret = io_uring_submit(ring);
	if (ret < 0) {
		fprintf(stderr, "failed to submit openat: %s\n", strerror(-ret));
		return;
	}

	struct io_uring_cqe *cqe;
	ret = io_uring_wait_cqe(ring, &cqe);
	int fd = cqe->res;
	io_uring_cqe_seen(ring, cqe);
	if (ret < 0) {
		fprintf(stderr, "wait_cqe failed: %s\n", strerror(-ret));
	}
	else if (fd < 0) {
		fprintf(stderr, "io_uring openat failed: %s\n", strerror(-fd));
	}
	else {
		fprintf(stderr, "io_uring openat succeeded\n");
		close(fd);
	}
}

int main(int argc, const char *argv[])
{
	const char *mode = "io_uring";
	const char *fn = "io_uring_openat_test";
	setup_rlimit();
	int dfd = open("/tmp", O_RDONLY | O_DIRECTORY);
	if (dfd < 0) {
		DIE("open /tmp: %s\n", strerror(errno));
	}
	struct io_uring ring;
	int ret = io_uring_queue_init(RSIZE, &ring, 0);
	if (ret < 0) {
		DIE("failed to init io_uring: %s\n", strerror(-ret));
	}

	open_sync(dfd, fn);
	open_io_uring(&ring, dfd, fn);

	io_uring_queue_exit(&ring);
	close(dfd);
	return 0;
}

Re: openat ignores changes to RLIMIT_NOFILE?

[Jens Axboe]

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

Re: openat ignores changes to RLIMIT_NOFILE?

[Dmitry Kadashev]

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
>

Re: openat ignores changes to RLIMIT_NOFILE?

[Jens Axboe]

On 3/20/20 8:03 AM, Dmitry Kadashev wrote:
> Hi Jens,
> 
> Yes, with the patch it works perfectly, thank you.

Great thanks, I'm going to add your Tested-by to the commit.

-- 
Jens Axboe