Skip to content

Commit

Permalink
filelock: Remove locks reliably when fcntl/close race is detected
Browse files Browse the repository at this point in the history
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: c293621 ("[PATCH] stale POSIX lock handling")
Cc: [email protected]
Link: https://bugs.chromium.org/p/project-zero/issues/detail?id=2563
Signed-off-by: Jann Horn <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: Christian Brauner <[email protected]>
[stable fixup: ->c.flc_type was ->fl_type in older kernels]
Signed-off-by: Jann Horn <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
(cherry picked from commit d30ff33040834c3b9eee29740acd92f9c7ba2250)
Signed-off-by: Harshit Mogalapalli <[email protected]>
Signed-off-by: KanonifyX <[email protected]>
  • Loading branch information
thejh authored and KanonifyX committed Feb 14, 2025
1 parent d76886d commit f5d7e04
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions fs/locks.c
Original file line number Diff line number Diff line change
Expand Up @@ -2284,8 +2284,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 &&
Expand All @@ -2299,9 +2300,7 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
f = fcheck(fd);
spin_unlock(&current->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, &current->files);
error = -EBADF;
}
}
Expand Down

0 comments on commit f5d7e04

Please sign in to comment.