Chuck Lever (1): Revert "fget: clarify and improve __fget_files() implementation"
Eric W. Biederman (2): file: Rename __fcheck_files to files_lookup_fd_raw file: Factor files_lookup_fd_locked out of fcheck_files
Jann Horn (1): filelock: Fix fcntl/close race recovery compat path
include/linux/fdtable.h | 11 ++++-- fs/file.c | 76 ++++++++++------------------------------- fs/locks.c | 23 +++++++------ fs/proc/fd.c | 2 +- 4 files changed, 40 insertions(+), 72 deletions(-)
From: Chuck Lever chuck.lever@oracle.com
stable inclusion from stable-v5.10.220 commit e6f42bc11a60571768c96f74a8bdf4fac5624801 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAGEF4 CVE: CVE-2024-41020
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
Temporarily revert commit 0849f83e4782 ("fget: clarify and improve __fget_files() implementation") to enable subsequent upstream commits to apply and build cleanly.
Stable-dep-of: bebf684bf330 ("file: Rename __fcheck_files to files_lookup_fd_raw") Signed-off-by: Chuck Lever chuck.lever@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org Conflicts: fs/file.c [Some context differences: the spaces are not same.] Signed-off-by: Yifan Qiao qiaoyifan4@huawei.com --- fs/file.c | 72 +++++++++++++------------------------------------------ 1 file changed, 16 insertions(+), 56 deletions(-)
diff --git a/fs/file.c b/fs/file.c index f1b6faa87e3d..79968149b863 100644 --- a/fs/file.c +++ b/fs/file.c @@ -909,68 +909,28 @@ void do_close_on_exec(struct files_struct *files) spin_unlock(&files->file_lock); }
-static inline struct file *__fget_files_rcu(struct files_struct *files, - unsigned int fd, fmode_t mask, unsigned int refs) -{ - for (;;) { - struct file *file; - struct fdtable *fdt = rcu_dereference_raw(files->fdt); - struct file __rcu **fdentry; - - if (unlikely(fd >= fdt->max_fds)) - return NULL; - - fdentry = fdt->fd + array_index_nospec(fd, fdt->max_fds); - file = rcu_dereference_raw(*fdentry); - if (unlikely(!file)) - return NULL; - - if (unlikely(file->f_mode & mask)) - return NULL; - - /* - * Ok, we have a file pointer. However, because we do - * this all locklessly under RCU, we may be racing with - * that file being closed. - * - * Such a race can take two forms: - * - * (a) the file ref already went down to zero, - * and get_file_rcu_many() fails. Just try - * again: - */ - if (unlikely(!get_file_rcu_many(file, refs))) - continue; - - /* - * (b) the file table entry has changed under us. - * Note that we don't need to re-check the 'fdt->fd' - * pointer having changed, because it always goes - * hand-in-hand with 'fdt'. - * - * If so, we need to put our refs and try again. - */ - if (unlikely(rcu_dereference_raw(files->fdt) != fdt) || - unlikely(rcu_dereference_raw(*fdentry) != file)) { - fput_many(file, refs); - continue; - } - - /* - * Ok, we have a ref to the file, and checked that it - * still exists. - */ - return file; - } -} - static struct file *__fget_files(struct files_struct *files, unsigned int fd, fmode_t mask, unsigned int refs) { struct file *file;
rcu_read_lock(); - file = __fget_files_rcu(files, fd, mask, refs); +loop: + file = fcheck_files(files, fd); + if (file) { + /* File object ref couldn't be taken. + * dup2() atomicity guarantee is the reason + * we loop to catch the new file (or NULL pointer) + */ + if (file->f_mode & mask) + file = NULL; + else if (!get_file_rcu_many(file, refs)) + goto loop; + else if (__fcheck_files(files, fd) != file) { + fput_many(file, refs); + goto loop; + } + } rcu_read_unlock();
return file;
From: "Eric W. Biederman" ebiederm@xmission.com
stable inclusion from stable-v5.10.220 commit ddb21f9984209b2c502ed28698918975528721f5 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAGEF4 CVE: CVE-2024-41020
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
[ Upstream commit bebf684bf330915e6c96313ad7db89a5480fc9c2 ]
The function fcheck despite it's comment is poorly named as it has no callers that only check it's return value. All of fcheck's callers use the returned file descriptor. The same is true for fcheck_files and __fcheck_files.
A new less confusing name is needed. In addition the names of these functions are confusing as they do not report the kind of locks that are needed to be held when these functions are called making error prone to use them.
To remedy this I am making the base functio name lookup_fd and will and prefixes and sufficies to indicate the rest of the context.
Name the function (previously called __fcheck_files) that proceeds from a struct files_struct, looks up the struct file of a file descriptor, and requires it's callers to verify all of the appropriate locks are held files_lookup_fd_raw.
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=kbRPqy... Link: https://lkml.kernel.org/r/20201120231441.29911-7-ebiederm@xmission.com Signed-off-by: Eric W. Biederman ebiederm@xmission.com [ cel: adjusted to apply to v5.10.y ] Signed-off-by: Chuck Lever chuck.lever@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org Signed-off-by: Yifan Qiao qiaoyifan4@huawei.com --- include/linux/fdtable.h | 4 ++-- fs/file.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h index e2df70d7bcc3..4b4410fc1282 100644 --- a/include/linux/fdtable.h +++ b/include/linux/fdtable.h @@ -81,7 +81,7 @@ struct dentry; /* * The caller must ensure that fd table isn't shared or hold rcu or file lock */ -static inline struct file *__fcheck_files(struct files_struct *files, unsigned int fd) +static inline struct file *files_lookup_fd_raw(struct files_struct *files, unsigned int fd) { struct fdtable *fdt = rcu_dereference_raw(files->fdt);
@@ -97,7 +97,7 @@ static inline struct file *fcheck_files(struct files_struct *files, unsigned int RCU_LOCKDEP_WARN(!rcu_read_lock_held() && !lockdep_is_held(&files->file_lock), "suspicious rcu_dereference_check() usage"); - return __fcheck_files(files, fd); + return files_lookup_fd_raw(files, fd); }
/* diff --git a/fs/file.c b/fs/file.c index 79968149b863..a35374680ff2 100644 --- a/fs/file.c +++ b/fs/file.c @@ -926,7 +926,7 @@ static struct file *__fget_files(struct files_struct *files, unsigned int fd, file = NULL; else if (!get_file_rcu_many(file, refs)) goto loop; - else if (__fcheck_files(files, fd) != file) { + else if (files_lookup_fd_raw(files, fd) != file) { fput_many(file, refs); goto loop; } @@ -993,7 +993,7 @@ static unsigned long __fget_light(unsigned int fd, fmode_t mask) struct file *file;
if (atomic_read(&files->count) == 1) { - file = __fcheck_files(files, fd); + file = files_lookup_fd_raw(files, fd); if (!file || unlikely(file->f_mode & mask)) return 0; return (unsigned long)file;
From: "Eric W. Biederman" ebiederm@xmission.com
stable inclusion from stable-v5.10.220 commit 9080557c56cd673941675f38805356f7f72949fa category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAGEF4 CVE: CVE-2024-41020
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
[ Upstream commit 120ce2b0cd52abe73e8b16c23461eb14df5a87d8 ]
To make it easy to tell where files->file_lock protection is being used when looking up a file create files_lookup_fd_locked. Only allow this function to be called with the file_lock held.
Update the callers of fcheck and fcheck_files that are called with the files->file_lock held to call files_lookup_fd_locked instead.
Hopefully this makes it easier to quickly understand what is going on.
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=kbRPqy... Link: https://lkml.kernel.org/r/20201120231441.29911-8-ebiederm@xmission.com Signed-off-by: Eric W. Biederman ebiederm@xmission.com Signed-off-by: Chuck Lever chuck.lever@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org Signed-off-by: Yifan Qiao qiaoyifan4@huawei.com --- include/linux/fdtable.h | 7 +++++++ fs/file.c | 2 +- fs/locks.c | 14 ++++++++------ fs/proc/fd.c | 2 +- 4 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h index 4b4410fc1282..e9f2884bc673 100644 --- a/include/linux/fdtable.h +++ b/include/linux/fdtable.h @@ -92,6 +92,13 @@ static inline struct file *files_lookup_fd_raw(struct files_struct *files, unsig return NULL; }
+static inline struct file *files_lookup_fd_locked(struct files_struct *files, unsigned int fd) +{ + RCU_LOCKDEP_WARN(!lockdep_is_held(&files->file_lock), + "suspicious rcu_dereference_check() usage"); + return files_lookup_fd_raw(files, fd); +} + static inline struct file *fcheck_files(struct files_struct *files, unsigned int fd) { RCU_LOCKDEP_WARN(!rcu_read_lock_held() && diff --git a/fs/file.c b/fs/file.c index a35374680ff2..50fb5964db4c 100644 --- a/fs/file.c +++ b/fs/file.c @@ -1214,7 +1214,7 @@ static int ksys_dup3(unsigned int oldfd, unsigned int newfd, int flags)
spin_lock(&files->file_lock); err = expand_files(files, newfd); - file = fcheck(oldfd); + file = files_lookup_fd_locked(files, oldfd); if (unlikely(!file)) goto Ebadf; if (unlikely(err < 0)) { diff --git a/fs/locks.c b/fs/locks.c index 6e118955b9b6..e9176510c0f9 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2536,14 +2536,15 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd, */ if (!error && file_lock->fl_type != F_UNLCK && !(file_lock->fl_flags & FL_OFDLCK)) { + struct files_struct *files = current->files; /* * We need that spin_lock here - it prevents reordering between * update of i_flctx->flc_posix and check for it done in * close(). rcu_read_lock() wouldn't do. */ - spin_lock(¤t->files->file_lock); - f = fcheck(fd); - spin_unlock(¤t->files->file_lock); + spin_lock(&files->file_lock); + f = files_lookup_fd_locked(files, fd); + spin_unlock(&files->file_lock); if (f != filp) { file_lock->fl_type = F_UNLCK; error = do_lock_file_wait(filp, cmd, file_lock); @@ -2667,14 +2668,15 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd, */ if (!error && file_lock->fl_type != F_UNLCK && !(file_lock->fl_flags & FL_OFDLCK)) { + struct files_struct *files = current->files; /* * We need that spin_lock here - it prevents reordering between * update of i_flctx->flc_posix and check for it done in * close(). rcu_read_lock() wouldn't do. */ - spin_lock(¤t->files->file_lock); - f = fcheck(fd); - spin_unlock(¤t->files->file_lock); + spin_lock(&files->file_lock); + f = files_lookup_fd_locked(files, fd); + spin_unlock(&files->file_lock); if (f != filp) { file_lock->fl_type = F_UNLCK; error = do_lock_file_wait(filp, cmd, file_lock); diff --git a/fs/proc/fd.c b/fs/proc/fd.c index 81882a13212d..6640ec2e5c48 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_locked(files, fd); if (file) { struct fdtable *fdt = files_fdtable(files);
From: Jann Horn jannh@google.com
stable inclusion from stable-v6.6.43 commit 73ae349534ebc377328e7d21891e589626c6e82c category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAGEF4 CVE: CVE-2024-41020
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=...
--------------------------------
commit f8138f2ad2f745b9a1c696a05b749eabe44337ea upstream.
When I wrote commit 3cad1bc01041 ("filelock: Remove locks reliably when fcntl/close race is detected"), I missed that there are two copies of the code I was patching: The normal version, and the version for 64-bit offsets on 32-bit kernels. Thanks to Greg KH for stumbling over this while doing the stable backport...
Apply exactly the same fix to the compat path for 32-bit kernels.
Fixes: c293621bbf67 ("[PATCH] stale POSIX lock handling") Cc: stable@kernel.org Link: https://bugs.chromium.org/p/project-zero/issues/detail?id=2563 Signed-off-by: Jann Horn jannh@google.com Link: https://lore.kernel.org/r/20240723-fs-lock-recover-compatfix-v1-1-1480967195... Signed-off-by: Christian Brauner brauner@kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Yifan Qiao qiaoyifan4@huawei.com --- fs/locks.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c index e9176510c0f9..8b9eb3e70d52 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2530,8 +2530,9 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd, error = do_lock_file_wait(filp, cmd, file_lock);
/* - * Attempt to detect a close/fcntl race and recover by releasing the - * lock that was just acquired. There is no need to do that when we're + * Detect close/fcntl races and recover by zapping all POSIX locks + * associated with this file and our files_struct, just like on + * filp_flush(). There is no need to do that when we're * unlocking though, or for OFD locks. */ if (!error && file_lock->fl_type != F_UNLCK && @@ -2546,9 +2547,7 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd, f = files_lookup_fd_locked(files, fd); spin_unlock(&files->file_lock); if (f != filp) { - file_lock->fl_type = F_UNLCK; - error = do_lock_file_wait(filp, cmd, file_lock); - WARN_ON_ONCE(error); + locks_remove_posix(filp, files); error = -EBADF; } }
反馈: 您发送到kernel@openeuler.org的补丁/补丁集,已成功转换为PR! PR链接地址: https://gitee.com/openeuler/kernel/pulls/10526 邮件列表地址:https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/G...
FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/10526 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/G...