linux-kernel - Re: [PATCH] v9fs: handle async processing of F_SETLK with FL

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]

Date:   Fri, 24 Dec 2021 08:14:05 +0900
From:   Dominique Martinet <asmadeus@...ewreck.org>
To:     Vasily Averin <vvs@...tuozzo.com>
Cc:     Eric Van Hensbergen <ericvh@...il.com>,
        Latchesar Ionkov <lucho@...kov.net>, kernel@...nvz.org,
        v9fs-developer@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] v9fs: handle async processing of F_SETLK with FL_SLEEP
 flag

Vasily Averin wrote on Thu, Dec 23, 2021 at 09:21:23PM +0300:
> kernel export thread (nfsd/lockd/ksmbd) uses F_SETLK cmd with the FL_SLEEP
> flag set to request asynchronous processing of blocking locks.
> 
> Currently v9fs does not support such requests and calls blocking
> locks_lock_file_wait() function.

There's two stages to 9p locks: the client first tries to get the lock
locally on the client, then if it was obtained locally also tries to get
it on the server.
I believe the servers should just ignores flags like FL_SLEEP they don't
know about, so we need to translate it as well if required.

> To work around the problem let's detect such request, drop FL_SLEEP
> before execution of potentially blocking functions.

I'm not up to date with lock mechanisms, could you confirm I understand
the flags right?
- F_SETLK: tries to lock, on conflict return immediately with error
- F_SETLKW|FL_SLEEP: tries to lock, on conflict wait for lock to become available
- F_SETLKW: not possible through flock/fcntl setlk, can happen otherwise?
but for 9p purpose same as above.
- F_SETLK|FL_SLEEP: tries to lock, on conflict ????? you'd want it to
return immediately but setup some callback to be woken up? how could
that work without passing some wake up struct? or just behave as plain
F_SETLK? but then FL_SLEEP has no purpose, I don't get it.


> 
> Dropped FL_SLEEP flag should be restored back because some calling
> function (nfsd4_lock) require it.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=215383
> Signed-off-by: Vasily Averin <vvs@...tuozzo.com>
> ---
>  fs/9p/vfs_file.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
> index 612e297f3763..81c98afdbb32 100644
> --- a/fs/9p/vfs_file.c
> +++ b/fs/9p/vfs_file.c
> @@ -135,6 +135,7 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl)
>  	int res = 0;
>  	unsigned char fl_type;
>  	struct v9fs_session_info *v9ses;
> +	bool async = false;
>  
>  	fid = filp->private_data;
>  	BUG_ON(fid == NULL);
> @@ -142,6 +143,10 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl)
>  	if ((fl->fl_flags & FL_POSIX) != FL_POSIX)
>  		BUG();
>  
> +	async = (fl->fl_flags & FL_SLEEP) && IS_SETLK(cmd);
> +	if (async)
> +		fl->fl_flags &= ~FL_SLEEP;
> +

So clearing the flag makes the local lock not wait at all in case of
SETLK|FL_SLEEP, and this errors instead.

I can't comment on this without understanding what's expected better

>  	res = locks_lock_file_wait(filp, fl);
>  	if (res < 0)
>  		goto out;
> @@ -230,6 +235,8 @@ static int v9fs_file_do_lock(struct file *filp, int cmd, struct file_lock *fl)
>  	if (flock.client_id != fid->clnt->name)
>  		kfree(flock.client_id);
>  out:
> +	if (async)
> +		fl->fl_flags |= FL_SLEEP;
>  	return res;
>  }
>  

-- 
Dominique