All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: linux-fsdevel@vger.kernel.org
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Christian Brauner <christian.brauner@ubuntu.com>,
	Christoph Hellwig <hch@lst.de>,
	Giuseppe Scrivano <gscrivan@redhat.com>
Subject: [PATCH 3/3] file: simplify logic in __close_range()
Date: Fri,  2 Apr 2021 14:35:48 +0200	[thread overview]
Message-ID: <20210402123548.108372-4-brauner@kernel.org> (raw)
In-Reply-To: <00000000000069c40405be6bdad4@google.com>

From: Christian Brauner <christian.brauner@ubuntu.com>

It never looked too pleasant and it doesn't really buy us anything
anymore now that CLOSE_RANGE_CLOEXEC exists and we need to retake the
current maximum under the lock for it anyway. This also makes the logic
easier to follow.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Giuseppe Scrivano <gscrivan@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 fs/file.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 740040346a98..ed46cd3ae225 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -701,7 +701,6 @@ static inline void __range_close(struct files_struct *cur_fds, unsigned int fd,
  */
 int __close_range(unsigned fd, unsigned max_fd, unsigned int flags)
 {
-	unsigned int cur_max;
 	struct task_struct *me = current;
 	struct files_struct *cur_fds = me->files, *fds = NULL;
 
@@ -711,26 +710,26 @@ int __close_range(unsigned fd, unsigned max_fd, unsigned int flags)
 	if (fd > max_fd)
 		return -EINVAL;
 
-	rcu_read_lock();
-	cur_max = files_fdtable(cur_fds)->max_fds;
-	rcu_read_unlock();
-
-	/* cap to last valid index into fdtable */
-	cur_max--;
-
 	if (flags & CLOSE_RANGE_UNSHARE) {
 		int ret;
 		unsigned int max_unshare_fds = NR_OPEN_MAX;
 
 		/*
-		 * If the requested range is greater than the current maximum,
-		 * we're closing everything so only copy all file descriptors
-		 * beneath the lowest file descriptor.
-		 * If the caller requested all fds to be made cloexec copy all
-		 * of the file descriptors since they still want to use them.
+		 * If the caller requested all fds to be made cloexec we always
+		 * copy all of the file descriptors since they still want to
+		 * use them.
 		 */
-		if (!(flags & CLOSE_RANGE_CLOEXEC) && (max_fd >= cur_max))
-			max_unshare_fds = fd;
+		if (!(flags & CLOSE_RANGE_CLOEXEC)) {
+			/*
+			 * If the requested range is greater than the current
+			 * maximum, we're closing everything so only copy all
+			 * file descriptors beneath the lowest file descriptor.
+			 */
+			rcu_read_lock();
+			if (max_fd >= last_fd(files_fdtable(cur_fds)))
+				max_unshare_fds = fd;
+			rcu_read_unlock();
+		}
 
 		ret = unshare_fd(CLONE_FILES, max_unshare_fds, &fds);
 		if (ret)
@@ -744,8 +743,6 @@ int __close_range(unsigned fd, unsigned max_fd, unsigned int flags)
 			swap(cur_fds, fds);
 	}
 
-	max_fd = min(max_fd, cur_max);
-
 	if (flags & CLOSE_RANGE_CLOEXEC)
 		__range_cloexec(cur_fds, fd, max_fd);
 	else
-- 
2.27.0


  parent reply	other threads:[~2021-04-02 12:37 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-26  7:55 [syzbot] KASAN: null-ptr-deref Read in filp_close (2) syzbot
2021-03-26  8:02 ` Dmitry Vyukov
2021-03-26  9:12   ` Christian Brauner
2021-03-26  9:21     ` Dmitry Vyukov
     [not found]       ` <CAHrFyr7iUpMh4sicxrMWwaUHKteU=qHt-1O-3hojAAX3d5879Q@mail.gmail.com>
2021-03-26 13:50         ` Christian Brauner
2021-03-26 14:22           ` Dmitry Vyukov
2021-03-27 23:33           ` Al Viro
2021-03-29  9:21             ` Christian Brauner
2021-03-29 17:35               ` Christian Brauner
2021-04-02 12:35 ` [PATCH 0/3] file: fix and simplify close_range() Christian Brauner
2021-04-02 12:35 ` [PATCH 1/3] file: fix close_range() for unshare+cloexec Christian Brauner
2021-04-02 12:35 ` [PATCH 2/3] file: let pick_file() tell caller it's done Christian Brauner
2021-04-02 20:09   ` kernel test robot
2021-04-02 12:35 ` Christian Brauner [this message]
2021-07-13  4:12 ` [syzbot] KASAN: null-ptr-deref Read in filp_close (2) syzbot
2021-07-13 18:49   ` Linus Torvalds
2021-07-14  7:59     ` Christian Brauner
2021-07-14  9:14       ` Christian Brauner
2021-07-14 11:45       ` Dmitry Vyukov
2021-07-14 13:51   ` Christian Brauner
2021-07-14 13:54     ` syzbot
2021-07-14 13:57     ` Christian Brauner
2021-07-14 14:16       ` syzbot
2021-07-14 13:53   ` Christian Brauner
2021-07-14 13:53     ` syzbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210402123548.108372-4-brauner@kernel.org \
    --to=brauner@kernel.org \
    --cc=christian.brauner@ubuntu.com \
    --cc=dvyukov@google.com \
    --cc=gscrivan@redhat.com \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.