| Message ID | 4E0EDEBE.8040902@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > > > -- 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
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 -- 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 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. -- 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 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) {
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(-)