nfsd41: error out when client sets maxreq_sz or, maxresp_sz too small

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

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Mi Jinlong

author

2011-07-02 08:58 UTC #0 lore raw

PATCH v2 1/2 nfsd41: error out when client sets maxreq_sz or, maxresp_sz too small

According to RFC5661, 18.36.3,

"if the client selects a value for ca_maxresponsesize such that
a replier on a channel could never send a response,the server SHOULD return NFS4ERR_TOOSMALL in the CREATE_SESSION reply."

This patch let server error out when client sets maxreq_sz less than SEQUENCE request size, and maxresp_sz less than a SEQUENCE reply size.

Signed-off-by Mi Jinlong <[email protected]>

---
 fs/nfsd/nfs4xdr.c |   26 +++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 1 deletions(-)

fs/nfsd/nfs4xdr.c +25 -1

index 9901811..bece272 100644
@@ -1135,11 +1135,25 @@ nfsd4_decode_create_session(struct nfsd4_compoundargs *argp,
 {
 	DECODE_HEAD;
 
-	u32 dummy;
+	struct xdr_buf *xb = &argp->rqstp->rq_arg;
+	u32 dummy, minreqlen = 0, minresplen = 0;
 	char *machine_name;
 	int i;
 	int nr_secflavs;
 
+	/* RPC header length and tag, minorversion, Opt count*/
+	minreqlen = (char *)argp->p - ((char *)argp->end - xb->len);
+	/* length with a SEQUENCE operation */
+	minreqlen = minreqlen + sizeof(struct nfs4_sessionid)
+		    + 4 * sizeof(__be32);
+
+	/* RPC header, status, tag len */
+	minresplen = argp->rqstp->rq_res.head[0].iov_len + 2 * sizeof(__be32)
+		     /* tag, opt count, opcode, op status */
+		     + ALIGN(argp->taglen, 4) + 3 * sizeof(__be32)
+		     /* sessionid, seqid, 3 * slotid, status*/
+		     + sizeof(struct nfs4_sessionid) + 5 * sizeof(__be32);
+
 	READ_BUF(16);
 	COPYMEM(&sess->clientid, 8);
 	READ32(sess->seqid);

@@ -1149,7 +1163,17 @@ nfsd4_decode_create_session(struct nfsd4_compoundargs *argp,
 	READ_BUF(28);
 	READ32(dummy); /* headerpadsz is always 0 */
 	READ32(sess->fore_channel.maxreq_sz);
+	if (sess->fore_channel.maxreq_sz < minreqlen) {
+		status = nfserr_toosmall;
+		goto out;
+	}
+
 	READ32(sess->fore_channel.maxresp_sz);
+	if (sess->fore_channel.maxresp_sz < minresplen) {
+		status = nfserr_toosmall;
+		goto out;
+	}
+
 	READ32(sess->fore_channel.maxresp_cached);
 	READ32(sess->fore_channel.maxops);
 	READ32(sess->fore_channel.maxreqs);

Signature · 1 line

1.7.5.4

J. Bruce Fields

2011-07-06 16:26 UTC #1 lore raw

Quoted 32 lines · Mi Jinlong

According to RFC5661, 18.36.3,
"if the client selects a value for ca_maxresponsesize such that
a replier on a channel could never send a response,the server SHOULD return NFS4ERR_TOOSMALL in the CREATE_SESSION reply."
This patch let server error out when client sets maxreq_sz less than SEQUENCE request size, and maxresp_sz less than a SEQUENCE reply size.
Signed-off-by Mi Jinlong <[email protected]>
---
 fs/nfsd/nfs4xdr.c |   26 +++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 1 deletions(-)
fs/nfsd/nfs4xdr.c +5 -1
index 9901811..bece272 100644
@@ -1135,11 +1135,25 @@ nfsd4_decode_create_session(struct nfsd4_compoundargs *argp,
 {
 	DECODE_HEAD;
 
-	u32 dummy;
+	struct xdr_buf *xb = &argp->rqstp->rq_arg;
+	u32 dummy, minreqlen = 0, minresplen = 0;
 	char *machine_name;
 	int i;
 	int nr_secflavs;
 
+	/* RPC header length and tag, minorversion, Opt count*/
+	minreqlen = (char *)argp->p - ((char *)argp->end - xb->len);
+	/* length with a SEQUENCE operation */

fs/nfsd/nfs4xdr.c nfsd4_decode_create_session(struct nfsd4_compoundargs *argp, · line 1156

Quoted 10 lines

fs/nfsd/nfs4xdr.c +10 -0

+	minreqlen = minreqlen + sizeof(struct nfs4_sessionid)
+		    + 4 * sizeof(__be32);
+
+	/* RPC header, status, tag len */
+	minresplen = argp->rqstp->rq_res.head[0].iov_len + 2 * sizeof(__be32)
+		     /* tag, opt count, opcode, op status */
+		     + ALIGN(argp->taglen, 4) + 3 * sizeof(__be32)
+		     /* sessionid, seqid, 3 * slotid, status*/
+		     + sizeof(struct nfs4_sessionid) + 5 * sizeof(__be32);
+

Could you just calculate this as a constant, assuming the smallest possible credential and verifier? NFSD_MIN_HDR_SEQ_SZ may be what you need for the reply.

Calculating it dynamically here using the cred and verifier size from the create_session request might be more accurate--or it might not, as there's no guarantee the same size cred adn verifier will be used for later rpc's.

But that's OK, our goal here isn't to get this 100% correct, as that's not possible, it's just to add a sanity check that may help catch a crazy client.

fs/nfsd/nfs4xdr.c nfsd4_decode_create_session(struct nfsd4_compoundargs *argp, · line 1167

Quoted 9 lines

fs/nfsd/nfs4xdr.c +2 -0

 	READ_BUF(16);
 	COPYMEM(&sess->clientid, 8);
 	READ32(sess->seqid);

@@ -1149,7 +1163,17 @@ nfsd4_decode_create_session(struct nfsd4_compoundargs *argp,
 	READ_BUF(28);
 	READ32(dummy); /* headerpadsz is always 0 */
 	READ32(sess->fore_channel.maxreq_sz);
+	if (sess->fore_channel.maxreq_sz < minreqlen) {
+		status = nfserr_toosmall;

Have you checked whether that error actually gets back to the client?

I think it gets turned into a bad_xdr error at some point.

We probably want this in nfsd4_create_session instead.

--b.

Quoted 17 lines

fs/nfsd/nfs4xdr.c +8 -0

+		goto out;
+	}
+
 	READ32(sess->fore_channel.maxresp_sz);
+	if (sess->fore_channel.maxresp_sz < minresplen) {
+		status = nfserr_toosmall;
+		goto out;
+	}
+
 	READ32(sess->fore_channel.maxresp_cached);
 	READ32(sess->fore_channel.maxops);
 	READ32(sess->fore_channel.maxreqs);

Signature · 1 line

1.7.5.4

Mi Jinlong

author

2011-07-09 03:41 UTC #2 lore raw

PATCH v3 1/2 nfsd41: error out when client sets maxreq_sz or, maxresp_sz too small

According to RFC5661, 18.36.3,

"if the client selects a value for ca_maxresponsesize such that
a replier on a channel could never send a response,the server SHOULD return NFS4ERR_TOOSMALL in the CREATE_SESSION reply."

This patch let server error out when client sets maxreq_sz less than SEQUENCE request size, and maxresp_sz less than a SEQUENCE reply size.

v3:
use constant for min request size and min response size, add two function for channel attrs checking(back channel checking not be implement),
move size check from xdr create_session to nfsd4_create_session,

Signed-off-by Mi Jinlong <[email protected]>

---
 fs/nfsd/nfs4state.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

fs/nfsd/nfs4state.c +33 -0

index e98f3c2..e30bf37 100644
@@ -1507,6 +1507,34 @@ nfsd4_replay_create_session(struct nfsd4_create_session *cr_ses,
 	return slot->sl_status;
 }
 
+#define NFSD_MIN_REQ_HDR_SEQ_SZ	((\
+			2 * 2 + /* credential,verifier: AUTH_NULL, length 0 */ \
+			1 +	/* MIN tag is length with zero, only length */ \
+			3 +	/* version, opcount, opcode */ \
+			XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
+				/* seqid, slotID, slotID, cache */ \
+			4 ) * sizeof(__be32))
+
+#define NFSD_MIN_RESP_HDR_SEQ_SZ ((\
+			2 +	/* verifier: AUTH_NULL, length 0 */\
+			1 +	/* status */ \
+			1 +	/* MIN tag is length with zero, only length */ \
+			3 +	/* opcount, opcode, opstatus*/ \
+			XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
+				/* seqid, slotID, slotID, slotID, status */ \
+			5 ) * sizeof(__be32))
+
+static __be32 check_forechannel_attrs(struct nfsd4_channel_attrs fchannel)
+{
+	return fchannel.maxreq_sz < NFSD_MIN_REQ_HDR_SEQ_SZ
+		|| fchannel.maxresp_sz < NFSD_MIN_RESP_HDR_SEQ_SZ;
+}
+
+static __be32 check_backchannel_attrs(struct nfsd4_channel_attrs bchannel)
+{
+	return 0;
+}
+
 __be32
 nfsd4_create_session(struct svc_rqst *rqstp,
 		     struct nfsd4_compound_state *cstate,

@@ -1575,6 +1603,11 @@ nfsd4_create_session(struct svc_rqst *rqstp,
 	cr_ses->flags &= ~SESSION4_PERSIST;
 	cr_ses->flags &= ~SESSION4_RDMA;
 
+	status = nfserr_toosmall;
+	if (check_forechannel_attrs(cr_ses->fore_channel)
+	    || check_backchannel_attrs(cr_ses->back_channel))
+		goto out;
+
 	status = nfserr_jukebox;
 	new = alloc_init_session(rqstp, conf, cr_ses);
 	if (!new)

Signature · 1 line

1.7.5.4

J. Bruce Fields

2011-07-12 11:09 UTC #3 lore raw

Quoted 45 lines · Mi Jinlong

According to RFC5661, 18.36.3,
"if the client selects a value for ca_maxresponsesize such that
a replier on a channel could never send a response,the server SHOULD return NFS4ERR_TOOSMALL in the CREATE_SESSION reply."
This patch let server error out when client sets maxreq_sz less than SEQUENCE request size, and maxresp_sz less than a SEQUENCE reply size.
v3:
use constant for min request size and min response size, add two function for channel attrs checking(back channel checking not be implement),
move size check from xdr create_session to nfsd4_create_session,
Signed-off-by Mi Jinlong <[email protected]>
---
 fs/nfsd/nfs4state.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
fs/nfsd/nfs4state.c +17 -0
index e98f3c2..e30bf37 100644
@@ -1507,6 +1507,34 @@ nfsd4_replay_create_session(struct nfsd4_create_session *cr_ses,
 	return slot->sl_status;
 }
 
+#define NFSD_MIN_REQ_HDR_SEQ_SZ	((\
+			2 * 2 + /* credential,verifier: AUTH_NULL, length 0 */ \
+			1 +	/* MIN tag is length with zero, only length */ \
+			3 +	/* version, opcount, opcode */ \
+			XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
+				/* seqid, slotID, slotID, cache */ \
+			4 ) * sizeof(__be32))
+
+#define NFSD_MIN_RESP_HDR_SEQ_SZ ((\
+			2 +	/* verifier: AUTH_NULL, length 0 */\
+			1 +	/* status */ \
+			1 +	/* MIN tag is length with zero, only length */ \
+			3 +	/* opcount, opcode, opstatus*/ \
+			XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
+				/* seqid, slotID, slotID, slotID, status */ \
+			5 ) * sizeof(__be32))
+

fs/nfsd/nfs4state.c nfsd4_replay_create_session(struct nfsd4_create_session *cr_ses, · line 1536

Quoted 10 lines

fs/nfsd/nfs4state.c +10 -0

+static __be32 check_forechannel_attrs(struct nfsd4_channel_attrs fchannel)
+{
+	return fchannel.maxreq_sz < NFSD_MIN_REQ_HDR_SEQ_SZ
+		|| fchannel.maxresp_sz < NFSD_MIN_RESP_HDR_SEQ_SZ;
+}
+
+static __be32 check_backchannel_attrs(struct nfsd4_channel_attrs bchannel)
+{
+	return 0;
+}

Let's not bother with check_backchannel_attrs() until it actually does something.

Other than that, this looks fine.--b.

Quoted 20 lines

fs/nfsd/nfs4state.c +6 -0

+
 __be32
 nfsd4_create_session(struct svc_rqst *rqstp,
 		     struct nfsd4_compound_state *cstate,

@@ -1575,6 +1603,11 @@ nfsd4_create_session(struct svc_rqst *rqstp,
 	cr_ses->flags &= ~SESSION4_PERSIST;
 	cr_ses->flags &= ~SESSION4_RDMA;
 
+	status = nfserr_toosmall;
+	if (check_forechannel_attrs(cr_ses->fore_channel)
+	    || check_backchannel_attrs(cr_ses->back_channel))
+		goto out;
+
 	status = nfserr_jukebox;
 	new = alloc_init_session(rqstp, conf, cr_ses);
 	if (!new)

Signature · 1 line

1.7.5.4

Mi Jinlong

author

2011-07-14 06:48 UTC #4 lore raw

PATCH v4 1/2 nfsd41: error out when client sets maxreq_sz or, maxresp_sz too small

According to RFC5661, 18.36.3,

"if the client selects a value for ca_maxresponsesize such that
a replier on a channel could never send a response,the server SHOULD return NFS4ERR_TOOSMALL in the CREATE_SESSION reply."

This patch let server error out when client sets maxreq_sz less than SEQUENCE request size, and maxresp_sz less than a SEQUENCE reply size.

v4:
Bruce said "Let's not bother with check_backchannel_attrs() until it actually does something.". So delete the function, only check fore channel attr.

Signed-off-by Mi Jinlong <[email protected]>

---
 fs/nfsd/nfs4state.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

fs/nfsd/nfs4state.c +27 -0

index e98f3c2..72899ec 100644
@@ -1507,6 +1507,29 @@ nfsd4_replay_create_session(struct nfsd4_create_session *cr_ses,
 	return slot->sl_status;
 }
 
+#define NFSD_MIN_REQ_HDR_SEQ_SZ	((\
+			2 * 2 + /* credential,verifier: AUTH_NULL, length 0 */ \
+			1 +	/* MIN tag is length with zero, only length */ \
+			3 +	/* version, opcount, opcode */ \
+			XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
+				/* seqid, slotID, slotID, cache */ \
+			4 ) * sizeof(__be32))
+
+#define NFSD_MIN_RESP_HDR_SEQ_SZ ((\
+			2 +	/* verifier: AUTH_NULL, length 0 */\
+			1 +	/* status */ \
+			1 +	/* MIN tag is length with zero, only length */ \
+			3 +	/* opcount, opcode, opstatus*/ \
+			XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN) + \
+				/* seqid, slotID, slotID, slotID, status */ \
+			5 ) * sizeof(__be32))
+
+static __be32 check_forechannel_attrs(struct nfsd4_channel_attrs fchannel)
+{
+	return fchannel.maxreq_sz < NFSD_MIN_REQ_HDR_SEQ_SZ
+		|| fchannel.maxresp_sz < NFSD_MIN_RESP_HDR_SEQ_SZ;
+}
+
 __be32
 nfsd4_create_session(struct svc_rqst *rqstp,
 		     struct nfsd4_compound_state *cstate,

@@ -1575,6 +1598,10 @@ nfsd4_create_session(struct svc_rqst *rqstp,
 	cr_ses->flags &= ~SESSION4_PERSIST;
 	cr_ses->flags &= ~SESSION4_RDMA;
 
+	status = nfserr_toosmall;
+	if (check_forechannel_attrs(cr_ses->fore_channel))
+		goto out;
+
 	status = nfserr_jukebox;
 	new = alloc_init_session(rqstp, conf, cr_ses);
 	if (!new)

Signature · 1 line

1.7.5.4