On Tue, Jul 23, 2024 at 2:56 PM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jul 22, 2024 at 04:22:50PM +0200, Jann Horn wrote:
commit 3cad1bc010416c6dd780643476bc59ed742436b9 upstream.
When fcntl_setlk() races with close(), it removes the created lock with do_lock_file_wait(). However, LSMs can allow the first do_lock_file_wait() that created the lock while denying the second do_lock_file_wait() that tries to remove the lock. In theory (but AFAIK not in practice), posix_lock_file() could also fail to remove a lock due to GFP_KERNEL allocation failure (when splitting a range in the middle).
After the bug has been triggered, use-after-free reads will occur in lock_get_status() when userspace reads /proc/locks. This can likely be used to read arbitrary kernel memory, but can't corrupt kernel memory. This only affects systems with SELinux / Smack / AppArmor / BPF-LSM in enforcing mode and only works from some security contexts.
Fix it by calling locks_remove_posix() instead, which is designed to reliably get rid of POSIX locks associated with the given file and files_struct and is also used by filp_flush().
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/20240702-fs-lock-recover-2-v1-1-edd456f63789@googl... Reviewed-by: Jeff Layton jlayton@kernel.org Signed-off-by: Christian Brauner brauner@kernel.org [stable fixup: ->c.flc_type was ->fl_type in older kernels] Signed-off-by: Jann Horn jannh@google.com
fs/locks.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c index fb717dae9029..31659a2d9862 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2381,8 +2381,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 &&@@ -2397,9 +2398,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);Wait, this breaks the build on 5.4.y with the error:
fs/locks.c: In function ‘fcntl_setlk’: fs/locks.c:2545:50: error: ‘files’ undeclared (first use in this function); did you mean ‘file’? 2545 | locks_remove_posix(filp, files); | ^~~~~ | file
I didn't do test-builds yesterday, my fault for not noticing this yet.
Ugh, sorry, I think maybe I only test-built on 6.6...
I've dropped this from the 5.4.y queues for now, can you fix this up and send an updated version, or give me a hint as to what to do instead?
Yeah, I'll have a look.
Odd that this works on 4.19.y, let me see why...