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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0616b6be151e2063627fb7f22100b9d3407aceb3.camel@kernel.org>
Date: Tue, 23 Jul 2024 11:50:37 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Jann Horn <jannh@...gle.com>, Alexander Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner
	 <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Chuck Lever
	 <chuck.lever@...cle.com>, Alexander Aring <alex.aring@...il.com>
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	stable@...nel.org
Subject: Re: [PATCH] filelock: Fix fcntl/close race recovery compat path

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@...nel.org
> Link: https://bugs.chromium.org/p/project-zero/issues/detail?id=2563
> Signed-off-by: Jann Horn <jannh@...gle.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@...nel.org>