From: Linus Torvalds Date: Tue, 31 Aug 2021 19:00:07 +0000 (-0700) Subject: Merge tag 'fs.close_range.v5.15' of git://git.kernel.org/pub/scm/linux/kernel/git... X-Git-Tag: microblaze-v5.16~175 X-Git-Url: http://git.monstr.eu/?p=linux-2.6-microblaze.git;a=commitdiff_plain;h=927bc120a248b658acc2f5206ec4e81a794d8a57;hp=1dd5915a5cbda100e67823e7a4ca7af919185ea6 Merge tag 'fs.close_range.v5.15' of git://git./linux/kernel/git/brauner/linux Pull close_range() cleanup from Christian Brauner: "This is a cleanup for close_range() which was sent as part of a bugfix we did some time ago in commit 9b5b872215fe ("file: fix close_range() for unshare+cloexec"). We used to share more code between some helpers for close_range() which made retrieving the maximum number of open fds before calling into the helpers sensible. But with the introduction of CLOSE_RANGE_CLOEXEC and the need to retrieve the number of maximum fds once more for CLOSE_RANGE_CLOEXEC that stopped making sense. So the code was in a dumb in-limbo state. Fix this by simplifying the code a bit. The original idea was to only fix the bug itself and make backporting easy. And since the cleanup wasn't very pressing I left it in linux-next for a very long time. I didn't pull the patches from the list again back then which is why they don't have lore-links. So I'm listing them below explicitly" Commit 03ba0fe4d09f ("file: simplify logic in __close_range()") Link: https://lore.kernel.org/linux-fsdevel/20210402123548.108372-3-brauner@kernel.org Commit f49fd6d3c070 ("file: let pick_file() tell caller it's done") Link: https://lore.kernel.org/linux-fsdevel/20210402123548.108372-4-brauner@kernel.org * tag 'fs.close_range.v5.15' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux: file: simplify logic in __close_range() file: let pick_file() tell caller it's done --- diff --git a/fs/file.c b/fs/file.c index 86dc9956af32..d8afa8266859 100644 --- a/fs/file.c +++ b/fs/file.c @@ -596,18 +596,32 @@ void fd_install(unsigned int fd, struct file *file) EXPORT_SYMBOL(fd_install); +/** + * pick_file - return file associatd with fd + * @files: file struct to retrieve file from + * @fd: file descriptor to retrieve file for + * + * If this functions returns an EINVAL error pointer the fd was beyond the + * current maximum number of file descriptors for that fdtable. + * + * Returns: The file associated with @fd, on error returns an error pointer. + */ static struct file *pick_file(struct files_struct *files, unsigned fd) { - struct file *file = NULL; + struct file *file; struct fdtable *fdt; spin_lock(&files->file_lock); fdt = files_fdtable(files); - if (fd >= fdt->max_fds) + if (fd >= fdt->max_fds) { + file = ERR_PTR(-EINVAL); goto out_unlock; + } file = fdt->fd[fd]; - if (!file) + if (!file) { + file = ERR_PTR(-EBADF); goto out_unlock; + } rcu_assign_pointer(fdt->fd[fd], NULL); __put_unused_fd(files, fd); @@ -622,7 +636,7 @@ int close_fd(unsigned fd) struct file *file; file = pick_file(files, fd); - if (!file) + if (IS_ERR(file)) return -EBADF; return filp_close(file, files); @@ -663,11 +677,16 @@ static inline void __range_close(struct files_struct *cur_fds, unsigned int fd, struct file *file; file = pick_file(cur_fds, fd++); - if (!file) + if (!IS_ERR(file)) { + /* found a valid file to close */ + filp_close(file, cur_fds); + cond_resched(); continue; + } - filp_close(file, cur_fds); - cond_resched(); + /* beyond the last fd in that table */ + if (PTR_ERR(file) == -EINVAL) + return; } } @@ -682,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; @@ -692,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) @@ -725,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