* [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence @ 2011-04-27 1:09 Mi Jinlong 2011-04-27 2:41 ` J. Bruce Fields 0 siblings, 1 reply; 5+ messages in thread From: Mi Jinlong @ 2011-04-27 1:09 UTC (permalink / raw) To: J. Bruce Fields; +Cc: NFS Make sure nfs server can distinguish request contains more ops than channel allowed. Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> --- fs/nfsd/nfs4state.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index fbde6f7..4f9fc68 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -1749,6 +1749,11 @@ nfsd4_sequence(struct svc_rqst *rqstp, if (!session) goto out; + status = nfserr_too_many_ops; + if (((struct nfsd4_compoundargs *)rqstp->rq_argp)->opcnt > + session->se_fchannel.maxops) + goto out; + status = nfserr_badslot; if (seq->slotid >= session->se_fchannel.maxreqs) goto out; -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence 2011-04-27 1:09 [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence Mi Jinlong @ 2011-04-27 2:41 ` J. Bruce Fields 2011-04-27 5:48 ` Mi Jinlong 0 siblings, 1 reply; 5+ messages in thread From: J. Bruce Fields @ 2011-04-27 2:41 UTC (permalink / raw) To: Mi Jinlong; +Cc: NFS On Wed, Apr 27, 2011 at 09:09:58AM +0800, Mi Jinlong wrote: > Make sure nfs server can distinguish request contains more ops > than channel allowed. Yes, sequence looks like a reasonable op to catch this error. The spec doesn't care as far as I can tell (and it's a buggy-client case, so why should it), and we already check that any compound not starting with a sequence has only one op. > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > --- > fs/nfsd/nfs4state.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index fbde6f7..4f9fc68 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -1749,6 +1749,11 @@ nfsd4_sequence(struct svc_rqst *rqstp, > if (!session) > goto out; > > + status = nfserr_too_many_ops; > + if (((struct nfsd4_compoundargs *)rqstp->rq_argp)->opcnt > Kind of a cumbersome construction, though. Eh, maybe overkill, but how about this?: --b. commit d5eee1629fb9d3a55e5793d156026248c14cb46c Author: Mi Jinlong <mijinlong@cn.fujitsu.com> Date: Wed Apr 27 09:09:58 2011 +0800 nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence Make sure nfs server errors out if request contains more ops than channel allows. Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> [bfields@redhat.com: use helper function] Signed-off-by: J. Bruce Fields <bfields@redhat.com> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index fbde6f7..487ba47 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -1721,6 +1721,13 @@ static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_sessi return; } +static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_session *session) +{ + struct nfsd4_compoundargs *args = rqstp->rq_argp; + + return args->opcnt > session->se_fchannel.maxops; +} + __be32 nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, @@ -1749,6 +1756,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, if (!session) goto out; + status = nfserr_too_many_ops; + if (nfsd4_session_too_many_ops(rqstp, session)) + goto out; + status = nfserr_badslot; if (seq->slotid >= session->se_fchannel.maxreqs) goto out; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence 2011-04-27 2:41 ` J. Bruce Fields @ 2011-04-27 5:48 ` Mi Jinlong 2011-05-03 3:48 ` J. Bruce Fields 0 siblings, 1 reply; 5+ messages in thread From: Mi Jinlong @ 2011-04-27 5:48 UTC (permalink / raw) To: J. Bruce Fields; +Cc: NFS J. Bruce Fields: > On Wed, Apr 27, 2011 at 09:09:58AM +0800, Mi Jinlong wrote: >> Make sure nfs server can distinguish request contains more ops >> than channel allowed. > > Yes, sequence looks like a reasonable op to catch this error. The spec > doesn't care as far as I can tell (and it's a buggy-client case, so why > should it), and we already check that any compound not starting with a > sequence has only one op. > >> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> >> --- >> fs/nfsd/nfs4state.c | 5 +++++ >> 1 files changed, 5 insertions(+), 0 deletions(-) >> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index fbde6f7..4f9fc68 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -1749,6 +1749,11 @@ nfsd4_sequence(struct svc_rqst *rqstp, >> if (!session) >> goto out; >> >> + status = nfserr_too_many_ops; >> + if (((struct nfsd4_compoundargs *)rqstp->rq_argp)->opcnt > > > Kind of a cumbersome construction, though. > > Eh, maybe overkill, but how about this?: That's great! -- ---- thanks Mi Jinlong > > --b. > > commit d5eee1629fb9d3a55e5793d156026248c14cb46c > Author: Mi Jinlong <mijinlong@cn.fujitsu.com> > Date: Wed Apr 27 09:09:58 2011 +0800 > > nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence > > Make sure nfs server errors out if request contains more ops > than channel allows. > > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > [bfields@redhat.com: use helper function] > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index fbde6f7..487ba47 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -1721,6 +1721,13 @@ static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_sessi > return; > } > > +static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_session *session) > +{ > + struct nfsd4_compoundargs *args = rqstp->rq_argp; > + > + return args->opcnt > session->se_fchannel.maxops; > +} > + > __be32 > nfsd4_sequence(struct svc_rqst *rqstp, > struct nfsd4_compound_state *cstate, > @@ -1749,6 +1756,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, > if (!session) > goto out; > > + status = nfserr_too_many_ops; > + if (nfsd4_session_too_many_ops(rqstp, session)) > + goto out; > + > status = nfserr_badslot; > if (seq->slotid >= session->se_fchannel.maxreqs) > goto out; > -- > 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 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence 2011-04-27 5:48 ` Mi Jinlong @ 2011-05-03 3:48 ` J. Bruce Fields 2011-05-09 9:26 ` Mi Jinlong 0 siblings, 1 reply; 5+ messages in thread From: J. Bruce Fields @ 2011-05-03 3:48 UTC (permalink / raw) To: Mi Jinlong; +Cc: NFS, iisaman On Wed, Apr 27, 2011 at 01:48:09PM +0800, Mi Jinlong wrote: > > > J. Bruce Fields: > > On Wed, Apr 27, 2011 at 09:09:58AM +0800, Mi Jinlong wrote: > >> Make sure nfs server can distinguish request contains more ops > >> than channel allowed. > > > > Yes, sequence looks like a reasonable op to catch this error. The spec > > doesn't care as far as I can tell (and it's a buggy-client case, so why > > should it), and we already check that any compound not starting with a > > sequence has only one op. > > > >> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > >> --- > >> fs/nfsd/nfs4state.c | 5 +++++ > >> 1 files changed, 5 insertions(+), 0 deletions(-) > >> > >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > >> index fbde6f7..4f9fc68 100644 > >> --- a/fs/nfsd/nfs4state.c > >> +++ b/fs/nfsd/nfs4state.c > >> @@ -1749,6 +1749,11 @@ nfsd4_sequence(struct svc_rqst *rqstp, > >> if (!session) > >> goto out; > >> > >> + status = nfserr_too_many_ops; > >> + if (((struct nfsd4_compoundargs *)rqstp->rq_argp)->opcnt > > > > > Kind of a cumbersome construction, though. > > > > Eh, maybe overkill, but how about this?: > > That's great! But note we're making a bunch of the pynfs tests fail now, since they aren't careful about how many ops they use in a compound. For example, LKPP1d fails for me now. Note it might not fail for you if you put the pynfs test directory right at the root of the export tree? I don't think pynfs hsould be using absolute paths for everything. And it should probably be doing single-component lookups in a loop instead of depending on being able to do the whole lookup in one compound, unless it's willing to adapt the compounds to take into account maxops--which sounds to me like more trouble than it's worth. The server could probably also raise its limit, though. --b. > > -- > ---- > thanks > Mi Jinlong > > > > > --b. > > > > commit d5eee1629fb9d3a55e5793d156026248c14cb46c > > Author: Mi Jinlong <mijinlong@cn.fujitsu.com> > > Date: Wed Apr 27 09:09:58 2011 +0800 > > > > nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence > > > > Make sure nfs server errors out if request contains more ops > > than channel allows. > > > > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > > [bfields@redhat.com: use helper function] > > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > index fbde6f7..487ba47 100644 > > --- a/fs/nfsd/nfs4state.c > > +++ b/fs/nfsd/nfs4state.c > > @@ -1721,6 +1721,13 @@ static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_sessi > > return; > > } > > > > +static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_session *session) > > +{ > > + struct nfsd4_compoundargs *args = rqstp->rq_argp; > > + > > + return args->opcnt > session->se_fchannel.maxops; > > +} > > + > > __be32 > > nfsd4_sequence(struct svc_rqst *rqstp, > > struct nfsd4_compound_state *cstate, > > @@ -1749,6 +1756,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, > > if (!session) > > goto out; > > > > + status = nfserr_too_many_ops; > > + if (nfsd4_session_too_many_ops(rqstp, session)) > > + goto out; > > + > > status = nfserr_badslot; > > if (seq->slotid >= session->se_fchannel.maxreqs) > > goto out; > > -- > > 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 > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence 2011-05-03 3:48 ` J. Bruce Fields @ 2011-05-09 9:26 ` Mi Jinlong 0 siblings, 0 replies; 5+ messages in thread From: Mi Jinlong @ 2011-05-09 9:26 UTC (permalink / raw) To: J. Bruce Fields; +Cc: NFS, iisaman J. Bruce Fields: > On Wed, Apr 27, 2011 at 01:48:09PM +0800, Mi Jinlong wrote: >> >> J. Bruce Fields: >>> On Wed, Apr 27, 2011 at 09:09:58AM +0800, Mi Jinlong wrote: >>>> Make sure nfs server can distinguish request contains more ops >>>> than channel allowed. >>> Yes, sequence looks like a reasonable op to catch this error. The spec >>> doesn't care as far as I can tell (and it's a buggy-client case, so why >>> should it), and we already check that any compound not starting with a >>> sequence has only one op. >>> >>>> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> >>>> --- >>>> fs/nfsd/nfs4state.c | 5 +++++ >>>> 1 files changed, 5 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >>>> index fbde6f7..4f9fc68 100644 >>>> --- a/fs/nfsd/nfs4state.c >>>> +++ b/fs/nfsd/nfs4state.c >>>> @@ -1749,6 +1749,11 @@ nfsd4_sequence(struct svc_rqst *rqstp, >>>> if (!session) >>>> goto out; >>>> >>>> + status = nfserr_too_many_ops; >>>> + if (((struct nfsd4_compoundargs *)rqstp->rq_argp)->opcnt > >>> Kind of a cumbersome construction, though. >>> >>> Eh, maybe overkill, but how about this?: >> That's great! > > But note we're making a bunch of the pynfs tests fail now, since they > aren't careful about how many ops they use in a compound. For example, > LKPP1d fails for me now. Note it might not fail for you if you put the > pynfs test directory right at the root of the export tree? Yes, I don't get LKPP1d fail. > I don't think pynfs hsould be using absolute paths for everything. And > it should probably be doing single-component lookups in a loop instead > of depending on being able to do the whole lookup in one compound, > unless it's willing to adapt the compounds to take into account > maxops--which sounds to me like more trouble than it's worth. I agree the second idea although it's more trouble. Because simulating a NFS4.1 client, we should make sure pynfs41 can make request with ops which less than maxops. > > The server could probably also raise its limit, though. I think, we can do it as raising the maxops at CREATE_SESSION ops when create session, I don't test it. -- ---- thanks Mi Jinlong >>> commit d5eee1629fb9d3a55e5793d156026248c14cb46c >>> Author: Mi Jinlong <mijinlong@cn.fujitsu.com> >>> Date: Wed Apr 27 09:09:58 2011 +0800 >>> >>> nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence >>> >>> Make sure nfs server errors out if request contains more ops >>> than channel allows. >>> >>> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> >>> [bfields@redhat.com: use helper function] >>> Signed-off-by: J. Bruce Fields <bfields@redhat.com> >>> >>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >>> index fbde6f7..487ba47 100644 >>> --- a/fs/nfsd/nfs4state.c >>> +++ b/fs/nfsd/nfs4state.c >>> @@ -1721,6 +1721,13 @@ static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_sessi >>> return; >>> } >>> >>> +static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_session *session) >>> +{ >>> + struct nfsd4_compoundargs *args = rqstp->rq_argp; >>> + >>> + return args->opcnt > session->se_fchannel.maxops; >>> +} >>> + >>> __be32 >>> nfsd4_sequence(struct svc_rqst *rqstp, >>> struct nfsd4_compound_state *cstate, >>> @@ -1749,6 +1756,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, >>> if (!session) >>> goto out; >>> >>> + status = nfserr_too_many_ops; >>> + if (nfsd4_session_too_many_ops(rqstp, session)) >>> + goto out; >>> + >>> status = nfserr_badslot; >>> if (seq->slotid >= session->se_fchannel.maxreqs) >>> goto out; >>> -- >>> 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 >>> >>> > -- > 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 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-05-09 9:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-04-27 1:09 [PATCH] nfsd41: compare request's opcnt with session's maxops at nfsd4_sequence Mi Jinlong 2011-04-27 2:41 ` J. Bruce Fields 2011-04-27 5:48 ` Mi Jinlong 2011-05-03 3:48 ` J. Bruce Fields 2011-05-09 9:26 ` Mi Jinlong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).