All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHSET] Fix io_uring async rlimit(RLIMIT_NOFILE)
@ 2020-03-20  2:22 Jens Axboe
  2020-03-20  2:22 ` [PATCH 1/2] io_uring: make sure openat/openat2 honor rlimit nofile Jens Axboe
  2020-03-20  2:22 ` [PATCH 2/2] io_uring: make sure accept " Jens Axboe
  0 siblings, 2 replies; 6+ messages in thread
From: Jens Axboe @ 2020-03-20  2:22 UTC (permalink / raw)
  To: io-uring; +Cc: davem, netdev

If we handle openat/openat2/accept in an async manner, then we need
to ensure that the max open file limit is honored. All of these end
up boiling down to the check in get_unused_fd_flags(), which does
rlimit(RLIMIT_NOFILE), which uses the current->signal->rlim[] limits.

Instead of fiddling with the task ->signal pointer, just allow us to
pass in the correct value as set from the original task at request
prep time.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/2] io_uring: make sure openat/openat2 honor rlimit nofile
  2020-03-20  2:22 [PATCHSET] Fix io_uring async rlimit(RLIMIT_NOFILE) Jens Axboe
@ 2020-03-20  2:22 ` Jens Axboe
  2020-03-20  4:43   ` David Miller
  2020-03-20  2:22 ` [PATCH 2/2] io_uring: make sure accept " Jens Axboe
  1 sibling, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2020-03-20  2:22 UTC (permalink / raw)
  To: io-uring; +Cc: davem, netdev, Jens Axboe, Dmitry Kadashev

Dmitry reports that a test case shows that io_uring isn't honoring a
modified rlimit nofile setting. get_unused_fd_flags() checks the task
signal->rlimi[] for the limits. As this isn't easily inheritable,
provide a __get_unused_fd_flags() that takes the value instead. Then we
can grab it when the request is prepared (from the original task), and
pass that in when we do the async part part of the open.

Reported-by: Dmitry Kadashev <dkadashev@gmail.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/file.c            | 7 ++++++-
 fs/io_uring.c        | 5 ++++-
 include/linux/file.h | 1 +
 3 files changed, 11 insertions(+), 2 deletions(-)

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 b1fbc4424aa6..fe5ded7c74ef 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -397,6 +397,7 @@ struct io_open {
 	struct filename			*filename;
 	struct statx __user		*buffer;
 	struct open_how			how;
+	unsigned long			nofile;
 };
 
 struct io_files_update {
@@ -2577,6 +2578,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;
 }
@@ -2618,6 +2620,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;
 }
@@ -2636,7 +2639,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);
 
-- 
2.25.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] io_uring: make sure accept honor rlimit nofile
  2020-03-20  2:22 [PATCHSET] Fix io_uring async rlimit(RLIMIT_NOFILE) Jens Axboe
  2020-03-20  2:22 ` [PATCH 1/2] io_uring: make sure openat/openat2 honor rlimit nofile Jens Axboe
