linux-kernel - Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

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: <Zt3mK3fn0gWEsD9d@tissot.1015granger.net>
Date: Sun, 8 Sep 2024 14:00:11 -0400
From: Chuck Lever <chuck.lever@...cle.com>
To: Jeff Layton <jlayton@...nel.org>
Cc: Neil Brown <neilb@...e.de>, Olga Kornievskaia <okorniev@...hat.com>,
        Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey <tom@...pey.com>,
        Trond Myklebust <trondmy@...nel.org>, Anna Schumaker <anna@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
        Tom Haynes <loghyr@...il.com>, linux-nfs@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v4 01/11] nfsd: fix initial getattr on write delegation

On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote:
> At this point in compound processing, currentfh refers to the parent of
> the file, not the file itself. Get the correct dentry from the delegation
> stateid instead.
> 
> Signed-off-by: Jeff Layton <jlayton@...nel.org>
> ---
>  fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index df69dc6af467..db90677fc016 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
>  	}
>  }
>  
> +static bool
> +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> +		     struct kstat *stat)
> +{
> +	struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file);

The xfstests workflow on NFSv4.2 exhausts the capacity of both the
main and scratch devices (backed by xfs) about half-way through
each test run.

Deleting all visible files on both devices frees only a little bit
of space. The test exports can be unshared but not unmounted
(EBUSY). Looks like unlinked but still open files, maybe.

Bisected to this here patch.

Should there be a matching nfsd_file_put() book-end for the new
find_rw_file() call site?


> +	struct path path;
> +
> +	if (!nf)
> +		return false;
> +
> +	path.mnt = currentfh->fh_export->ex_path.mnt;
> +	path.dentry = file_dentry(nf->nf_file);
> +
> +	if (vfs_getattr(&path, stat,
> +			(STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> +			AT_STATX_SYNC_AS_STAT))
> +		return false;
> +	return true;
> +}
> +
>  /*
>   * The Linux NFS server does not offer write delegations to NFSv4.0
>   * clients in order to avoid conflicts between write delegations and
> @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  	int cb_up;
>  	int status = 0;
>  	struct kstat stat;
> -	struct path path;
>  
>  	cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
>  	open->op_recall = false;
> @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  	memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid));
>  
>  	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
> -		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> -		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> -		path.mnt = currentfh->fh_export->ex_path.mnt;
> -		path.dentry = currentfh->fh_dentry;
> -		if (vfs_getattr(&path, &stat,
> -				(STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> -				AT_STATX_SYNC_AS_STAT)) {
> +		if (!nfs4_delegation_stat(dp, currentfh, &stat)) {
>  			nfs4_put_stid(&dp->dl_stid);
>  			destroy_delegation(dp);
>  			goto out_no_deleg;
>  		}
> +		open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
>  		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
>  		dp->dl_cb_fattr.ncf_initial_cinfo =
>  			nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
> +		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
>  	} else {
>  		open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
>  		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> 
> -- 
> 2.46.0
> 

-- 
Chuck Lever