[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5d6688d419f19512a8170ca915ec5825df8f489a.camel@kernel.org>
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>
Powered by blists - more mailing lists