@ 2020-03-20  2:22 ` Jens Axboe
  2020-03-20  4:43   ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2020-03-20  2:22 UTC (permalink / raw)
  To: io-uring; +Cc: davem, netdev, Jens Axboe

Just like commit 21ec2da35ce3, this fixes the fact that
IORING_OP_ACCEPT ends up using get_unused_fd_flags(), which checks
current->signal->rlim[] for limits.

Add an extra argument to __sys_accept4_file() that allows us to pass
in the proper nofile limit, and grab it at request prep time.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c          | 5 ++++-
 include/linux/socket.h | 3 ++-
 net/socket.c           | 8 +++++---
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index fe5ded7c74ef..3affd96a98ba 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -343,6 +343,7 @@ struct io_accept {
 	struct sockaddr __user		*addr;
 	int __user			*addr_len;
 	int				flags;
+	unsigned long			nofile;
 };
 
 struct io_sync {
@@ -3324,6 +3325,7 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	accept->addr = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	accept->addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr2));
 	accept->flags = READ_ONCE(sqe->accept_flags);
+	accept->nofile = rlimit(RLIMIT_NOFILE);
 	return 0;
 #else
 	return -EOPNOTSUPP;
@@ -3340,7 +3342,8 @@ static int __io_accept(struct io_kiocb *req, struct io_kiocb **nxt,
 
 	file_flags = force_nonblock ? O_NONBLOCK : 0;
 	ret = __sys_accept4_file(req->file, file_flags, accept->addr,
-					accept->addr_len, accept->flags);
+					accept->addr_len, accept->flags,
+					accept->nofile);
 	if (ret == -EAGAIN && force_nonblock)
 		return -EAGAIN;
 	if (ret == -ERESTARTSYS)
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 2d2313403101..15f3412d481e 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -401,7 +401,8 @@ extern int __sys_sendto(int fd, void __user *buff, size_t len,
 			int addr_len);
 extern int __sys_accept4_file(struct file *file, unsigned file_flags,
 			struct sockaddr __user *upeer_sockaddr,
-			 int __user *upeer_addrlen, int flags);
+			 int __user *upeer_addrlen, int flags,
+			 unsigned long nofile);
 extern int __sys_accept4(int fd, struct sockaddr __user *upeer_sockaddr,
 			 int __user *upeer_addrlen, int flags);
 extern int __sys_socket(int family, int type, int protocol);
diff --git a/net/socket.c b/net/socket.c
index b79a05de7c6e..2eecf1517f76 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1707,7 +1707,8 @@ SYSCALL_DEFINE2(listen, int, fd, int, backlog)
 
 int __sys_accept4_file(struct file *file, unsigned file_flags,
 		       struct sockaddr __user *upeer_sockaddr,
-		       int __user *upeer_addrlen, int flags)
+		       int __user *upeer_addrlen, int flags,
+		       unsigned long nofile)
 {
 	struct socket *sock, *newsock;
 	struct file *newfile;
@@ -1738,7 +1739,7 @@ int __sys_accept4_file(struct file *file, unsigned file_flags,
 	 */
 	__module_get(newsock->ops->owner);
 
-	newfd = get_unused_fd_flags(flags);
+	newfd = __get_unused_fd_flags(flags, nofile);
 	if (unlikely(newfd < 0)) {
 		err = newfd;
 		sock_release(newsock);
@@ -1807,7 +1808,8 @@ int __sys_accept4(int fd, struct sockaddr __user *upeer_sockaddr,
 	f = fdget(fd);
 	if (f.file) {
 		ret = __sys_accept4_file(f.file, 0, upeer_sockaddr,
-						upeer_addrlen, flags);
+						upeer_addrlen, flags,
+						rlimit(RLIMIT_NOFILE));
 		if (f.flags)
 			fput(f.file);
 	}
-- 
2.25.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] io_uring: make sure openat/openat2 honor rlimit nofile
  2020-03-20  2:22 ` [PATCH 1/2] io_uring: make sure openat/openat2 honor rlimit nofile Jens Axboe
@ 2020-03-20  4:43   ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-03-20  4:43 UTC (permalink / raw)
  To: axboe; +Cc: io-uring, netdev, dkadashev

From: Jens Axboe <axboe@kernel.dk>
Date: Thu, 19 Mar 2020 20:22:15 -0600

> Dmitry reports that a test case shows that io_uring isn't honoring a
> modified rlimit nofile setting. get_unused_fd_flags() checks the task
> signal->rlimi[] for the limits. As this isn't easily inheritable,
> provide a __get_unused_fd_flags() that takes the value instead. Then we
> can grab it when the request is prepared (from the original task), and
> pass that in when we do the async part part of the open.
> 
> Reported-by: Dmitry Kadashev <dkadashev@gmail.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] io_uring: make sure accept honor rlimit nofile
  2020-03-20  2:22 ` [PATCH 2/2] io_uring: make sure accept " Jens Axboe
@ 2020-03-20  4:43   ` David Miller
  2020-03-20 14:48     ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2020-03-20  4:43 UTC (permalink / raw)
  To: axboe; +Cc: io-uring, netdev

From: Jens Axboe <axboe@kernel.dk>
Date: Thu, 19 Mar 2020 20:22:16 -0600

> Just like commit 21ec2da35ce3, this fixes the fact that
> IORING_OP_ACCEPT ends up using get_unused_fd_flags(), which checks
> current->signal->rlim[] for limits.
> 
> Add an extra argument to __sys_accept4_file() that allows us to pass
> in the proper nofile limit, and grab it at request prep time.
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] io_uring: make sure accept honor rlimit nofile
  2020-03-20  4:43   ` David Miller
@ 2020-03-20 14:48     ` Jens Axboe
  0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2020-03-20 14:48 UTC (permalink / raw)
  To: David Miller; +Cc: io-uring, netdev

On 3/19/20 10:43 PM, David Miller wrote:
> From: Jens Axboe <axboe@kernel.dk>
> Date: Thu, 19 Mar 2020 20:22:16 -0600
> 
>> Just like commit 21ec2da35ce3, this fixes the fact that
>> IORING_OP_ACCEPT ends up using get_unused_fd_flags(), which checks
>> current->signal->rlim[] for limits.
>>
>> Add an extra argument to __sys_accept4_file() that allows us to pass
>> in the proper nofile limit, and grab it at request prep time.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Thanks, added to both.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-20 14:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20  2:22 [PATCHSET] Fix io_uring async rlimit(RLIMIT_NOFILE) Jens Axboe
2020-03-20  2:22 ` [PATCH 1/2] io_uring: make sure openat/openat2 honor rlimit nofile Jens Axboe
2020-03-20  4:43   ` David Miller
2020-03-20  2:22 ` [PATCH 2/2] io_uring: make sure accept " Jens Axboe
2020-03-20  4:43   ` David Miller
2020-03-20 14:48     ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.