[PATCH] filelock: Fix fcntl/close race recovery compat path

[PATCH] filelock: Fix fcntl/close race recovery compat path

[Jann Horn]

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>
---
 fs/locks.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index bdd94c32256f..9afb16e0683f 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2570,8 +2570,9 @@ int fcntl_setlk64(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->c.flc_type != F_UNLCK &&
@@ -2586,9 +2587,7 @@ int fcntl_setlk64(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->c.flc_type = F_UNLCK;
-			error = do_lock_file_wait(filp, cmd, file_lock);
-			WARN_ON_ONCE(error);
+			locks_remove_posix(filp, files);
 			error = -EBADF;
 		}
 	}

---
base-commit: 66ebbdfdeb093e097399b1883390079cd4c3022b
change-id: 20240723-fs-lock-recover-compatfix-cf2cbab343d1
-- 
Jann Horn <jannh@google.com>

Re: [PATCH] filelock: Fix fcntl/close race recovery compat path

[Christian Brauner]

On Tue, 23 Jul 2024 17:03:56 +0200, Jann Horn wrote:
> 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...
> 
> [...]

Thanks for fixing the compat path as well!

---

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] filelock: Fix fcntl/close race recovery compat path
      https://git.kernel.org/vfs/vfs/c/4bc443404754

Re: [PATCH] filelock: Fix fcntl/close race recovery compat path

[Jeff Layton]

On Tue, 2024-07-23 at 17:03 +0200, Jann Horn wrote:
> 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>
> ---
>  fs/locks.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index bdd94c32256f..9afb16e0683f 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -2570,8 +2570,9 @@ int fcntl_setlk64(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->c.flc_type != F_UNLCK &&
> @@ -2586,9 +2587,7 @@ int fcntl_setlk64(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->c.flc_type = F_UNLCK;
> -			error = do_lock_file_wait(filp, cmd, file_lock);
> -			WARN_ON_ONCE(error);
> +			locks_remove_posix(filp, files);
>  			error = -EBADF;
>  		}
>  	}
> 
> ---
> base-commit: 66ebbdfdeb093e097399b1883390079cd4c3022b
> change-id: 20240723-fs-lock-recover-compatfix-cf2cbab343d1

Doh! Good catch.

Reviewed-by: Jeff Layton <jlayton@kernel.org>