* [PATCH 2/2 v2] nfsd41: check the size of request @ 2011-07-02 9:02 Mi Jinlong 2011-07-06 16:43 ` J. Bruce Fields 0 siblings, 1 reply; 8+ messages in thread From: Mi Jinlong @ 2011-07-02 9:02 UTC (permalink / raw) To: J. Bruce Fields; +Cc: NFS This patch just check request's size when it consists SEQUENCE. Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> --- fs/nfsd/nfs4state.c | 16 ++++++++++++++++ 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index e98f3c2..17e30bf 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -1736,6 +1736,21 @@ static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_sess return args->opcnt > session->se_fchannel.maxops; } +static int nfsd4_check_request_size(struct nfsd4_compoundargs *args, + struct nfsd4_session *session) +{ + struct xdr_buf *xb = &args->rqstp->rq_arg; + + /* Only SEQUENCE operation */ + if (args->opcnt == 1) + return 0; + + if (xb->len > session->se_fchannel.maxreq_sz) + return nfserr_req_too_big; + + return 0; +} + __be32 nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, @@ -1804,6 +1819,7 @@ nfsd4_sequence(struct svc_rqst *rqstp, cstate->slot = slot; cstate->session = session; + status = nfsd4_check_request_size(rqstp->rq_argp, session); out: /* Hold a session reference until done processing the compound. */ if (cstate->session) { -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v2] nfsd41: check the size of request 2011-07-02 9:02 [PATCH 2/2 v2] nfsd41: check the size of request Mi Jinlong @ 2011-07-06 16:43 ` J. Bruce Fields 2011-07-08 9:13 ` Mi Jinlong 0 siblings, 1 reply; 8+ messages in thread From: J. Bruce Fields @ 2011-07-06 16:43 UTC (permalink / raw) To: Mi Jinlong; +Cc: NFS On Sat, Jul 02, 2011 at 05:02:54PM +0800, Mi Jinlong wrote: > This patch just check request's size when it consists SEQUENCE. > > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > --- > fs/nfsd/nfs4state.c | 16 ++++++++++++++++ > 1 files changed, 16 insertions(+), 0 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index e98f3c2..17e30bf 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -1736,6 +1736,21 @@ static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_sess > return args->opcnt > session->se_fchannel.maxops; > } > > +static int nfsd4_check_request_size(struct nfsd4_compoundargs *args, > + struct nfsd4_session *session) > +{ > + struct xdr_buf *xb = &args->rqstp->rq_arg; > + > + /* Only SEQUENCE operation */ > + if (args->opcnt == 1) > + return 0; Do we need this special check? Sure, it's possible that a crazy client could set se_fchannel.maxreq_sz too small, and then we'd get a failure here even when they only sent a single sequence. Such a client gets what it deserves. Seems OK otherwise. Of course, dealing with the maximum response size is going to be the difficult part. --b. > + > + if (xb->len > session->se_fchannel.maxreq_sz) > + return nfserr_req_too_big; > + > + return 0; > +} > + > __be32 > nfsd4_sequence(struct svc_rqst *rqstp, > struct nfsd4_compound_state *cstate, > @@ -1804,6 +1819,7 @@ nfsd4_sequence(struct svc_rqst *rqstp, > cstate->slot = slot; > cstate->session = session; > > + status = nfsd4_check_request_size(rqstp->rq_argp, session); > out: > /* Hold a session reference until done processing the compound. */ > if (cstate->session) { > -- > 1.7.5.4 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v2] nfsd41: check the size of request 2011-07-06 16:43 ` J. Bruce Fields @ 2011-07-08 9:13 ` Mi Jinlong 2011-07-08 20:03 ` J. Bruce Fields 0 siblings, 1 reply; 8+ messages in thread From: Mi Jinlong @ 2011-07-08 9:13 UTC (permalink / raw) To: J. Bruce Fields; +Cc: NFS J. Bruce Fields 写道: > On Sat, Jul 02, 2011 at 05:02:54PM +0800, Mi Jinlong wrote: >> This patch just check request's size when it consists SEQUENCE. >> >> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> >> --- >> fs/nfsd/nfs4state.c | 16 ++++++++++++++++ >> 1 files changed, 16 insertions(+), 0 deletions(-) >> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index e98f3c2..17e30bf 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -1736,6 +1736,21 @@ static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_sess >> return args->opcnt > session->se_fchannel.maxops; >> } >> >> +static int nfsd4_check_request_size(struct nfsd4_compoundargs *args, >> + struct nfsd4_session *session) >> +{ >> + struct xdr_buf *xb = &args->rqstp->rq_arg; >> + >> + /* Only SEQUENCE operation */ >> + if (args->opcnt == 1) >> + return 0; > > Do we need this special check? > > Sure, it's possible that a crazy client could set se_fchannel.maxreq_sz > too small, and then we'd get a failure here even when they only sent a > single sequence. Such a client gets what it deserves. If kernel err out when crazy client sets maxreq_sz to small, only SEQUENCE operation's size will never exceed maxreq_sz. According to RFC5661 18.36.3, if client set maxreq_sz too small, the server SHOULD return NFS4ERR_TOOSMALL in the CREATE_SESSION reply. thanks, Mi Jinlong ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v2] nfsd41: check the size of request 2011-07-08 9:13 ` Mi Jinlong @ 2011-07-08 20:03 ` J. Bruce Fields 2011-07-09 2:19 ` [PATCH 2/2 v3] " Mi Jinlong 0 siblings, 1 reply; 8+ messages in thread From: J. Bruce Fields @ 2011-07-08 20:03 UTC (permalink / raw) To: Mi Jinlong; +Cc: NFS On Fri, Jul 08, 2011 at 05:13:44PM +0800, Mi Jinlong wrote: > > > J. Bruce Fields 写道: > > On Sat, Jul 02, 2011 at 05:02:54PM +0800, Mi Jinlong wrote: > >> This patch just check request's size when it consists SEQUENCE. > >> > >> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > >> --- > >> fs/nfsd/nfs4state.c | 16 ++++++++++++++++ > >> 1 files changed, 16 insertions(+), 0 deletions(-) > >> > >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > >> index e98f3c2..17e30bf 100644 > >> --- a/fs/nfsd/nfs4state.c > >> +++ b/fs/nfsd/nfs4state.c > >> @@ -1736,6 +1736,21 @@ static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_sess > >> return args->opcnt > session->se_fchannel.maxops; > >> } > >> > >> +static int nfsd4_check_request_size(struct nfsd4_compoundargs *args, > >> + struct nfsd4_session *session) > >> +{ > >> + struct xdr_buf *xb = &args->rqstp->rq_arg; > >> + > >> + /* Only SEQUENCE operation */ > >> + if (args->opcnt == 1) > >> + return 0; > > > > Do we need this special check? > > > > Sure, it's possible that a crazy client could set se_fchannel.maxreq_sz > > too small, and then we'd get a failure here even when they only sent a > > single sequence. Such a client gets what it deserves. > > If kernel err out when crazy client sets maxreq_sz to small, > only SEQUENCE operation's size will never exceed maxreq_sz. > > According to RFC5661 18.36.3, if client set maxreq_sz too small, > the server SHOULD return NFS4ERR_TOOSMALL in the CREATE_SESSION reply. The rpc header size can vary. If you reject any CREATE_SESSION attempts with maxreq_sz smaller than largest possible rpc header + SEQUENCE then you risk rejecting some legitimate CREATE_SESSIONs. But if you allow maxreq_sz's less than that, then it becomes possible for a client to send a single-SEQUENCE compound that still exceeds maxreq_sz. So let's not try to safe every broken client from itself. Instead, let's keep things simple, catch those client errors that are easy to catch, and that we know are errors, and don't worry about the rest. --b. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2 v3] nfsd41: check the size of request 2011-07-08 20:03 ` J. Bruce Fields @ 2011-07-09 2:19 ` Mi Jinlong 2011-07-13 0:25 ` J. Bruce Fields 0 siblings, 1 reply; 8+ messages in thread From: Mi Jinlong @ 2011-07-09 2:19 UTC (permalink / raw) To: J. Bruce Fields; +Cc: NFS This patch checks request's size when it consists SEQUENCE. Also modifies the format of some function which length exceed 80. v3: remove the check about only SEQUENCE operation. check the size immediately after find session. modify some function's format which length exceed 80. Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> --- fs/nfsd/nfs4state.c | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index e98f3c2..9cce11c 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -1645,7 +1645,8 @@ __be32 nfsd4_bind_conn_to_session(struct svc_rqst *rqstp, return status; } -static bool nfsd4_compound_in_session(struct nfsd4_session *session, struct nfs4_sessionid *sid) +static bool nfsd4_compound_in_session(struct nfsd4_session *session, + struct nfs4_sessionid *sid) { if (!session) return 0; @@ -1695,7 +1696,8 @@ out: return status; } -static struct nfsd4_conn *__nfsd4_find_conn(struct svc_xprt *xpt, struct nfsd4_session *s) +static struct nfsd4_conn *__nfsd4_find_conn(struct svc_xprt *xpt, + struct nfsd4_session *s) { struct nfsd4_conn *c; @@ -1707,7 +1709,8 @@ static struct nfsd4_conn *__nfsd4_find_conn(struct svc_xprt *xpt, struct nfsd4_s return NULL; } -static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_session *ses) +static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, + struct nfsd4_session *ses) { struct nfs4_client *clp = ses->se_client; struct nfsd4_conn *c; @@ -1729,13 +1732,22 @@ 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) +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; } +static bool nfsd4_request_too_big(struct svc_rqst *rqstp, + struct nfsd4_session *session) +{ + struct xdr_buf *xb = &rqstp->rq_arg; + + return xb->len > session->se_fchannel.maxreq_sz; +} + __be32 nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, @@ -1768,6 +1780,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, if (nfsd4_session_too_many_ops(rqstp, session)) goto out; + status = nfserr_req_too_big; + if (nfsd4_request_too_big(rqstp, session)) + goto out; + status = nfserr_badslot; if (seq->slotid >= session->se_fchannel.maxreqs) goto out; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v3] nfsd41: check the size of request 2011-07-09 2:19 ` [PATCH 2/2 v3] " Mi Jinlong @ 2011-07-13 0:25 ` J. Bruce Fields 2011-07-14 6:56 ` [PATCH 2/2 v4] " Mi Jinlong 0 siblings, 1 reply; 8+ messages in thread From: J. Bruce Fields @ 2011-07-13 0:25 UTC (permalink / raw) To: Mi Jinlong; +Cc: NFS On Sat, Jul 09, 2011 at 10:19:56AM +0800, Mi Jinlong wrote: > This patch checks request's size when it consists SEQUENCE. > Also modifies the format of some function which length exceed 80. > > v3: > remove the check about only SEQUENCE operation. > check the size immediately after find session. > modify some function's format which length exceed 80. No, please don't ever mix unrelated formatting changes into a patch. We can leave those lines alone; the occasional long line isn't a big problem. --b. > > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > --- > fs/nfsd/nfs4state.c | 24 ++++++++++++++++++++---- > 1 files changed, 20 insertions(+), 4 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index e98f3c2..9cce11c 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -1645,7 +1645,8 @@ __be32 nfsd4_bind_conn_to_session(struct svc_rqst *rqstp, > return status; > } > > -static bool nfsd4_compound_in_session(struct nfsd4_session *session, struct nfs4_sessionid *sid) > +static bool nfsd4_compound_in_session(struct nfsd4_session *session, > + struct nfs4_sessionid *sid) > { > if (!session) > return 0; > @@ -1695,7 +1696,8 @@ out: > return status; > } > > -static struct nfsd4_conn *__nfsd4_find_conn(struct svc_xprt *xpt, struct nfsd4_session *s) > +static struct nfsd4_conn *__nfsd4_find_conn(struct svc_xprt *xpt, > + struct nfsd4_session *s) > { > struct nfsd4_conn *c; > > @@ -1707,7 +1709,8 @@ static struct nfsd4_conn *__nfsd4_find_conn(struct svc_xprt *xpt, struct nfsd4_s > return NULL; > } > > -static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_session *ses) > +static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, > + struct nfsd4_session *ses) > { > struct nfs4_client *clp = ses->se_client; > struct nfsd4_conn *c; > @@ -1729,13 +1732,22 @@ 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) > +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; > } > > +static bool nfsd4_request_too_big(struct svc_rqst *rqstp, > + struct nfsd4_session *session) > +{ > + struct xdr_buf *xb = &rqstp->rq_arg; > + > + return xb->len > session->se_fchannel.maxreq_sz; > +} > + > __be32 > nfsd4_sequence(struct svc_rqst *rqstp, > struct nfsd4_compound_state *cstate, > @@ -1768,6 +1780,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, > if (nfsd4_session_too_many_ops(rqstp, session)) > goto out; > > + status = nfserr_req_too_big; > + if (nfsd4_request_too_big(rqstp, session)) > + goto out; > + > status = nfserr_badslot; > if (seq->slotid >= session->se_fchannel.maxreqs) > goto out; > -- > 1.7.5.4 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2 v4] nfsd41: check the size of request 2011-07-13 0:25 ` J. Bruce Fields @ 2011-07-14 6:56 ` Mi Jinlong 2011-07-14 14:51 ` J. Bruce Fields 0 siblings, 1 reply; 8+ messages in thread From: Mi Jinlong @ 2011-07-14 6:56 UTC (permalink / raw) To: J. Bruce Fields; +Cc: NFS This patch checks request's size when it consists SEQUENCE. v4: delete format fixing which length exceed 80 v3: remove the check about only SEQUENCE operation. check the size immediately after find session. modify some function's format which length exceed 80. Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> --- fs/nfsd/nfs4state.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 72899ec..cc20587 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -1763,6 +1763,14 @@ static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_sess return args->opcnt > session->se_fchannel.maxops; } +static bool nfsd4_request_too_big(struct svc_rqst *rqstp, + struct nfsd4_session *session) +{ + struct xdr_buf *xb = &rqstp->rq_arg; + + return xb->len > session->se_fchannel.maxreq_sz; +} + __be32 nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, @@ -1795,6 +1803,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, if (nfsd4_session_too_many_ops(rqstp, session)) goto out; + status = nfserr_req_too_big; + if (nfsd4_request_too_big(rqstp, session)) + goto out; + status = nfserr_badslot; if (seq->slotid >= session->se_fchannel.maxreqs) goto out; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v4] nfsd41: check the size of request 2011-07-14 6:56 ` [PATCH 2/2 v4] " Mi Jinlong @ 2011-07-14 14:51 ` J. Bruce Fields 0 siblings, 0 replies; 8+ messages in thread From: J. Bruce Fields @ 2011-07-14 14:51 UTC (permalink / raw) To: Mi Jinlong; +Cc: NFS On Thu, Jul 14, 2011 at 02:56:02PM +0800, Mi Jinlong wrote: > This patch checks request's size when it consists SEQUENCE. > > v4: > delete format fixing which length exceed 80 > > v3: > remove the check about only SEQUENCE operation. > check the size immediately after find session. > modify some function's format which length exceed 80. Both applied, thanks! --b. > > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > --- > fs/nfsd/nfs4state.c | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index 72899ec..cc20587 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -1763,6 +1763,14 @@ static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_sess > return args->opcnt > session->se_fchannel.maxops; > } > > +static bool nfsd4_request_too_big(struct svc_rqst *rqstp, > + struct nfsd4_session *session) > +{ > + struct xdr_buf *xb = &rqstp->rq_arg; > + > + return xb->len > session->se_fchannel.maxreq_sz; > +} > + > __be32 > nfsd4_sequence(struct svc_rqst *rqstp, > struct nfsd4_compound_state *cstate, > @@ -1795,6 +1803,10 @@ nfsd4_sequence(struct svc_rqst *rqstp, > if (nfsd4_session_too_many_ops(rqstp, session)) > goto out; > > + status = nfserr_req_too_big; > + if (nfsd4_request_too_big(rqstp, session)) > + goto out; > + > status = nfserr_badslot; > if (seq->slotid >= session->se_fchannel.maxreqs) > goto out; > -- > 1.7.5.4 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-07-14 14:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-07-02 9:02 [PATCH 2/2 v2] nfsd41: check the size of request Mi Jinlong 2011-07-06 16:43 ` J. Bruce Fields 2011-07-08 9:13 ` Mi Jinlong 2011-07-08 20:03 ` J. Bruce Fields 2011-07-09 2:19 ` [PATCH 2/2 v3] " Mi Jinlong 2011-07-13 0:25 ` J. Bruce Fields 2011-07-14 6:56 ` [PATCH 2/2 v4] " Mi Jinlong 2011-07-14 14:51 ` J. Bruce Fields
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).