linux-kernel - Re: [PATCH 1/3] fs/locks: F_UNLCK extension for F_OFD

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <c6d4e620cad72da5f85df03443a64747b5719939.camel@kernel.org>
Date:   Tue, 20 Jun 2023 06:46:50 -0400
From:   Jeff Layton <jlayton@...nel.org>
To:     Stas Sergeev <stsp2@...dex.ru>, linux-kernel@...r.kernel.org
Cc:     Chuck Lever <chuck.lever@...cle.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 1/3] fs/locks: F_UNLCK extension for F_OFD_GETLK

On Tue, 2023-06-20 at 14:55 +0500, Stas Sergeev wrote:
> Currently F_UNLCK with F_OFD_GETLK returns -EINVAL.
> The proposed extension allows to use it for getting the lock
> information from the particular fd.
> 
> Signed-off-by: Stas Sergeev <stsp2@...dex.ru>
> 
> CC: Jeff Layton <jlayton@...nel.org>
> CC: Chuck Lever <chuck.lever@...cle.com>
> CC: Alexander Viro <viro@...iv.linux.org.uk>
> CC: Christian Brauner <brauner@...nel.org>
> CC: linux-fsdevel@...r.kernel.org
> CC: linux-kernel@...r.kernel.org
> 
> ---
>  fs/locks.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index df8b26a42524..210766007e63 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -868,6 +868,21 @@ static bool posix_locks_conflict(struct file_lock *caller_fl,
>  	return locks_conflict(caller_fl, sys_fl);
>  }
>  
> +/* Determine if lock sys_fl blocks lock caller_fl. Used on xx_GETLK
> + * path so checks for additional GETLK-specific things like F_UNLCK.
> + */
> +static bool posix_test_locks_conflict(struct file_lock *caller_fl,
> +				      struct file_lock *sys_fl)
> +{
> +	/* F_UNLCK checks any locks on the same fd. */
> +	if (caller_fl->fl_type == F_UNLCK) {
> +		if (!posix_same_owner(caller_fl, sys_fl))
> +			return false;
> +		return locks_overlap(caller_fl, sys_fl);
> +	}
> +	return posix_locks_conflict(caller_fl, sys_fl);
> +}
> +
>  /* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
>   * checking before calling the locks_conflict().
>   */
> @@ -901,7 +916,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
>  retry:
>  	spin_lock(&ctx->flc_lock);
>  	list_for_each_entry(cfl, &ctx->flc_posix, fl_list) {
> -		if (!posix_locks_conflict(fl, cfl))
> +		if (!posix_test_locks_conflict(fl, cfl))
>  			continue;
>  		if (cfl->fl_lmops && cfl->fl_lmops->lm_lock_expirable
>  			&& (*cfl->fl_lmops->lm_lock_expirable)(cfl)) {
> @@ -2207,7 +2222,8 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
>  	if (fl == NULL)
>  		return -ENOMEM;
>  	error = -EINVAL;
> -	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
> +	if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK
> +			&& flock->l_type != F_WRLCK)
>  		goto out;
>  
>  	error = flock_to_posix_lock(filp, fl, flock);
> @@ -2414,7 +2430,8 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
>  		return -ENOMEM;
>  
>  	error = -EINVAL;
> -	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
> +	if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK
> +			&& flock->l_type != F_WRLCK)
>  		goto out;
>  
>  	error = flock64_to_posix_lock(filp, fl, flock);

This seems like a reasonable sort of interface to add, particularly for
the CRIU case. Using F_UNLCK for this is a bit kludgey, but adding a new
constant is probably worse.

I'm willing to take this in with an eye toward v6.6. Are you also
willing to draft up some manpage patches that detail this new interface?

-- 
Jeff Layton <jlayton@...nel.org>