| Message ID | 1438264341-18048-2-git-send-email-jeff.layton@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 30 Jul 2015 09:52:12 -0400 Jeff Layton <jlayton@poochiereds.net> wrote: > Currently, preprocess_stateid_op calls nfs4_check_olstateid which > verifies that the open stateid corresponds to the current_fh in the > call by calling nfs4_check_fh. > > If the stateid is a NFS4_DELEG_STID however, then no such check is > done. Move the call to nfs4_check_fh into nfs4_check_file instead > so that it can be done for all stateid types. > > Signed-off-by: Jeff Layton <jeff.layton@primarydata.com> > --- > fs/nfsd/nfs4state.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index cd8c33186e26..75f617a052cf 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -4406,9 +4406,9 @@ laundromat_main(struct work_struct *laundry) > queue_delayed_work(laundry_wq, &nn->laundromat_work, t*HZ); > } > > -static inline __be32 nfs4_check_fh(struct svc_fh *fhp, struct nfs4_ol_stateid *stp) > +static inline __be32 nfs4_check_fh(struct svc_fh *fhp, struct nfs4_stid *stp) > { > - if (!fh_match(&fhp->fh_handle, &stp->st_stid.sc_file->fi_fhandle)) > + if (!fh_match(&fhp->fh_handle, &stp->sc_file->fi_fhandle)) > return nfserr_bad_stateid; > return nfs_ok; > } > @@ -4611,9 +4611,6 @@ nfs4_check_olstateid(struct svc_fh *fhp, struct nfs4_ol_stateid *ols, int flags) > { > __be32 status; > > - status = nfs4_check_fh(fhp, ols); > - if (status) > - return status; > status = nfsd4_check_openowner_confirmed(ols); > if (status) > return status; > @@ -4628,6 +4625,10 @@ nfs4_check_file(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfs4_stid *s, > struct file *file; > __be32 status; > > + status = nfs4_check_fh(fhp, s); > + if (status) > + return status; > + > file = nfs4_find_file(s, flags); > if (file) { > status = nfsd_permission(rqstp, fhp->fh_export, fhp->fh_dentry, > @@ -4808,7 +4809,7 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_ > status = check_stateid_generation(stateid, &stp->st_stid.sc_stateid, nfsd4_has_session(cstate)); > if (status) > return status; > - return nfs4_check_fh(current_fh, stp); > + return nfs4_check_fh(current_fh, &stp->st_stid); > } > > /* Doh! Sorry -- I had already sent this separately but forgot to clean out the directory before running git-send-email. It's identical though to the one that I sent earlier today.
> return status; > @@ -4628,6 +4625,10 @@ nfs4_check_file(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfs4_stid *s, > struct file *file; > __be32 status; > > + status = nfs4_check_fh(fhp, s); > + if (status) > + return status; > + This means we check the file handle for all stateids now, not just open and lock stateids. That seems reasonable to me but should be mentioned in the changelog. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 30 Jul 2015 08:51:35 -0700 Christoph Hellwig <hch@infradead.org> wrote: > > return status; > > @@ -4628,6 +4625,10 @@ nfs4_check_file(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfs4_stid *s, > > struct file *file; > > __be32 status; > > > > + status = nfs4_check_fh(fhp, s); > > + if (status) > > + return status; > > + > > This means we check the file handle for all stateids now, not just > open and lock stateids. That seems reasonable to me but should be > mentioned in the changelog. This code is only called from nfs4_preprocess_stateid_op (which I typoed in the changelog -- maybe Bruce can fix that). Anything other than an open, lock or delegation stateid is explicitly rejected before this point. So, this just adds this check to delegation stateids (which is necessary I think). That is mentioned in the changelog though. Do you think it needs more elaboration or is that sufficient?
On Thu, Jul 30, 2015 at 12:20:48PM -0400, Jeff Layton wrote: > So, this just adds this check to delegation stateids (which is > necessary I think). That is mentioned in the changelog though. Do you > think it needs more elaboration or is that sufficient? No, I'm just thick today. The patch looks fine as-is! -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index cd8c33186e26..75f617a052cf 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -4406,9 +4406,9 @@ laundromat_main(struct work_struct *laundry) queue_delayed_work(laundry_wq, &nn->laundromat_work, t*HZ); } -static inline __be32 nfs4_check_fh(struct svc_fh *fhp, struct nfs4_ol_stateid *stp) +static inline __be32 nfs4_check_fh(struct svc_fh *fhp, struct nfs4_stid *stp) { - if (!fh_match(&fhp->fh_handle, &stp->st_stid.sc_file->fi_fhandle)) + if (!fh_match(&fhp->fh_handle, &stp->sc_file->fi_fhandle)) return nfserr_bad_stateid; return nfs_ok; } @@ -4611,9 +4611,6 @@ nfs4_check_olstateid(struct svc_fh *fhp, struct nfs4_ol_stateid *ols, int flags) { __be32 status; - status = nfs4_check_fh(fhp, ols); - if (status) - return status; status = nfsd4_check_openowner_confirmed(ols); if (status) return status; @@ -4628,6 +4625,10 @@ nfs4_check_file(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfs4_stid *s, struct file *file; __be32 status; + status = nfs4_check_fh(fhp, s); + if (status) + return status; + file = nfs4_find_file(s, flags); if (file) { status = nfsd_permission(rqstp, fhp->fh_export, fhp->fh_dentry, @@ -4808,7 +4809,7 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_ status = check_stateid_generation(stateid, &stp->st_stid.sc_stateid, nfsd4_has_session(cstate)); if (status) return status; - return nfs4_check_fh(current_fh, stp); + return nfs4_check_fh(current_fh, &stp->st_stid); } /*
Currently, preprocess_stateid_op calls nfs4_check_olstateid which verifies that the open stateid corresponds to the current_fh in the call by calling nfs4_check_fh. If the stateid is a NFS4_DELEG_STID however, then no such check is done. Move the call to nfs4_check_fh into nfs4_check_file instead so that it can be done for all stateid types. Signed-off-by: Jeff Layton <jeff.layton@primarydata.com> --- fs/nfsd/nfs4state.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)