Skip to contentSkip to conversation

nfsd41: check the size of request

PATCH Superseded Mi Jinlong 2 participants 8 messages Jul 2, 2011 – Jul 14, 2011

Subsystems (2)
NFS client NFS server

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Mi Jinlong

author
#0 lore raw

PATCH v2 2/2 nfsd41: check the size of request

This patch just check request's size when it consists SEQUENCE.

Signed-off-by Mi Jinlong <[email protected]>
---
 fs/nfsd/nfs4state.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
fs/nfsd/nfs4state.c +16 -0
index e98f3c2..17e30bf 100644
@@ -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) {
Signature · 1 line
1.7.5.4
#1 lore raw
Quoted 13 lines · Mi Jinlong

This patch just check request's size when it consists SEQUENCE.

Signed-off-by Mi Jinlong <[email protected]>
---
 fs/nfsd/nfs4state.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
fs/nfsd/nfs4state.c +0 -0
index e98f3c2..17e30bf 100644
@@ -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;

fs/nfsd/nfs4state.c static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_sess · line 1746

Quoted 10 lines
fs/nfsd/nfs4state.c +8 -0
 }
 
+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.

Quoted 23 lines
fs/nfsd/nfs4state.c +8 -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) {
Signature · 1 line
1.7.5.4
author
#2 lore raw
Quoted 27 lines · J. Bruce Fields
Quoted 13 lines · Mi Jinlong

This patch just check request's size when it consists SEQUENCE.

Signed-off-by Mi Jinlong <[email protected]>
---
 fs/nfsd/nfs4state.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
fs/nfsd/nfs4state.c +0 -0
index e98f3c2..17e30bf 100644
@@ -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;

fs/nfsd/nfs4state.c static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_sess · line 1746

Quoted 10 lines
fs/nfsd/nfs4state.c +8 -0
 }
 
+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?

Quoted 3 lines

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

#3 lore raw
Quoted 34 lines · Mi Jinlong
Quoted 27 lines · J. Bruce Fields
Quoted 13 lines · Mi Jinlong

This patch just check request's size when it consists SEQUENCE.

Signed-off-by Mi Jinlong <[email protected]>
---
 fs/nfsd/nfs4state.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
fs/nfsd/nfs4state.c +0 -0
index e98f3c2..17e30bf 100644
@@ -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;

fs/nfsd/nfs4state.c static bool nfsd4_session_too_many_ops(struct svc_rqst *rqstp, struct nfsd4_sess · line 1746

Quoted 10 lines
fs/nfsd/nfs4state.c +8 -0
 }
 
+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?

Quoted 3 lines

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.

Quoted 5 lines

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.

author
#4 lore raw

PATCH v3 2/2 nfsd41: check the size of request

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 <[email protected]>
---
 fs/nfsd/nfs4state.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)
fs/nfsd/nfs4state.c +20 -4
index e98f3c2..9cce11c 100644
@@ -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;
Signature · 1 line
1.7.5.4
#5 lore raw
Quoted 7 lines · Mi Jinlong

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.

Quoted 80 lines
Signed-off-by Mi Jinlong <[email protected]>
---
 fs/nfsd/nfs4state.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)
fs/nfsd/nfs4state.c +20 -4
index e98f3c2..9cce11c 100644
@@ -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;
Signature · 1 line
1.7.5.4
author
#6 lore raw

PATCH v4 2/2 nfsd41: check the size of request

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 <[email protected]>
---
 fs/nfsd/nfs4state.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
fs/nfsd/nfs4state.c +12 -0
index 72899ec..cc20587 100644
@@ -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;
Signature · 1 line
1.7.5.4
#7 lore raw
Quoted 9 lines · Mi Jinlong

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.

Quoted 41 lines
Signed-off-by Mi Jinlong <[email protected]>
---
 fs/nfsd/nfs4state.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
fs/nfsd/nfs4state.c +12 -0
index 72899ec..cc20587 100644
@@ -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;
Signature · 1 line
1.7.5.4