linux-kernel - Re: [PATCH v3 01/13] nfsd: fix nfsd4_deleg_getattr_conflict in presence of third party lease

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: <ZtCRGfPRayPPDXRM@tissot.1015granger.net>
Date: Thu, 29 Aug 2024 11:17:45 -0400
From: Chuck Lever <chuck.lever@...cle.com>
To: Jeff Layton <jlayton@...nel.org>
Cc: Neil Brown <neilb@...e.de>, Olga Kornievskaia <kolga@...app.com>,
        Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey <tom@...pey.com>,
        Trond Myklebust <trondmy@...nel.org>, Anna Schumaker <anna@...nel.org>,
        Olga Kornievskaia <okorniev@...hat.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
        Jonathan Corbet <corbet@....net>, Tom Haynes <loghyr@...il.com>,
        linux-kernel@...r.kernel.org, linux-nfs@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v3 01/13] nfsd: fix nfsd4_deleg_getattr_conflict in
 presence of third party lease

On Thu, Aug 29, 2024 at 09:26:39AM -0400, Jeff Layton wrote:
> From: NeilBrown <neilb@...e.de>
> 
> It is not safe to dereference fl->c.flc_owner without first confirming
> fl->fl_lmops is the expected manager.  nfsd4_deleg_getattr_conflict()
> tests fl_lmops but largely ignores the result and assumes that flc_owner
> is an nfs4_delegation anyway.  This is wrong.
> 
> With this patch we restore the "!= &nfsd_lease_mng_ops" case to behave
> as it did before the changed mentioned below.  This the same as the
> current code, but without any reference to a possible delegation.
> 
> Fixes: c5967721e106 ("NFSD: handle GETATTR conflict with write delegation")
> Signed-off-by: NeilBrown <neilb@...e.de>
> Signed-off-by: Jeff Layton <jlayton@...nel.org>

I've already applied this to nfsd-fixes.

If I include this commit in both nfsd-fixes and nfsd-next then the
linux-next merge whines about duplicate patches. Stephen Rothwell
suggested git-merging nfsd-fixes and nfsd-next but I'm not quite
confident enough to try that.

Barring another solution, merging this series will have to wait a
few days before the two trees can sync up.


> ---
>  fs/nfsd/nfs4state.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index b6bf39c64d78..eaa11d42d1b1 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -8854,7 +8854,15 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
>  			 */
>  			if (type == F_RDLCK)
>  				break;
> -			goto break_lease;
> +
> +			nfsd_stats_wdeleg_getattr_inc(nn);
> +			spin_unlock(&ctx->flc_lock);
> +
> +			status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ));
> +			if (status != nfserr_jukebox ||
> +			    !nfsd_wait_for_delegreturn(rqstp, inode))
> +				return status;
> +			return 0;
>  		}
>  		if (type == F_WRLCK) {
>  			struct nfs4_delegation *dp = fl->c.flc_owner;
> @@ -8863,7 +8871,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
>  				spin_unlock(&ctx->flc_lock);
>  				return 0;
>  			}
> -break_lease:
>  			nfsd_stats_wdeleg_getattr_inc(nn);
>  			dp = fl->c.flc_owner;
>  			refcount_inc(&dp->dl_stid.sc_count);
> 
> -- 
> 2.46.0
> 

-- 
Chuck Lever