From: cel@kernel.org
To: Neil Brown <neilb@suse.de>, Mike Snitzer <snitzer@kernel.org>
Cc: <linux-nfs@vger.kernel.org>
Subject: [RFC PATCH 5/6] nfsd: factor out __fh_verify to allow NULL rqstp to be passed
Date: Tue, 27 Aug 2024 20:44:44 -0400 [thread overview]
Message-ID: <20240828004445.22634-6-cel@kernel.org> (raw)
In-Reply-To: <20240828004445.22634-1-cel@kernel.org>
From: NeilBrown <neilb@suse.de>
__fh_verify() offers an interface like fh_verify() but doesn't require
a struct svc_rqst *, instead it also takes the specific parts as
explicit required arguments. So it is safe to call __fh_verify() with
a NULL rqstp, but the net, cred, and client args must not be NULL.
__fh_verify() does not use SVC_NET(), nor does the functions it calls.
Rather then depending on rqstp->rq_vers to determine nfs version, pass
it in explicitly. This removes another dependency on rqstp and ensures
the correct version is checked. The rqstp can be for an NLM request and
while some code tests that, other code does not.
Rather than using rqstp->rq_client pass the client and gssclient
explicitly to __fh_verify and then to nfsd_set_fh_dentry().
Lastly, 4 associated tracepoints are only used if rqstp is not NULL
(this is a stop-gap that should be properly fixed so localio also
benefits from the utility these tracepoints provide when debugging
fh_verify issues).
Signed-off-by: NeilBrown <neilb@suse.de>
Co-developed-by: Mike Snitzer <snitzer@kernel.org>
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfsd/nfsfh.c | 168 ++++++++++++++++++++++++++----------------------
1 file changed, 92 insertions(+), 76 deletions(-)
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index 77acc26e8b02..80c06e170e9a 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -142,7 +142,11 @@ static inline __be32 check_pseudo_root(struct dentry *dentry,
* dentry. On success, the results are used to set fh_export and
* fh_dentry.
*/
-static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
+static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct net *net,
+ struct svc_cred *cred,
+ struct auth_domain *client,
+ struct auth_domain *gssclient,
+ struct svc_fh *fhp)
{
struct knfsd_fh *fh = &fhp->fh_handle;
struct fid *fid = NULL;
@@ -184,8 +188,8 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
data_left -= len;
if (data_left < 0)
return error;
- exp = rqst_exp_find(&rqstp->rq_chandle, SVC_NET(rqstp),
- rqstp->rq_client, rqstp->rq_gssclient,
+ exp = rqst_exp_find(rqstp ? &rqstp->rq_chandle : NULL,
+ net, client, gssclient,
fh->fh_fsid_type, fh->fh_fsid);
fid = (struct fid *)(fh->fh_fsid + len);
@@ -220,7 +224,7 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
put_cred(override_creds(new));
put_cred(new);
} else {
- error = nfsd_setuser_and_check_port(rqstp, &rqstp->rq_cred, exp);
+ error = nfsd_setuser_and_check_port(rqstp, cred, exp);
if (error)
goto out;
}
@@ -297,6 +301,87 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
return error;
}
+static __be32
+__fh_verify(struct svc_rqst *rqstp,
+ struct net *net, struct svc_cred *cred,
+ struct auth_domain *client,
+ struct auth_domain *gssclient,
+ struct svc_fh *fhp, umode_t type, int access)
+{
+ struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+ struct svc_export *exp = NULL;
+ struct dentry *dentry;
+ __be32 error;
+
+ if (!fhp->fh_dentry) {
+ error = nfsd_set_fh_dentry(rqstp, net, cred, client,
+ gssclient, fhp);
+ if (error)
+ goto out;
+ }
+ dentry = fhp->fh_dentry;
+ exp = fhp->fh_export;
+
+ trace_nfsd_fh_verify(rqstp, fhp, type, access);
+
+ /*
+ * We still have to do all these permission checks, even when
+ * fh_dentry is already set:
+ * - fh_verify may be called multiple times with different
+ * "access" arguments (e.g. nfsd_proc_create calls
+ * fh_verify(...,NFSD_MAY_EXEC) first, then later (in
+ * nfsd_create) calls fh_verify(...,NFSD_MAY_CREATE).
+ * - in the NFSv4 case, the filehandle may have been filled
+ * in by fh_compose, and given a dentry, but further
+ * compound operations performed with that filehandle
+ * still need permissions checks. In the worst case, a
+ * mountpoint crossing may have changed the export
+ * options, and we may now need to use a different uid
+ * (for example, if different id-squashing options are in
+ * effect on the new filesystem).
+ */
+ error = check_pseudo_root(dentry, exp);
+ if (error)
+ goto out;
+
+ error = nfsd_setuser_and_check_port(rqstp, cred, exp);
+ if (error)
+ goto out;
+
+ error = nfsd_mode_check(dentry, type);
+ if (error)
+ goto out;
+
+ /*
+ * pseudoflavor restrictions are not enforced on NLM,
+ * which clients virtually always use auth_sys for,
+ * even while using RPCSEC_GSS for NFS.
+ */
+ if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
+ goto skip_pseudoflavor_check;
+ /*
+ * Clients may expect to be able to use auth_sys during mount,
+ * even if they use gss for everything else; see section 2.3.2
+ * of rfc 2623.
+ */
+ if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT
+ && exp->ex_path.dentry == dentry)
+ goto skip_pseudoflavor_check;
+
+ error = check_nfsd_access(exp, rqstp);
+ if (error)
+ goto out;
+
+skip_pseudoflavor_check:
+ /* Finally, check access permissions. */
+ error = nfsd_permission(cred, exp, dentry, access);
+out:
+ trace_nfsd_fh_verify_err(rqstp, fhp, type, access, error);
+ if (error == nfserr_stale)
+ nfsd_stats_fh_stale_inc(nn, exp);
+ return error;
+}
+
/**
* fh_verify - filehandle lookup and access checking
* @rqstp: pointer to current rpc request
@@ -327,80 +412,11 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
__be32
fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
{
- struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
- struct svc_export *exp = NULL;
- struct dentry *dentry;
- __be32 error;
-
- if (!fhp->fh_dentry) {
- error = nfsd_set_fh_dentry(rqstp, fhp);
- if (error)
- goto out;
- }
- dentry = fhp->fh_dentry;
- exp = fhp->fh_export;
-
- trace_nfsd_fh_verify(rqstp, fhp, type, access);
-
- /*
- * We still have to do all these permission checks, even when
- * fh_dentry is already set:
- * - fh_verify may be called multiple times with different
- * "access" arguments (e.g. nfsd_proc_create calls
- * fh_verify(...,NFSD_MAY_EXEC) first, then later (in
- * nfsd_create) calls fh_verify(...,NFSD_MAY_CREATE).
- * - in the NFSv4 case, the filehandle may have been filled
- * in by fh_compose, and given a dentry, but further
- * compound operations performed with that filehandle
- * still need permissions checks. In the worst case, a
- * mountpoint crossing may have changed the export
- * options, and we may now need to use a different uid
- * (for example, if different id-squashing options are in
- * effect on the new filesystem).
- */
- error = check_pseudo_root(dentry, exp);
- if (error)
- goto out;
-
- error = nfsd_setuser_and_check_port(rqstp, &rqstp->rq_cred, exp);
- if (error)
- goto out;
-
- error = nfsd_mode_check(dentry, type);
- if (error)
- goto out;
-
- /*
- * pseudoflavor restrictions are not enforced on NLM,
- * which clients virtually always use auth_sys for,
- * even while using RPCSEC_GSS for NFS.
- */
- if (access & NFSD_MAY_LOCK || access & NFSD_MAY_BYPASS_GSS)
- goto skip_pseudoflavor_check;
- /*
- * Clients may expect to be able to use auth_sys during mount,
- * even if they use gss for everything else; see section 2.3.2
- * of rfc 2623.
- */
- if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT
- && exp->ex_path.dentry == dentry)
- goto skip_pseudoflavor_check;
-
- error = check_nfsd_access(exp, rqstp);
- if (error)
- goto out;
-
-skip_pseudoflavor_check:
- /* Finally, check access permissions. */
- error = nfsd_permission(&rqstp->rq_cred, exp, dentry, access);
-out:
- trace_nfsd_fh_verify_err(rqstp, fhp, type, access, error);
- if (error == nfserr_stale)
- nfsd_stats_fh_stale_inc(nn, exp);
- return error;
+ return __fh_verify(rqstp, SVC_NET(rqstp), &rqstp->rq_cred,
+ rqstp->rq_client, rqstp->rq_gssclient,
+ fhp, type, access);
}
-
/*
* Compose a file handle for an NFS reply.
*
--
2.45.2
next prev parent reply other threads:[~2024-08-28 0:44 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-28 0:44 [RFC PATCH 0/6] Split up refactoring of fh_verify() cel
2024-08-28 0:44 ` [RFC PATCH 1/6] NFSD: Handle @rqstp == NULL in check_nfsd_access() cel
2024-08-28 1:12 ` NeilBrown
2024-08-28 3:00 ` Mike Snitzer
2024-08-28 6:30 ` NeilBrown
2024-08-28 13:26 ` Chuck Lever III
2024-08-28 13:45 ` Mike Snitzer
2024-08-28 21:05 ` NeilBrown
2024-08-29 0:27 ` Mike Snitzer
2024-08-28 0:44 ` [RFC PATCH 2/6] NFSD: Refactor nfsd_setuser_and_check_port() cel
2024-08-28 0:44 ` [RFC PATCH 3/6] NFSD: Avoid using rqstp->rq_vers in nfsd_set_fh_dentry() cel
2024-08-28 5:02 ` NeilBrown
2024-08-28 13:45 ` Chuck Lever
2024-08-28 14:19 ` Mike Snitzer
2024-08-28 0:44 ` [RFC PATCH 4/6] NFSD: Short-circuit fh_verify tracepoints for LOCALIO cel
2024-08-28 0:44 ` cel [this message]
2024-08-28 0:44 ` [RFC PATCH 6/6] nfsd: add nfsd_file_acquire_local() cel
2024-08-28 1:08 ` [RFC PATCH 0/6] Split up refactoring of fh_verify() Mike Snitzer
2024-08-28 2:32 ` Mike Snitzer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240828004445.22634-6-cel@kernel.org \
--to=cel@kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neilb@suse.de \
--cc=snitzer@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.