linux-kernel - Re: [PATCH RFC 11/24] nfsd: allow DELEGRETURN on directories

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: Mon, 18 Mar 2024 07:22:04 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Trond Myklebust <trondmy@...merspace.com>, "chuck.lever@...cle.com"
	 <chuck.lever@...cle.com>
Cc: "senozhatsky@...omium.org" <senozhatsky@...omium.org>, 
 "sfrench@...ba.org" <sfrench@...ba.org>, "ecryptfs@...r.kernel.org"
 <ecryptfs@...r.kernel.org>,  "linux-unionfs@...r.kernel.org"
 <linux-unionfs@...r.kernel.org>, "davem@...emloft.net"
 <davem@...emloft.net>,  "viro@...iv.linux.org.uk"
 <viro@...iv.linux.org.uk>, "anna@...nel.org" <anna@...nel.org>,
 "jack@...e.cz" <jack@...e.cz>, "tom@...pey.com" <tom@...pey.com>,
 "pabeni@...hat.com" <pabeni@...hat.com>, "netdev@...r.kernel.org"
 <netdev@...r.kernel.org>,  "linux-fsdevel@...r.kernel.org"
 <linux-fsdevel@...r.kernel.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "ronniesahlberg@...il.com"
 <ronniesahlberg@...il.com>, "samba-technical@...ts.samba.org"
 <samba-technical@...ts.samba.org>, "dhowells@...hat.com"
 <dhowells@...hat.com>,  "Dai.Ngo@...cle.com" <Dai.Ngo@...cle.com>,
 "kuba@...nel.org" <kuba@...nel.org>, "rafael@...nel.org"
 <rafael@...nel.org>, "alex.aring@...il.com" <alex.aring@...il.com>, 
 "pc@...guebit.com" <pc@...guebit.com>, "amir73il@...il.com"
 <amir73il@...il.com>,  "kolga@...app.com" <kolga@...app.com>,
 "sprasad@...rosoft.com" <sprasad@...rosoft.com>,  "code@...icks.com"
 <code@...icks.com>, "brauner@...nel.org" <brauner@...nel.org>, 
 "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
 "edumazet@...gle.com" <edumazet@...gle.com>,  "linux-cifs@...r.kernel.org"
 <linux-cifs@...r.kernel.org>, "linkinjeon@...nel.org"
 <linkinjeon@...nel.org>,  "neilb@...e.de" <neilb@...e.de>,
 "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>, 
 "netfs@...ts.linux.dev" <netfs@...ts.linux.dev>, "miklos@...redi.hu"
 <miklos@...redi.hu>
Subject: Re: [PATCH RFC 11/24] nfsd: allow DELEGRETURN on directories

On Sun, 2024-03-17 at 16:03 +0000, Trond Myklebust wrote:
> On Sun, 2024-03-17 at 11:09 -0400, Chuck Lever wrote:
> > On Fri, Mar 15, 2024 at 12:53:02PM -0400, Jeff Layton wrote:
> > > fh_verify only allows you to filter on a single type of inode, so
> > > have
> > > nfsd4_delegreturn not filter by type. Once fh_verify returns, do
> > > the
> > > appropriate check of the type and return an error if it's not a
> > > regular
> > > file or directory.
> > > 
> > > Signed-off-by: Jeff Layton <jlayton@...nel.org>
> > > ---
> > >  fs/nfsd/nfs4state.c | 14 +++++++++++++-
> > >  1 file changed, 13 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > index 17d09d72632b..c52e807f9672 100644
> > > --- a/fs/nfsd/nfs4state.c
> > > +++ b/fs/nfsd/nfs4state.c
> > > @@ -7425,12 +7425,24 @@ nfsd4_delegreturn(struct svc_rqst *rqstp,
> > > struct nfsd4_compound_state *cstate,
> > >  	struct nfs4_delegation *dp;
> > >  	stateid_t *stateid = &dr->dr_stateid;
> > >  	struct nfs4_stid *s;
> > > +	umode_t mode;
> > >  	__be32 status;
> > >  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp),
> > > nfsd_net_id);
> > >  
> > > -	if ((status = fh_verify(rqstp, &cstate->current_fh,
> > > S_IFREG, 0)))
> > > +	if ((status = fh_verify(rqstp, &cstate->current_fh, 0,
> > > 0)))
> > >  		return status;
> > >  
> > > +	mode = d_inode(cstate->current_fh.fh_dentry)->i_mode &
> > > S_IFMT;
> > > +	switch(mode) {
> > > +	case S_IFREG:
> > > +	case S_IFDIR:
> > > +		break;
> > > +	case S_IFLNK:
> > > +		return nfserr_symlink;
> > > +	default:
> > > +		return nfserr_inval;
> > > +	}
> > > +
> > 
> > RFC 8881 Section 15.2 does not list NFS4ERR_SYMLINK among the
> > valid status codes for the DELEGRETURN operation. Maybe the naked
> > fh_verify() call has gotten it wrong all these years...?
> 
> The WANT_DELEGATION operation allows the server to hand out delegations
> for aggressive caching of symlinks. It is not an error to return that
> delegation using DELEGRETURN.
> 
> Furthermore, provided that the presented stateid is actually valid, it
> is also sufficient to uniquely identify the file to which it is
> associated (see RFC8881 Section 8.2.4), so the filehandle should be
> considered mostly irrelevant for operations like DELEGRETURN.
> 

Ok. I think we can probably just drop the switch altogether. We already
don't validate that the stateid is associated with current_fh, AFAICT.
It looks possible to send a valid stateid alongside an FH that refers to
a completely different file, and we'll just accept it.

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