From f7bd4eb682f23c909b59cf71d5b309d53778f9d0 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Nov 2020 17:14:26 -0600 Subject: [PATCH] BACKPORT: file: Replace fcheck_files with files_lookup_fd_rcu This change renames fcheck_files to files_lookup_fd_rcu. All of the remaining callers take the rcu_read_lock before calling this function so the _rcu suffix is appropriate. This change also tightens up the debug check to verify that all callers hold the rcu_read_lock. All callers that used to call files_check with the files->file_lock held have now been changed to call files_lookup_fd_locked. This change of name has helped remind me of which locks and which guarantees are in place helping me to catch bugs later in the patchset. The need for better names became apparent in the last round of discussion of this set of changes[1]. [1] https://lkml.kernel.org/r/CAHk-=wj8BQbgJFLa+J0e=iT-1qpmCRTbPAJ8gd6MJQ=kbRPqyQ@mail.gmail.com Link: https://lkml.kernel.org/r/20201120231441.29911-9-ebiederm@xmission.com Change-Id: I000a112a13ee2b6ef0e7a2dca7923ce1b985bd2f Signed-off-by: Eric W. Biederman --- Documentation/filesystems/files.txt | 6 +++--- fs/file.c | 2 +- fs/proc/fd.c | 8 ++++---- include/linux/fdtable.h | 7 +++---- kernel/bpf/task_iter.c | 2 +- kernel/kcmp.c | 4 ++-- 6 files changed, 14 insertions(+), 15 deletions(-) diff --git a/Documentation/filesystems/files.txt b/Documentation/filesystems/files.txt index 46dfc6b038c3..6eb2eb7dde78 100644 --- a/Documentation/filesystems/files.txt +++ b/Documentation/filesystems/files.txt @@ -59,7 +59,7 @@ the fdtable structure - be held. 4. To look up the file structure given an fd, a reader - must use either fcheck() or fcheck_files() APIs. These + must use either fcheck() or files_lookup_fd_rcu() APIs. These take care of barrier requirements due to lock-free lookup. An example : @@ -80,7 +80,7 @@ the fdtable structure - on ->f_count : rcu_read_lock(); - file = fcheck_files(files, fd); + file = files_lookup_fd_rcu(files, fd); if (file) { if (atomic_long_inc_not_zero(&file->f_count)) *fput_needed = 1; @@ -100,7 +100,7 @@ the fdtable structure - lock-free, they must be installed using rcu_assign_pointer() API. If they are looked up lock-free, rcu_dereference() must be used. However it is advisable to use files_fdtable() - and fcheck()/fcheck_files() which take care of these issues. + and fcheck()/files_lookup_fd_rcu() which take care of these issues. 7. While updating, the fdtable pointer must be looked up while holding files->file_lock. If ->file_lock is dropped, then diff --git a/fs/file.c b/fs/file.c index e5f722274508..70b9a80b851b 100644 --- a/fs/file.c +++ b/fs/file.c @@ -1133,7 +1133,7 @@ SYSCALL_DEFINE2(dup2, unsigned int, oldfd, unsigned int, newfd) int retval = oldfd; rcu_read_lock(); - if (!fcheck_files(files, oldfd)) + if (!files_lookup_fd_rcu(files, oldfd)) retval = -EBADF; rcu_read_unlock(); return retval; diff --git a/fs/proc/fd.c b/fs/proc/fd.c index 81882a13212d..253e503970cd 100644 --- a/fs/proc/fd.c +++ b/fs/proc/fd.c @@ -35,7 +35,7 @@ static int seq_show(struct seq_file *m, void *v) unsigned int fd = proc_fd(m->private); spin_lock(&files->file_lock); - file = fcheck_files(files, fd); + file = files_lookup_fd_rcu(files, fd); if (file) { struct fdtable *fdt = files_fdtable(files); @@ -90,7 +90,7 @@ static bool tid_fd_mode(struct task_struct *task, unsigned fd, fmode_t *mode) return false; rcu_read_lock(); - file = fcheck_files(files, fd); + file = files_lookup_fd_rcu(files, fd); if (file) *mode = file->f_mode; rcu_read_unlock(); @@ -161,7 +161,7 @@ static int proc_fd_link(struct dentry *dentry, struct path *path) struct file *fd_file; spin_lock(&files->file_lock); - fd_file = fcheck_files(files, fd); + fd_file = files_lookup_fd_rcu(files, fd); if (fd_file) { *path = fd_file->f_path; path_get(&fd_file->f_path); @@ -250,7 +250,7 @@ static int proc_readfd_common(struct file *file, struct dir_context *ctx, char name[10 + 1]; unsigned int len; - f = fcheck_files(files, fd); + f = files_lookup_fd_rcu(files, fd); if (!f) continue; data.mode = f->f_mode; diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h index 6662e2a91472..590b5072e9f8 100644 --- a/include/linux/fdtable.h +++ b/include/linux/fdtable.h @@ -90,10 +90,9 @@ static inline struct file *__fcheck_files(struct files_struct *files, unsigned i return NULL; } -static inline struct file *fcheck_files(struct files_struct *files, unsigned int fd) +static inline struct file *files_lookup_fd_rcu(struct files_struct *files, unsigned int fd) { - RCU_LOCKDEP_WARN(!rcu_read_lock_held() && - !lockdep_is_held(&files->file_lock), + RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "suspicious rcu_dereference_check() usage"); return __fcheck_files(files, fd); } @@ -101,7 +100,7 @@ static inline struct file *fcheck_files(struct files_struct *files, unsigned int /* * Check whether the specified fd has an open file. */ -#define fcheck(fd) fcheck_files(current->files, fd) +#define fcheck(fd) files_lookup_fd_rcu(current->files, fd) struct task_struct; diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c index 8033ab19138a..61c3cbde1210 100644 --- a/kernel/bpf/task_iter.c +++ b/kernel/bpf/task_iter.c @@ -185,7 +185,7 @@ again: for (; curr_fd < max_fds; curr_fd++) { struct file *f; - f = fcheck_files(curr_files, curr_fd); + f = files_lookup_fd_rcu(curr_files, curr_fd); if (!f) continue; if (!get_file_rcu(f)) diff --git a/kernel/kcmp.c b/kernel/kcmp.c index a0e3d7a0e8b8..961feee30c35 100644 --- a/kernel/kcmp.c +++ b/kernel/kcmp.c @@ -67,7 +67,7 @@ get_file_raw_ptr(struct task_struct *task, unsigned int idx) rcu_read_lock(); if (task->files) - file = fcheck_files(task->files, idx); + file = files_lookup_fd_rcu(task->files, idx); rcu_read_unlock(); task_unlock(task); @@ -121,7 +121,7 @@ static int kcmp_epoll_target(struct task_struct *task1, return -EBADF; spin_lock(&files->file_lock); - filp_epoll = fcheck_files(files, slot.efd); + filp_epoll = files_lookup_fd_rcu(files, slot.efd); if (filp_epoll) get_file(filp_epoll); else