[PATCH v1 00/25] Server-side RPC call header parsing overhaul

[PATCH v1 00/25] Server-side RPC call header parsing overhaul

[Chuck Lever]

Happy new year, campers.

The following series has been percolating for quite a while, thanks
to the extended 6.1-rc cycle. I'd like to get this work reviewed
for possible inclusion in v6.3, so I'm starting early.

The purpose of this series is to replace the svc_get* macros in the
Linux kernel server's RPC call header parsing code with xdr_stream
helpers. I've measured no change in CPU utilization after the
overhaul; svc_recv() and friends remain the highest CPU consumers by
an order of magnitude.

Memory safety: Buffer bounds checking after decoding each XDR item
is more memory-safe than the current decoding mechanism. Subsequent
memory safety improvements to the xdr_stream helpers will benefit
all who use them.

Audit friendliness: The new code has lots of comments and other
clean-up to help align it with the relevant RPC specifications. The
use of common helpers also makes the decoders easier to audit.

I've split the full series in half to make it easier to review. The
patches posted here handle RPC call header decoding. Remaining
patches, to be posted later, deal with RPC reply header encoding.

Yes, there are a lot of patches, but they are each small, easily
chewed mechanical changes.

---

Chuck Lever (25):
      SUNRPC: Push svcxdr_init_decode() into svc_process_common()
      SUNRPC: Move svcxdr_init_decode() into ->accept methods
      SUNRPC: Add an XDR decoding helper for struct opaque_auth
      SUNRPC: Convert svcauth_null_accept() to use xdr_stream
      SUNRPC: Convert svcauth_unix_accept() to use xdr_stream
      SUNRPC: Convert svcauth_tls_accept() to use xdr_stream
      SUNRPC: Move the server-side GSS upcall to a noinline function
      SUNRPC: Hoist common verifier decoding code into svcauth_gss_proc_init()
      SUNRPC: Remove gss_read_common_verf()
      SUNRPC: Remove gss_read_verf()
      SUNRPC: Convert server-side GSS upcall helpers to use xdr_stream
      SUNRPC: Replace read_u32_from_xdr_buf() with existing XDR helper
      SUNRPC: Rename automatic variables in unwrap_integ_data()
      SUNRPC: Convert unwrap_integ_data() to use xdr_stream
      SUNRPC: Rename automatic variables in unwrap_priv_data()
      SUNRPC: Convert unwrap_priv_data() to use xdr_stream
      SUNRPC: Convert gss_verify_header() to use xdr_stream
      SUNRPC: Clean up svcauth_gss_accept's NULL procedure check
      SUNRPC: Convert the svcauth_gss_accept() pre-amble to use xdr_stream
      SUNRPC: Hoist init_decode out of svc_authenticate()
      SUNRPC: Re-order construction of the first reply fields
      SUNRPC: Eliminate unneeded variable
      SUNRPC: Decode most of RPC header with xdr_stream
      SUNRPC: Remove svc_process_common's argv parameter
      SUNRPC: Hoist svcxdr_init_decode() into svc_process()


 fs/lockd/svc.c                    |   1 -
 fs/nfs/callback_xdr.c             |   1 -
 fs/nfsd/nfssvc.c                  |   1 -
 include/linux/sunrpc/msg_prot.h   |   5 +
 include/linux/sunrpc/xdr.h        |   5 +-
 net/sunrpc/auth_gss/svcauth_gss.c | 512 ++++++++++++++++--------------
 net/sunrpc/svc.c                  |  69 ++--
 net/sunrpc/svc_xprt.c             |   1 -
 net/sunrpc/svcauth.c              |  13 +-
 net/sunrpc/svcauth_unix.c         | 132 +++++---
 net/sunrpc/xdr.c                  |  50 ++-
 11 files changed, 468 insertions(+), 322 deletions(-)

--
Chuck Lever

[PATCH v1 01/25] SUNRPC: Push svcxdr_init_decode() into svc_process_common()

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Now that all vs_dispatch functions invoke svcxdr_init_decode(), it
is common code and can be pushed down into the generic RPC server.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/lockd/svc.c        |    1 -
 fs/nfs/callback_xdr.c |    1 -
 fs/nfsd/nfssvc.c      |    1 -
 net/sunrpc/svc.c      |    1 +
 4 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 59ef8a1f843f..e56d85335599 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -695,7 +695,6 @@ static int nlmsvc_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 {
 	const struct svc_procedure *procp = rqstp->rq_procinfo;
 
-	svcxdr_init_decode(rqstp);
 	if (!procp->pc_decode(rqstp, &rqstp->rq_arg_stream))
 		goto out_decode_err;
 
diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index d0cccddb7d08..46d3f5986b4e 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -984,7 +984,6 @@ nfs_callback_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 {
 	const struct svc_procedure *procp = rqstp->rq_procinfo;
 
-	svcxdr_init_decode(rqstp);
 	svcxdr_init_encode(rqstp);
 
 	*statp = procp->pc_func(rqstp);
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 56fba1cba3af..5375a28b102a 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -1040,7 +1040,6 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
 	 */
 	rqstp->rq_cachetype = proc->pc_cachetype;
 
-	svcxdr_init_decode(rqstp);
 	if (!proc->pc_decode(rqstp, &rqstp->rq_arg_stream))
 		goto out_decode_err;
 
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 85f0c3cfc877..96806c3c61a5 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1302,6 +1302,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	if (progp == NULL)
 		goto err_bad_prog;
 
+	svcxdr_init_decode(rqstp);
 	rpc_stat = progp->pg_init_request(rqstp, progp, &process);
 	switch (rpc_stat) {
 	case rpc_success:

[PATCH v1 02/25] SUNRPC: Move svcxdr_init_decode() into ->accept methods

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Refactor: So that the overhaul of each ->accept method can be done
in separate smaller patches, temporarily move the
svcxdr_init_decode() call into those methods.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |    5 +++++
 net/sunrpc/svc.c                  |    1 -
 net/sunrpc/svcauth_unix.c         |    3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 148bb0a7fa5b..8ebc06bebbec 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1280,6 +1280,7 @@ static int svcauth_gss_legacy_init(struct svc_rqst *rqstp,
 			   rsip->major_status, rsip->minor_status))
 		goto out;
 
+	svcxdr_init_decode(rqstp);
 	ret = SVC_COMPLETE;
 out:
 	cache_put(&rsip->h, sn->rsi_cache);
@@ -1408,6 +1409,7 @@ static int svcauth_gss_proxy_init(struct svc_rqst *rqstp,
 			   ud.major_status, ud.minor_status))
 		goto out;
 
+	svcxdr_init_decode(rqstp);
 	ret = SVC_COMPLETE;
 out:
 	gss_free_in_token_pages(&ud.in_token);
@@ -1644,6 +1646,7 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 		rqstp->rq_auth_stat = rpc_autherr_badcred;
 		switch (gc->gc_svc) {
 		case RPC_GSS_SVC_NONE:
+			svcxdr_init_decode(rqstp);
 			break;
 		case RPC_GSS_SVC_INTEGRITY:
 			/* placeholders for length and seq. number: */
@@ -1653,6 +1656,7 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 					gc->gc_seq, rsci->mechctx))
 				goto garbage_args;
 			rqstp->rq_auth_slack = RPC_MAX_AUTH_SIZE;
+			svcxdr_init_decode(rqstp);
 			break;
 		case RPC_GSS_SVC_PRIVACY:
 			/* placeholders for length and seq. number: */
@@ -1662,6 +1666,7 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 					gc->gc_seq, rsci->mechctx))
 				goto garbage_args;
 			rqstp->rq_auth_slack = RPC_MAX_AUTH_SIZE * 2;
+			svcxdr_init_decode(rqstp);
 			break;
 		default:
 			goto auth_err;
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 96806c3c61a5..85f0c3cfc877 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1302,7 +1302,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	if (progp == NULL)
 		goto err_bad_prog;
 
-	svcxdr_init_decode(rqstp);
 	rpc_stat = progp->pg_init_request(rqstp, progp, &process);
 	switch (rpc_stat) {
 	case rpc_success:
diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index b1efc34db6ed..3a77f3be2cf0 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -762,6 +762,7 @@ svcauth_null_accept(struct svc_rqst *rqstp)
 	svc_putnl(resv, 0);
 
 	rqstp->rq_cred.cr_flavor = RPC_AUTH_NULL;
+	svcxdr_init_decode(rqstp);
 	return SVC_OK;
 }
 
@@ -835,6 +836,7 @@ svcauth_tls_accept(struct svc_rqst *rqstp)
 		svc_putnl(resv, 0);
 
 	rqstp->rq_cred.cr_flavor = RPC_AUTH_TLS;
+	svcxdr_init_decode(rqstp);
 	return SVC_OK;
 }
 
@@ -900,6 +902,7 @@ svcauth_unix_accept(struct svc_rqst *rqstp)
 	svc_putnl(resv, 0);
 
 	rqstp->rq_cred.cr_flavor = RPC_AUTH_UNIX;
+	svcxdr_init_decode(rqstp);
 	return SVC_OK;
 
 badcred:

[PATCH v1 03/25] SUNRPC: Add an XDR decoding helper for struct opaque_auth

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

RFC 5531 defines the body of an RPC Call message like this:

	struct call_body {
		unsigned int rpcvers;
		unsigned int prog;
		unsigned int vers;
		unsigned int proc;
		opaque_auth cred;
		opaque_auth verf;
		/* procedure-specific parameters start here */
	};

In the current server code, decoding a struct opaque_auth type is
open-coded in several places, and is thus difficult to harden
everywhere.

Introduce a helper for decoding an opaque_auth within the context
of a xdr_stream. This helper can be shared with all authentication
flavor implemenations, even on the client-side.

Done as part of hardening the server-side RPC header decoding paths.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/xdr.h |    3 +++
 net/sunrpc/xdr.c           |   28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index f84e2a1358e1..8b5c9d0cdcb5 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -346,6 +346,9 @@ ssize_t xdr_stream_decode_string(struct xdr_stream *xdr, char *str,
 		size_t size);
 ssize_t xdr_stream_decode_string_dup(struct xdr_stream *xdr, char **str,
 		size_t maxlen, gfp_t gfp_flags);
+ssize_t xdr_stream_decode_opaque_auth(struct xdr_stream *xdr, u32 *flavor,
+		void **body, unsigned int *body_len);
+
 /**
  * xdr_align_size - Calculate padded size of an object
  * @n: Size of an object being XDR encoded (in bytes)
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index f7767bf22406..4845ba2113fd 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -2274,3 +2274,31 @@ ssize_t xdr_stream_decode_string_dup(struct xdr_stream *xdr, char **str,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(xdr_stream_decode_string_dup);
+
+/**
+ * xdr_stream_decode_opaque_auth - Decode struct opaque_auth (RFC5531 S8.2)
+ * @xdr: pointer to xdr_stream
+ * @flavor: location to store decoded flavor
+ * @body: location to store decode body
+ * @body_len: location to store length of decoded body
+ *
+ * Return values:
+ *   On success, returns the number of buffer bytes consumed
+ *   %-EBADMSG on XDR buffer overflow
+ *   %-EMSGSIZE if the decoded size of the body field exceeds 400 octets
+ */
+ssize_t xdr_stream_decode_opaque_auth(struct xdr_stream *xdr, u32 *flavor,
+				      void **body, unsigned int *body_len)
+{
+	ssize_t ret, len;
+
+	len = xdr_stream_decode_u32(xdr, flavor);
+	if (unlikely(len < 0))
+		return len;
+	ret = xdr_stream_decode_opaque_inline(xdr, body, RPC_MAX_AUTH_SIZE);
+	if (unlikely(ret < 0))
+		return ret;
+	*body_len = ret;
+	return len + ret;
+}
+EXPORT_SYMBOL_GPL(xdr_stream_decode_opaque_auth);

[PATCH v1 04/25] SUNRPC: Convert svcauth_null_accept() to use xdr_stream

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Done as part of hardening the server-side RPC header decoding path.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/svcauth_unix.c |   33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index 3a77f3be2cf0..95354f03bb05 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -729,23 +729,41 @@ svcauth_unix_set_client(struct svc_rqst *rqstp)
 
 EXPORT_SYMBOL_GPL(svcauth_unix_set_client);
 
+/**
+ * svcauth_null_accept - Decode and validate incoming RPC_AUTH_NULL credential
+ * @rqstp: RPC transaction
+ *
+ * Return values:
+ *   %SVC_OK: Both credential and verifier are valid
+ *   %SVC_DENIED: Credential or verifier is not valid
+ *   %SVC_GARBAGE: Failed to decode credential or verifier
+ *   %SVC_CLOSE: Temporary failure
+ *
+ * rqstp->rq_auth_stat is set as mandated by RFC 5531.
+ */
 static int
 svcauth_null_accept(struct svc_rqst *rqstp)
 {
-	struct kvec	*argv = &rqstp->rq_arg.head[0];
 	struct kvec	*resv = &rqstp->rq_res.head[0];
+	struct xdr_stream *xdr = &rqstp->rq_arg_stream;
 	struct svc_cred	*cred = &rqstp->rq_cred;
+	u32 flavor, len;
+	void *body;
 
-	if (argv->iov_len < 3*4)
-		return SVC_GARBAGE;
+	svcxdr_init_decode(rqstp);
 
-	if (svc_getu32(argv) != 0) {
-		dprintk("svc: bad null cred\n");
+	/* Length of Call's credential body field: */
+	if (xdr_stream_decode_u32(xdr, &len) < 0)
+		return SVC_GARBAGE;
+	if (len != 0) {
 		rqstp->rq_auth_stat = rpc_autherr_badcred;
 		return SVC_DENIED;
 	}
-	if (svc_getu32(argv) != htonl(RPC_AUTH_NULL) || svc_getu32(argv) != 0) {
-		dprintk("svc: bad null verf\n");
+
+	/* Call's verf field: */
+	if (xdr_stream_decode_opaque_auth(xdr, &flavor, &body, &len) < 0)
+		return SVC_GARBAGE;
+	if (flavor != RPC_AUTH_NULL || len != 0) {
 		rqstp->rq_auth_stat = rpc_autherr_badverf;
 		return SVC_DENIED;
 	}
@@ -762,7 +780,6 @@ svcauth_null_accept(struct svc_rqst *rqstp)
 	svc_putnl(resv, 0);
 
 	rqstp->rq_cred.cr_flavor = RPC_AUTH_NULL;
-	svcxdr_init_decode(rqstp);
 	return SVC_OK;
 }
 

[PATCH v1 05/25] SUNRPC: Convert svcauth_unix_accept() to use xdr_stream

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Done as part of hardening the server-side RPC header decoding path.

Since the server-side of the Linux kernel SunRPC implementation
ignores the contents of the Call's machinename field, there's no
need for its RPC_AUTH_UNIX authenticator to reject names that are
larger than UNX_MAXNODENAME.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/msg_prot.h |    5 +++
 net/sunrpc/svcauth_unix.c       |   71 ++++++++++++++++++++++++++++-----------
 2 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/include/linux/sunrpc/msg_prot.h b/include/linux/sunrpc/msg_prot.h
index 02117ed0fa2e..c4b0eb2b2f04 100644
--- a/include/linux/sunrpc/msg_prot.h
+++ b/include/linux/sunrpc/msg_prot.h
@@ -34,6 +34,11 @@ enum rpc_auth_flavors {
 	RPC_AUTH_GSS_SPKMP = 390011,
 };
 
+/* Maximum size (in octets) of the machinename in an AUTH_UNIX
+ * credential (per RFC 5531 Appendix A)
+ */
+#define RPC_MAX_MACHINENAME	(255)
+
 /* Maximum size (in bytes) of an rpc credential or verifier */
 #define RPC_MAX_AUTH_SIZE (400)
 
diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index 95354f03bb05..b6aef9c5113b 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -867,26 +867,45 @@ struct auth_ops svcauth_tls = {
 };
 
 
+/**
+ * svcauth_unix_accept - Decode and validate incoming RPC_AUTH_SYS credential
+ * @rqstp: RPC transaction
+ *
+ * Return values:
+ *   %SVC_OK: Both credential and verifier are valid
+ *   %SVC_DENIED: Credential or verifier is not valid
+ *   %SVC_GARBAGE: Failed to decode credential or verifier
+ *   %SVC_CLOSE: Temporary failure
+ *
+ * rqstp->rq_auth_stat is set as mandated by RFC 5531.
+ */
 static int
 svcauth_unix_accept(struct svc_rqst *rqstp)
 {
-	struct kvec	*argv = &rqstp->rq_arg.head[0];
 	struct kvec	*resv = &rqstp->rq_res.head[0];
+	struct xdr_stream *xdr = &rqstp->rq_arg_stream;
 	struct svc_cred	*cred = &rqstp->rq_cred;
 	struct user_namespace *userns;
-	u32		slen, i;
-	int		len   = argv->iov_len;
+	u32 flavor, len, i;
+	void *body;
+	__be32 *p;
+
+	svcxdr_init_decode(rqstp);
 
-	if ((len -= 3*4) < 0)
+	/*
+	 * This implementation ignores the length of the Call's
+	 * credential body field and the timestamp and machinename
+	 * fields.
+	 */
+	p = xdr_inline_decode(xdr, XDR_UNIT * 3);
+	if (!p)
+		return SVC_GARBAGE;
+	len = be32_to_cpup(p + 2);
+	if (len > RPC_MAX_MACHINENAME)
+		return SVC_GARBAGE;
+	if (!xdr_inline_decode(xdr, len))
 		return SVC_GARBAGE;
 
-	svc_getu32(argv);			/* length */
-	svc_getu32(argv);			/* time stamp */
-	slen = XDR_QUADLEN(svc_getnl(argv));	/* machname length */
-	if (slen > 64 || (len -= (slen + 3)*4) < 0)
-		goto badcred;
-	argv->iov_base = (void*)((__be32*)argv->iov_base + slen);	/* skip machname */
-	argv->iov_len -= slen*4;
 	/*
 	 * Note: we skip uid_valid()/gid_valid() checks here for
 	 * backwards compatibility with clients that use -1 id's.
@@ -896,20 +915,33 @@ svcauth_unix_accept(struct svc_rqst *rqstp)
 	 */
 	userns = (rqstp->rq_xprt && rqstp->rq_xprt->xpt_cred) ?
 		rqstp->rq_xprt->xpt_cred->user_ns : &init_user_ns;
-	cred->cr_uid = make_kuid(userns, svc_getnl(argv)); /* uid */
-	cred->cr_gid = make_kgid(userns, svc_getnl(argv)); /* gid */
-	slen = svc_getnl(argv);			/* gids length */
-	if (slen > UNX_NGROUPS || (len -= (slen + 2)*4) < 0)
+	if (xdr_stream_decode_u32(xdr, &i) < 0)
+		return SVC_GARBAGE;
+	cred->cr_uid = make_kuid(userns, i);
+	if (xdr_stream_decode_u32(xdr, &i) < 0)
+		return SVC_GARBAGE;
+	cred->cr_gid = make_kgid(userns, i);
+
+	if (xdr_stream_decode_u32(xdr, &len) < 0)
+		return SVC_GARBAGE;
+	if (len > UNX_NGROUPS)
 		goto badcred;
-	cred->cr_group_info = groups_alloc(slen);
+	p = xdr_inline_decode(xdr, XDR_UNIT * len);
+	if (!p)
+		return SVC_GARBAGE;
+	cred->cr_group_info = groups_alloc(len);
 	if (cred->cr_group_info == NULL)
 		return SVC_CLOSE;
-	for (i = 0; i < slen; i++) {
-		kgid_t kgid = make_kgid(userns, svc_getnl(argv));
+	for (i = 0; i < len; i++) {
+		kgid_t kgid = make_kgid(userns, be32_to_cpup(p++));
 		cred->cr_group_info->gid[i] = kgid;
 	}
 	groups_sort(cred->cr_group_info);
-	if (svc_getu32(argv) != htonl(RPC_AUTH_NULL) || svc_getu32(argv) != 0) {
+
+	/* Call's verf field: */
+	if (xdr_stream_decode_opaque_auth(xdr, &flavor, &body, &len) < 0)
+		return SVC_GARBAGE;
+	if (flavor != RPC_AUTH_NULL || len != 0) {
 		rqstp->rq_auth_stat = rpc_autherr_badverf;
 		return SVC_DENIED;
 	}
@@ -919,7 +951,6 @@ svcauth_unix_accept(struct svc_rqst *rqstp)
 	svc_putnl(resv, 0);
 
 	rqstp->rq_cred.cr_flavor = RPC_AUTH_UNIX;
-	svcxdr_init_decode(rqstp);
 	return SVC_OK;
 
 badcred:

[PATCH v1 06/25] SUNRPC: Convert svcauth_tls_accept() to use xdr_stream

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Done as part of hardening the server-side RPC header decoding path.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/svcauth_unix.c |   37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index b6aef9c5113b..168e12137754 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -807,25 +807,41 @@ struct auth_ops svcauth_null = {
 };
 
 
+/**
+ * svcauth_tls_accept - Decode and validate incoming RPC_AUTH_TLS credential
+ * @rqstp: RPC transaction
+ *
+ * Return values:
+ *   %SVC_OK: Both credential and verifier are valid
+ *   %SVC_DENIED: Credential or verifier is not valid
+ *   %SVC_GARBAGE: Failed to decode credential or verifier
+ *   %SVC_CLOSE: Temporary failure
+ *
+ * rqstp->rq_auth_stat is set as mandated by RFC 5531.
+ */
 static int
 svcauth_tls_accept(struct svc_rqst *rqstp)
 {
+	struct xdr_stream *xdr = &rqstp->rq_arg_stream;
 	struct svc_cred	*cred = &rqstp->rq_cred;
-	struct kvec *argv = rqstp->rq_arg.head;
 	struct kvec *resv = rqstp->rq_res.head;
+	u32 flavor, len;
+	void *body;
 
-	if (argv->iov_len < XDR_UNIT * 3)
-		return SVC_GARBAGE;
+	svcxdr_init_decode(rqstp);
 
-	/* Call's cred length */
-	if (svc_getu32(argv) != xdr_zero) {
+	/* Length of Call's credential body field: */
+	if (xdr_stream_decode_u32(xdr, &len) < 0)
+		return SVC_GARBAGE;
+	if (len != 0) {
 		rqstp->rq_auth_stat = rpc_autherr_badcred;
 		return SVC_DENIED;
 	}
 
-	/* Call's verifier flavor and its length */
-	if (svc_getu32(argv) != rpc_auth_null ||
-	    svc_getu32(argv) != xdr_zero) {
+	/* Call's verf field: */
+	if (xdr_stream_decode_opaque_auth(xdr, &flavor, &body, &len) < 0)
+		return SVC_GARBAGE;
+	if (flavor != RPC_AUTH_NULL || len != 0) {
 		rqstp->rq_auth_stat = rpc_autherr_badverf;
 		return SVC_DENIED;
 	}
@@ -836,12 +852,12 @@ svcauth_tls_accept(struct svc_rqst *rqstp)
 		return SVC_DENIED;
 	}
 
-	/* Mapping to nobody uid/gid is required */
+	/* Signal that mapping to nobody uid/gid is required */
 	cred->cr_uid = INVALID_UID;
 	cred->cr_gid = INVALID_GID;
 	cred->cr_group_info = groups_alloc(0);
 	if (cred->cr_group_info == NULL)
-		return SVC_CLOSE; /* kmalloc failure - client must retry */
+		return SVC_CLOSE;
 
 	/* Reply's verifier */
 	svc_putnl(resv, RPC_AUTH_NULL);
@@ -853,7 +869,6 @@ svcauth_tls_accept(struct svc_rqst *rqstp)
 		svc_putnl(resv, 0);
 
 	rqstp->rq_cred.cr_flavor = RPC_AUTH_TLS;
-	svcxdr_init_decode(rqstp);
 	return SVC_OK;
 }
 

[PATCH v1 07/25] SUNRPC: Move the server-side GSS upcall to a noinline function

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Since upcalls are infrequent, ensure the compiler places the upcall
mechanism out-of-line from the I/O path.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 8ebc06bebbec..2a11721dd5c4 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1444,6 +1444,14 @@ static bool use_gss_proxy(struct net *net)
 	return sn->use_gss_proxy;
 }
 
+static noinline_for_stack int
+svcauth_gss_proc_init(struct svc_rqst *rqstp, struct rpc_gss_wire_cred *gc)
+{
+	if (!use_gss_proxy(SVC_NET(rqstp)))
+		return svcauth_gss_legacy_init(rqstp, gc);
+	return svcauth_gss_proxy_init(rqstp, gc);
+}
+
 #ifdef CONFIG_PROC_FS
 
 static ssize_t write_gssp(struct file *file, const char __user *buf,
@@ -1600,10 +1608,7 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 	switch (gc->gc_proc) {
 	case RPC_GSS_PROC_INIT:
 	case RPC_GSS_PROC_CONTINUE_INIT:
-		if (use_gss_proxy(SVC_NET(rqstp)))
-			return svcauth_gss_proxy_init(rqstp, gc);
-		else
-			return svcauth_gss_legacy_init(rqstp, gc);
+		return svcauth_gss_proc_init(rqstp, gc);
 	case RPC_GSS_PROC_DATA:
 	case RPC_GSS_PROC_DESTROY:
 		/* Look up the context, and check the verifier: */

[PATCH v1 08/25] SUNRPC: Hoist common verifier decoding code into svcauth_gss_proc_init()

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Pre-requisite to replacing gss_read_common_verf().

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |   26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 2a11721dd5c4..ba32c05f0258 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1095,18 +1095,6 @@ gss_read_common_verf(struct rpc_gss_wire_cred *gc,
 		     struct kvec *argv, __be32 *authp,
 		     struct xdr_netobj *in_handle)
 {
-	/* Read the verifier; should be NULL: */
-	*authp = rpc_autherr_badverf;
-	if (argv->iov_len < 2 * 4)
-		return SVC_DENIED;
-	if (svc_getnl(argv) != RPC_AUTH_NULL)
-		return SVC_DENIED;
-	if (svc_getnl(argv) != 0)
-		return SVC_DENIED;
-	/* Martial context handle and token for upcall: */
-	*authp = rpc_autherr_badcred;
-	if (gc->gc_proc == RPC_GSS_PROC_INIT && gc->gc_ctx.len != 0)
-		return SVC_DENIED;
 	if (dup_netobj(in_handle, &gc->gc_ctx))
 		return SVC_CLOSE;
 	*authp = rpc_autherr_badverf;
@@ -1447,6 +1435,20 @@ static bool use_gss_proxy(struct net *net)
 static noinline_for_stack int
 svcauth_gss_proc_init(struct svc_rqst *rqstp, struct rpc_gss_wire_cred *gc)
 {
+	struct kvec *argv = rqstp->rq_arg.head;
+
+	if (argv->iov_len < 2 * 4)
+		return SVC_DENIED;
+	if (svc_getnl(argv) != RPC_AUTH_NULL)
+		return SVC_DENIED;
+	if (svc_getnl(argv) != 0)
+		return SVC_DENIED;
+
+	if (gc->gc_proc == RPC_GSS_PROC_INIT && gc->gc_ctx.len != 0) {
+		rqstp->rq_auth_stat = rpc_autherr_badcred;
+		return SVC_DENIED;
+	}
+
 	if (!use_gss_proxy(SVC_NET(rqstp)))
 		return svcauth_gss_legacy_init(rqstp, gc);
 	return svcauth_gss_proxy_init(rqstp, gc);

[PATCH v1 09/25] SUNRPC: Remove gss_read_common_verf()

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

gss_read_common_verf() is now just a wrapper for dup_netobj(), thus
it can be replaced with direct calls to dup_netobj().

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |   26 +++++---------------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index ba32c05f0258..5da4a3d48a7d 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1090,18 +1090,6 @@ gss_write_init_verf(struct cache_detail *cd, struct svc_rqst *rqstp,
 	return rc;
 }
 
-static inline int
-gss_read_common_verf(struct rpc_gss_wire_cred *gc,
-		     struct kvec *argv, __be32 *authp,
-		     struct xdr_netobj *in_handle)
-{
-	if (dup_netobj(in_handle, &gc->gc_ctx))
-		return SVC_CLOSE;
-	*authp = rpc_autherr_badverf;
-
-	return 0;
-}
-
 static inline int
 gss_read_verf(struct rpc_gss_wire_cred *gc,
 	      struct kvec *argv, __be32 *authp,
@@ -1109,12 +1097,9 @@ gss_read_verf(struct rpc_gss_wire_cred *gc,
 	      struct xdr_netobj *in_token)
 {
 	struct xdr_netobj tmpobj;
-	int res;
-
-	res = gss_read_common_verf(gc, argv, authp, in_handle);
-	if (res)
-		return res;
 
+	if (dup_netobj(in_handle, &gc->gc_ctx))
+		return SVC_CLOSE;
 	if (svc_safe_getnetobj(argv, &tmpobj)) {
 		kfree(in_handle->data);
 		return SVC_DENIED;
@@ -1151,12 +1136,11 @@ static int gss_read_proxy_verf(struct svc_rqst *rqstp,
 {
 	struct kvec *argv = &rqstp->rq_arg.head[0];
 	unsigned int length, pgto_offs, pgfrom_offs;
-	int pages, i, res, pgto, pgfrom;
 	size_t inlen, to_offs, from_offs;
+	int pages, i, pgto, pgfrom;
 
-	res = gss_read_common_verf(gc, argv, &rqstp->rq_auth_stat, in_handle);
-	if (res)
-		return res;
+	if (dup_netobj(in_handle, &gc->gc_ctx))
+		return SVC_CLOSE;
 
 	inlen = svc_getnl(argv);
 	if (inlen > (argv->iov_len + rqstp->rq_arg.page_len)) {

[PATCH v1 10/25] SUNRPC: Remove gss_read_verf()

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

gss_read_verf() is already short. Fold it into its only caller.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |   37 +++++++++++--------------------------
 1 file changed, 11 insertions(+), 26 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 5da4a3d48a7d..5b03a97e32b7 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1090,28 +1090,6 @@ gss_write_init_verf(struct cache_detail *cd, struct svc_rqst *rqstp,
 	return rc;
 }
 
-static inline int
-gss_read_verf(struct rpc_gss_wire_cred *gc,
-	      struct kvec *argv, __be32 *authp,
-	      struct xdr_netobj *in_handle,
-	      struct xdr_netobj *in_token)
-{
-	struct xdr_netobj tmpobj;
-
-	if (dup_netobj(in_handle, &gc->gc_ctx))
-		return SVC_CLOSE;
-	if (svc_safe_getnetobj(argv, &tmpobj)) {
-		kfree(in_handle->data);
-		return SVC_DENIED;
-	}
-	if (dup_netobj(in_token, &tmpobj)) {
-		kfree(in_handle->data);
-		return SVC_CLOSE;
-	}
-
-	return 0;
-}
-
 static void gss_free_in_token_pages(struct gssp_in_token *in_token)
 {
 	u32 inlen;
@@ -1224,14 +1202,21 @@ static int svcauth_gss_legacy_init(struct svc_rqst *rqstp,
 	struct kvec *argv = &rqstp->rq_arg.head[0];
 	struct kvec *resv = &rqstp->rq_res.head[0];
 	struct rsi *rsip, rsikey;
+	struct xdr_netobj tmpobj;
 	int ret;
 	struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
 
 	memset(&rsikey, 0, sizeof(rsikey));
-	ret = gss_read_verf(gc, argv, &rqstp->rq_auth_stat,
-			    &rsikey.in_handle, &rsikey.in_token);
-	if (ret)
-		return ret;
+	if (dup_netobj(&rsikey.in_handle, &gc->gc_ctx))
+		return SVC_CLOSE;
+	if (svc_safe_getnetobj(argv, &tmpobj)) {
+		kfree(rsikey.in_handle.data);
+		return SVC_DENIED;
+	}
+	if (dup_netobj(&rsikey.in_token, &tmpobj)) {
+		kfree(rsikey.in_handle.data);
+		return SVC_CLOSE;
+	}
 
 	/* Perform upcall, or find upcall result: */
 	rsip = rsi_lookup(sn->rsi_cache, &rsikey);

[PATCH v1 11/25] SUNRPC: Convert server-side GSS upcall helpers to use xdr_stream

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

The entire RPC_GSS_PROC_INIT path is converted over to xdr_stream
for decoding the Call credential and verifier.

Done as part of hardening the server-side RPC header decoding path.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |   87 +++++++++++++++++++++++++------------
 1 file changed, 58 insertions(+), 29 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 5b03a97e32b7..8e8dec664a89 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1112,39 +1112,43 @@ static int gss_read_proxy_verf(struct svc_rqst *rqstp,
 			       struct xdr_netobj *in_handle,
 			       struct gssp_in_token *in_token)
 {
-	struct kvec *argv = &rqstp->rq_arg.head[0];
+	struct xdr_stream *xdr = &rqstp->rq_arg_stream;
 	unsigned int length, pgto_offs, pgfrom_offs;
-	size_t inlen, to_offs, from_offs;
 	int pages, i, pgto, pgfrom;
+	size_t to_offs, from_offs;
+	u32 inlen;
 
 	if (dup_netobj(in_handle, &gc->gc_ctx))
 		return SVC_CLOSE;
 
-	inlen = svc_getnl(argv);
-	if (inlen > (argv->iov_len + rqstp->rq_arg.page_len)) {
-		kfree(in_handle->data);
-		return SVC_DENIED;
-	}
+	/*
+	 *  RFC 2203 Section 5.2.2
+	 *
+	 *	struct rpc_gss_init_arg {
+	 *		opaque gss_token<>;
+	 *	};
+	 */
+	if (xdr_stream_decode_u32(xdr, &inlen) < 0)
+		goto out_denied_free;
+	if (inlen > xdr_stream_remaining(xdr))
+		goto out_denied_free;
 
 	pages = DIV_ROUND_UP(inlen, PAGE_SIZE);
 	in_token->pages = kcalloc(pages, sizeof(struct page *), GFP_KERNEL);
-	if (!in_token->pages) {
-		kfree(in_handle->data);
-		return SVC_DENIED;
-	}
+	if (!in_token->pages)
+		goto out_denied_free;
 	in_token->page_base = 0;
 	in_token->page_len = inlen;
 	for (i = 0; i < pages; i++) {
 		in_token->pages[i] = alloc_page(GFP_KERNEL);
 		if (!in_token->pages[i]) {
-			kfree(in_handle->data);
 			gss_free_in_token_pages(in_token);
-			return SVC_DENIED;
+			goto out_denied_free;
 		}
 	}
 
-	length = min_t(unsigned int, inlen, argv->iov_len);
-	memcpy(page_address(in_token->pages[0]), argv->iov_base, length);
+	length = min_t(unsigned int, inlen, (char *)xdr->end - (char *)xdr->p);
+	memcpy(page_address(in_token->pages[0]), xdr->p, length);
 	inlen -= length;
 
 	to_offs = length;
@@ -1167,6 +1171,10 @@ static int gss_read_proxy_verf(struct svc_rqst *rqstp,
 		inlen -= length;
 	}
 	return 0;
+
+out_denied_free:
+	kfree(in_handle->data);
+	return SVC_DENIED;
 }
 
 static inline int
@@ -1196,27 +1204,45 @@ gss_write_resv(struct kvec *resv, size_t size_limit,
  * the upcall results are available, write the verifier and result.
  * Otherwise, drop the request pending an answer to the upcall.
  */
-static int svcauth_gss_legacy_init(struct svc_rqst *rqstp,
-				   struct rpc_gss_wire_cred *gc)
+static int
+svcauth_gss_legacy_init(struct svc_rqst *rqstp,
+			struct rpc_gss_wire_cred *gc)
 {
-	struct kvec *argv = &rqstp->rq_arg.head[0];
+	struct xdr_stream *xdr = &rqstp->rq_arg_stream;
 	struct kvec *resv = &rqstp->rq_res.head[0];
 	struct rsi *rsip, rsikey;
-	struct xdr_netobj tmpobj;
+	__be32 *p;
+	u32 len;
 	int ret;
 	struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
 
 	memset(&rsikey, 0, sizeof(rsikey));
 	if (dup_netobj(&rsikey.in_handle, &gc->gc_ctx))
 		return SVC_CLOSE;
-	if (svc_safe_getnetobj(argv, &tmpobj)) {
+
+	/*
+	 *  RFC 2203 Section 5.2.2
+	 *
+	 *	struct rpc_gss_init_arg {
+	 *		opaque gss_token<>;
+	 *	};
+	 */
+	if (xdr_stream_decode_u32(xdr, &len) < 0) {
+		kfree(rsikey.in_handle.data);
+		return SVC_DENIED;
+	}
+	p = xdr_inline_decode(xdr, len);
+	if (!p) {
 		kfree(rsikey.in_handle.data);
 		return SVC_DENIED;
 	}
-	if (dup_netobj(&rsikey.in_token, &tmpobj)) {
+	rsikey.in_token.data = kmalloc(len, GFP_KERNEL);
+	if (ZERO_OR_NULL_PTR(rsikey.in_token.data)) {
 		kfree(rsikey.in_handle.data);
 		return SVC_CLOSE;
 	}
+	memcpy(rsikey.in_token.data, p, len);
+	rsikey.in_token.len = len;
 
 	/* Perform upcall, or find upcall result: */
 	rsip = rsi_lookup(sn->rsi_cache, &rsikey);
@@ -1237,7 +1263,6 @@ static int svcauth_gss_legacy_init(struct svc_rqst *rqstp,
 			   rsip->major_status, rsip->minor_status))
 		goto out;
 
-	svcxdr_init_decode(rqstp);
 	ret = SVC_COMPLETE;
 out:
 	cache_put(&rsip->h, sn->rsi_cache);
@@ -1366,7 +1391,6 @@ static int svcauth_gss_proxy_init(struct svc_rqst *rqstp,
 			   ud.major_status, ud.minor_status))
 		goto out;
 
-	svcxdr_init_decode(rqstp);
 	ret = SVC_COMPLETE;
 out:
 	gss_free_in_token_pages(&ud.in_token);
@@ -1404,14 +1428,19 @@ static bool use_gss_proxy(struct net *net)
 static noinline_for_stack int
 svcauth_gss_proc_init(struct svc_rqst *rqstp, struct rpc_gss_wire_cred *gc)
 {
-	struct kvec *argv = rqstp->rq_arg.head;
+	struct xdr_stream *xdr = &rqstp->rq_arg_stream;
+	u32 flavor, len;
+	void *body;
 
-	if (argv->iov_len < 2 * 4)
-		return SVC_DENIED;
-	if (svc_getnl(argv) != RPC_AUTH_NULL)
-		return SVC_DENIED;
-	if (svc_getnl(argv) != 0)
+	svcxdr_init_decode(rqstp);
+
+	/* Call's verf field: */
+	if (xdr_stream_decode_opaque_auth(xdr, &flavor, &body, &len) < 0)
+		return SVC_GARBAGE;
+	if (flavor != RPC_AUTH_NULL || len != 0) {
+		rqstp->rq_auth_stat = rpc_autherr_badverf;
 		return SVC_DENIED;
+	}
 
 	if (gc->gc_proc == RPC_GSS_PROC_INIT && gc->gc_ctx.len != 0) {
 		rqstp->rq_auth_stat = rpc_autherr_badcred;

[PATCH v1 12/25] SUNRPC: Replace read_u32_from_xdr_buf() with existing XDR helper

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Clean up / code de-duplication - this functionality is already
available in the generic XDR layer.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |   16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 8e8dec664a89..0f9700c86f62 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -891,19 +891,6 @@ svcauth_gss_register_pseudoflavor(u32 pseudoflavor, char * name)
 }
 EXPORT_SYMBOL_GPL(svcauth_gss_register_pseudoflavor);
 
-static inline int
-read_u32_from_xdr_buf(struct xdr_buf *buf, int base, u32 *obj)
-{
-	__be32  raw;
-	int     status;
-
-	status = read_bytes_from_xdr_buf(buf, base, &raw, sizeof(*obj));
-	if (status)
-		return status;
-	*obj = ntohl(raw);
-	return 0;
-}
-
 /* It would be nice if this bit of code could be shared with the client.
  * Obstacles:
  *	The client shouldn't malloc(), would have to pass in own memory.
@@ -937,8 +924,7 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
 	if (xdr_buf_subsegment(buf, &integ_buf, 0, integ_len))
 		goto unwrap_failed;
 
-	/* copy out mic... */
-	if (read_u32_from_xdr_buf(buf, integ_len, &mic.len))
+	if (xdr_decode_word(buf, integ_len, &mic.len))
 		goto unwrap_failed;
 	if (mic.len > sizeof(gsd->gsd_scratch))
 		goto unwrap_failed;

[PATCH v1 13/25] SUNRPC: Rename automatic variables in unwrap_integ_data()

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Clean up: To help orient readers, name the stack variables to match
the XDR field names.

For readability, I'm also going to rename the unwrap and wrap
functions in a consistent manner, starting with unwrap_integ_data().

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |   57 +++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 24 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 0f9700c86f62..33fe307372d0 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -891,18 +891,27 @@ svcauth_gss_register_pseudoflavor(u32 pseudoflavor, char * name)
 }
 EXPORT_SYMBOL_GPL(svcauth_gss_register_pseudoflavor);
 
-/* It would be nice if this bit of code could be shared with the client.
- * Obstacles:
- *	The client shouldn't malloc(), would have to pass in own memory.
- *	The server uses base of head iovec as read pointer, while the
- *	client uses separate pointer. */
+/*
+ * RFC 2203, Section 5.3.2.2
+ *
+ *	struct rpc_gss_integ_data {
+ *		opaque databody_integ<>;
+ *		opaque checksum<>;
+ *	};
+ *
+ *	struct rpc_gss_data_t {
+ *		unsigned int seq_num;
+ *		proc_req_arg_t arg;
+ *	};
+ */
 static int
-unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gss_ctx *ctx)
+svcauth_gss_unwrap_integ(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq,
+			 struct gss_ctx *ctx)
 {
 	struct gss_svc_data *gsd = rqstp->rq_auth_data;
-	u32 integ_len, rseqno, maj_stat;
-	struct xdr_netobj mic;
-	struct xdr_buf integ_buf;
+	struct xdr_buf databody_integ;
+	u32 len, seq_num, maj_stat;
+	struct xdr_netobj checksum;
 
 	/* NFS READ normally uses splice to send data in-place. However
 	 * the data in cache can change after the reply's MIC is computed
@@ -916,36 +925,36 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g
 	if (rqstp->rq_deferred)
 		return 0;
 
-	integ_len = svc_getnl(&buf->head[0]);
-	if (integ_len & 3)
+	len = svc_getnl(&buf->head[0]);
+	if (len & 3)
 		goto unwrap_failed;
-	if (integ_len > buf->len)
+	if (len > buf->len)
 		goto unwrap_failed;
-	if (xdr_buf_subsegment(buf, &integ_buf, 0, integ_len))
+	if (xdr_buf_subsegment(buf, &databody_integ, 0, len))
 		goto unwrap_failed;
 
-	if (xdr_decode_word(buf, integ_len, &mic.len))
+	if (xdr_decode_word(buf, len, &checksum.len))
 		goto unwrap_failed;
-	if (mic.len > sizeof(gsd->gsd_scratch))
+	if (checksum.len > sizeof(gsd->gsd_scratch))
 		goto unwrap_failed;
-	mic.data = gsd->gsd_scratch;
-	if (read_bytes_from_xdr_buf(buf, integ_len + 4, mic.data, mic.len))
+	checksum.data = gsd->gsd_scratch;
+	if (read_bytes_from_xdr_buf(buf, len + 4, checksum.data, checksum.len))
 		goto unwrap_failed;
-	maj_stat = gss_verify_mic(ctx, &integ_buf, &mic);
+	maj_stat = gss_verify_mic(ctx, &databody_integ, &checksum);
 	if (maj_stat != GSS_S_COMPLETE)
 		goto bad_mic;
-	rseqno = svc_getnl(&buf->head[0]);
-	if (rseqno != seq)
+	seq_num = svc_getnl(&buf->head[0]);
+	if (seq_num != seq)
 		goto bad_seqno;
 	/* trim off the mic and padding at the end before returning */
-	xdr_buf_trim(buf, round_up_to_quad(mic.len) + 4);
+	xdr_buf_trim(buf, round_up_to_quad(checksum.len) + 4);
 	return 0;
 
 unwrap_failed:
 	trace_rpcgss_svc_unwrap_failed(rqstp);
 	return -EINVAL;
 bad_seqno:
-	trace_rpcgss_svc_seqno_bad(rqstp, seq, rseqno);
+	trace_rpcgss_svc_seqno_bad(rqstp, seq, seq_num);
 	return -EINVAL;
 bad_mic:
 	trace_rpcgss_svc_mic(rqstp, maj_stat);
@@ -1643,8 +1652,8 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 			/* placeholders for length and seq. number: */
 			svc_putnl(resv, 0);
 			svc_putnl(resv, 0);
-			if (unwrap_integ_data(rqstp, &rqstp->rq_arg,
-					gc->gc_seq, rsci->mechctx))
+			if (svcauth_gss_unwrap_integ(rqstp, &rqstp->rq_arg,
+						     gc->gc_seq, rsci->mechctx))
 				goto garbage_args;
 			rqstp->rq_auth_slack = RPC_MAX_AUTH_SIZE;
 			svcxdr_init_decode(rqstp);

[PATCH v1 14/25] SUNRPC: Convert unwrap_integ_data() to use xdr_stream

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Done as part of hardening the server-side RPC header decoding path.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/xdr.h        |    1 +
 net/sunrpc/auth_gss/svcauth_gss.c |   47 ++++++++++++++++++++++++-------------
 net/sunrpc/xdr.c                  |   15 ++++++++++++
 3 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index 8b5c9d0cdcb5..accfe8d6e283 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -247,6 +247,7 @@ extern int xdr_reserve_space_vec(struct xdr_stream *xdr, struct kvec *vec,
 		size_t nbytes);
 extern void __xdr_commit_encode(struct xdr_stream *xdr);
 extern void xdr_truncate_encode(struct xdr_stream *xdr, size_t len);
+extern void xdr_truncate_decode(struct xdr_stream *xdr, size_t len);
 extern int xdr_restrict_buflen(struct xdr_stream *xdr, int newbuflen);
 extern void xdr_write_pages(struct xdr_stream *xdr, struct page **pages,
 		unsigned int base, unsigned int len);
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 33fe307372d0..d049db997ab7 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -904,13 +904,14 @@ EXPORT_SYMBOL_GPL(svcauth_gss_register_pseudoflavor);
  *		proc_req_arg_t arg;
  *	};
  */
-static int
-svcauth_gss_unwrap_integ(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq,
-			 struct gss_ctx *ctx)
+static noinline_for_stack int
+svcauth_gss_unwrap_integ(struct svc_rqst *rqstp, u32 seq, struct gss_ctx *ctx)
 {
 	struct gss_svc_data *gsd = rqstp->rq_auth_data;
+	struct xdr_stream *xdr = &rqstp->rq_arg_stream;
+	u32 len, offset, seq_num, maj_stat;
+	struct xdr_buf *buf = xdr->buf;
 	struct xdr_buf databody_integ;
-	u32 len, seq_num, maj_stat;
 	struct xdr_netobj checksum;
 
 	/* NFS READ normally uses splice to send data in-place. However
@@ -925,29 +926,43 @@ svcauth_gss_unwrap_integ(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq,
 	if (rqstp->rq_deferred)
 		return 0;
 
-	len = svc_getnl(&buf->head[0]);
-	if (len & 3)
+	if (xdr_stream_decode_u32(xdr, &len) < 0)
 		goto unwrap_failed;
-	if (len > buf->len)
+	if (len & 3)
 		goto unwrap_failed;
-	if (xdr_buf_subsegment(buf, &databody_integ, 0, len))
+	offset = xdr_stream_pos(xdr);
+	if (xdr_buf_subsegment(buf, &databody_integ, offset, len))
 		goto unwrap_failed;
 
-	if (xdr_decode_word(buf, len, &checksum.len))
+	/*
+	 * The xdr_stream now points to the @seq_num field. The next
+	 * XDR data item is the @arg field, which contains the clear
+	 * text RPC program payload. The checksum, which follows the
+	 * @arg field, is located and decoded without updating the
+	 * xdr_stream.
+	 */
+
+	offset += len;
+	if (xdr_decode_word(buf, offset, &checksum.len))
 		goto unwrap_failed;
 	if (checksum.len > sizeof(gsd->gsd_scratch))
 		goto unwrap_failed;
 	checksum.data = gsd->gsd_scratch;
-	if (read_bytes_from_xdr_buf(buf, len + 4, checksum.data, checksum.len))
+	if (read_bytes_from_xdr_buf(buf, offset + XDR_UNIT, checksum.data,
+				    checksum.len))
 		goto unwrap_failed;
+
 	maj_stat = gss_verify_mic(ctx, &databody_integ, &checksum);
 	if (maj_stat != GSS_S_COMPLETE)
 		goto bad_mic;
-	seq_num = svc_getnl(&buf->head[0]);
+
+	/* The received seqno is protected by the checksum. */
+	if (xdr_stream_decode_u32(xdr, &seq_num) < 0)
+		goto unwrap_failed;
 	if (seq_num != seq)
 		goto bad_seqno;
-	/* trim off the mic and padding at the end before returning */
-	xdr_buf_trim(buf, round_up_to_quad(checksum.len) + 4);
+
+	xdr_truncate_decode(xdr, XDR_UNIT + checksum.len);
 	return 0;
 
 unwrap_failed:
@@ -1652,11 +1667,11 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 			/* placeholders for length and seq. number: */
 			svc_putnl(resv, 0);
 			svc_putnl(resv, 0);
-			if (svcauth_gss_unwrap_integ(rqstp, &rqstp->rq_arg,
-						     gc->gc_seq, rsci->mechctx))
+			svcxdr_init_decode(rqstp);
+			if (svcauth_gss_unwrap_integ(rqstp, gc->gc_seq,
+						     rsci->mechctx))
 				goto garbage_args;
 			rqstp->rq_auth_slack = RPC_MAX_AUTH_SIZE;
-			svcxdr_init_decode(rqstp);
 			break;
 		case RPC_GSS_SVC_PRIVACY:
 			/* placeholders for length and seq. number: */
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index 4845ba2113fd..c7e89921d511 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -1192,6 +1192,21 @@ void xdr_truncate_encode(struct xdr_stream *xdr, size_t len)
 }
 EXPORT_SYMBOL(xdr_truncate_encode);
 
+/**
+ * xdr_truncate_decode - Truncate a decoding stream
+ * @xdr: pointer to struct xdr_stream
+ * @len: Number of bytes to remove
+ *
+ */
+void xdr_truncate_decode(struct xdr_stream *xdr, size_t len)
+{
+	unsigned int nbytes = xdr_align_size(len);
+
+	xdr->buf->len -= nbytes;
+	xdr->nwords -= XDR_QUADLEN(nbytes);
+}
+EXPORT_SYMBOL_GPL(xdr_truncate_decode);
+
 /**
  * xdr_restrict_buflen - decrease available buffer space
  * @xdr: pointer to xdr_stream

[PATCH v1 15/25] SUNRPC: Rename automatic variables in unwrap_priv_data()

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Clean up: To help orient readers, name the stack variables to match
the XDR field names.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |   38 ++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index d049db997ab7..8f91768d0be0 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -993,16 +993,28 @@ fix_priv_head(struct xdr_buf *buf, int pad)
 	}
 }
 
+/*
+ * RFC 2203, Section 5.3.2.3
+ *
+ *	struct rpc_gss_priv_data {
+ *		opaque databody_priv<>
+ *	};
+ *
+ *	struct rpc_gss_data_t {
+ *		unsigned int seq_num;
+ *		proc_req_arg_t arg;
+ *	};
+ */
 static int
-unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gss_ctx *ctx)
+svcauth_gss_unwrap_priv(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq,
+			struct gss_ctx *ctx)
 {
-	u32 priv_len, maj_stat;
+	u32 len, seq_num, maj_stat;
 	int pad, remaining_len, offset;
-	u32 rseqno;
 
 	__clear_bit(RQ_SPLICE_OK, &rqstp->rq_flags);
 
-	priv_len = svc_getnl(&buf->head[0]);
+	len = svc_getnl(&buf->head[0]);
 	if (rqstp->rq_deferred) {
 		/* Already decrypted last time through! The sequence number
 		 * check at out_seq is unnecessary but harmless: */
@@ -1012,14 +1024,14 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gs
 	 * request to the end, where head[0].iov_len is just the bytes
 	 * not yet read from the head, so these two values are different: */
 	remaining_len = total_buf_len(buf);
-	if (priv_len > remaining_len)
+	if (len > remaining_len)
 		goto unwrap_failed;
-	pad = remaining_len - priv_len;
+	pad = remaining_len - len;
 	buf->len -= pad;
 	fix_priv_head(buf, pad);
 
-	maj_stat = gss_unwrap(ctx, 0, priv_len, buf);
-	pad = priv_len - buf->len;
+	maj_stat = gss_unwrap(ctx, 0, len, buf);
+	pad = len - buf->len;
 	/* The upper layers assume the buffer is aligned on 4-byte boundaries.
 	 * In the krb5p case, at least, the data ends up offset, so we need to
 	 * move it around. */
@@ -1035,8 +1047,8 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gs
 	if (maj_stat != GSS_S_COMPLETE)
 		goto bad_unwrap;
 out_seq:
-	rseqno = svc_getnl(&buf->head[0]);
-	if (rseqno != seq)
+	seq_num = svc_getnl(&buf->head[0]);
+	if (seq_num != seq)
 		goto bad_seqno;
 	return 0;
 
@@ -1044,7 +1056,7 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gs
 	trace_rpcgss_svc_unwrap_failed(rqstp);
 	return -EINVAL;
 bad_seqno:
-	trace_rpcgss_svc_seqno_bad(rqstp, seq, rseqno);
+	trace_rpcgss_svc_seqno_bad(rqstp, seq, seq_num);
 	return -EINVAL;
 bad_unwrap:
 	trace_rpcgss_svc_unwrap(rqstp, maj_stat);
@@ -1677,8 +1689,8 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 			/* placeholders for length and seq. number: */
 			svc_putnl(resv, 0);
 			svc_putnl(resv, 0);
-			if (unwrap_priv_data(rqstp, &rqstp->rq_arg,
-					gc->gc_seq, rsci->mechctx))
+			if (svcauth_gss_unwrap_priv(rqstp, &rqstp->rq_arg,
+						    gc->gc_seq, rsci->mechctx))
 				goto garbage_args;
 			rqstp->rq_auth_slack = RPC_MAX_AUTH_SIZE * 2;
 			svcxdr_init_decode(rqstp);

[PATCH v1 16/25] SUNRPC: Convert unwrap_priv_data() to use xdr_stream

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Done as part of hardening the server-side RPC header decoding path.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/xdr.h        |    1 -
 net/sunrpc/auth_gss/svcauth_gss.c |   65 ++++++++++++-------------------------
 net/sunrpc/xdr.c                  |    7 ----
 3 files changed, 21 insertions(+), 52 deletions(-)

diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index accfe8d6e283..884df67009f4 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -188,7 +188,6 @@ xdr_adjust_iovec(struct kvec *iov, __be32 *p)
 /*
  * XDR buffer helper functions
  */
-extern void xdr_shift_buf(struct xdr_buf *, size_t);
 extern void xdr_buf_from_iov(const struct kvec *, struct xdr_buf *);
 extern int xdr_buf_subsegment(const struct xdr_buf *, struct xdr_buf *, unsigned int, unsigned int);
 extern void xdr_buf_trim(struct xdr_buf *, unsigned int);
diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 8f91768d0be0..074bfe9fb838 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -982,17 +982,6 @@ total_buf_len(struct xdr_buf *buf)
 	return buf->head[0].iov_len + buf->page_len + buf->tail[0].iov_len;
 }
 
-static void
-fix_priv_head(struct xdr_buf *buf, int pad)
-{
-	if (buf->page_len == 0) {
-		/* We need to adjust head and buf->len in tandem in this
-		 * case to make svc_defer() work--it finds the original
-		 * buffer start using buf->len - buf->head[0].iov_len. */
-		buf->head[0].iov_len -= pad;
-	}
-}
-
 /*
  * RFC 2203, Section 5.3.2.3
  *
@@ -1005,49 +994,37 @@ fix_priv_head(struct xdr_buf *buf, int pad)
  *		proc_req_arg_t arg;
  *	};
  */
-static int
-svcauth_gss_unwrap_priv(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq,
-			struct gss_ctx *ctx)
+static noinline_for_stack int
+svcauth_gss_unwrap_priv(struct svc_rqst *rqstp, u32 seq, struct gss_ctx *ctx)
 {
-	u32 len, seq_num, maj_stat;
-	int pad, remaining_len, offset;
+	struct xdr_stream *xdr = &rqstp->rq_arg_stream;
+	u32 len, maj_stat, seq_num, offset;
+	struct xdr_buf *buf = xdr->buf;
+	unsigned int saved_len;
 
 	__clear_bit(RQ_SPLICE_OK, &rqstp->rq_flags);
 
-	len = svc_getnl(&buf->head[0]);
+	if (xdr_stream_decode_u32(xdr, &len) < 0)
+		goto unwrap_failed;
 	if (rqstp->rq_deferred) {
 		/* Already decrypted last time through! The sequence number
 		 * check at out_seq is unnecessary but harmless: */
 		goto out_seq;
 	}
-	/* buf->len is the number of bytes from the original start of the
-	 * request to the end, where head[0].iov_len is just the bytes
-	 * not yet read from the head, so these two values are different: */
-	remaining_len = total_buf_len(buf);
-	if (len > remaining_len)
+	if (len > xdr_stream_remaining(xdr))
 		goto unwrap_failed;
-	pad = remaining_len - len;
-	buf->len -= pad;
-	fix_priv_head(buf, pad);
-
-	maj_stat = gss_unwrap(ctx, 0, len, buf);
-	pad = len - buf->len;
-	/* The upper layers assume the buffer is aligned on 4-byte boundaries.
-	 * In the krb5p case, at least, the data ends up offset, so we need to
-	 * move it around. */
-	/* XXX: This is very inefficient.  It would be better to either do
-	 * this while we encrypt, or maybe in the receive code, if we can peak
-	 * ahead and work out the service and mechanism there. */
-	offset = xdr_pad_size(buf->head[0].iov_len);
-	if (offset) {
-		buf->buflen = RPCSVC_MAXPAYLOAD;
-		xdr_shift_buf(buf, offset);
-		fix_priv_head(buf, pad);
-	}
+	offset = xdr_stream_pos(xdr);
+
+	saved_len = buf->len;
+	maj_stat = gss_unwrap(ctx, offset, offset + len, buf);
 	if (maj_stat != GSS_S_COMPLETE)
 		goto bad_unwrap;
+	xdr->nwords -= XDR_QUADLEN(saved_len - buf->len);
+
 out_seq:
-	seq_num = svc_getnl(&buf->head[0]);
+	/* gss_unwrap() decrypted the sequence number. */
+	if (xdr_stream_decode_u32(xdr, &seq_num) < 0)
+		goto unwrap_failed;
 	if (seq_num != seq)
 		goto bad_seqno;
 	return 0;
@@ -1689,11 +1666,11 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 			/* placeholders for length and seq. number: */
 			svc_putnl(resv, 0);
 			svc_putnl(resv, 0);
-			if (svcauth_gss_unwrap_priv(rqstp, &rqstp->rq_arg,
-						    gc->gc_seq, rsci->mechctx))
+			svcxdr_init_decode(rqstp);
+			if (svcauth_gss_unwrap_priv(rqstp, gc->gc_seq,
+						    rsci->mechctx))
 				goto garbage_args;
 			rqstp->rq_auth_slack = RPC_MAX_AUTH_SIZE * 2;
-			svcxdr_init_decode(rqstp);
 			break;
 		default:
 			goto auth_err;
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index c7e89921d511..56d87c784c9e 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -863,13 +863,6 @@ static unsigned int xdr_shrink_pagelen(struct xdr_buf *buf, unsigned int len)
 	return shift;
 }
 
-void
-xdr_shift_buf(struct xdr_buf *buf, size_t len)
-{
-	xdr_shrink_bufhead(buf, buf->head->iov_len - len);
-}
-EXPORT_SYMBOL_GPL(xdr_shift_buf);
-
 /**
  * xdr_stream_pos - Return the current offset from the start of the xdr_stream
  * @xdr: pointer to struct xdr_stream

[PATCH v1 17/25] SUNRPC: Convert gss_verify_header() to use xdr_stream

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Done as part of hardening the server-side RPC header decoding path.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |   52 +++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 074bfe9fb838..8362fc143754 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -732,38 +732,48 @@ svc_safe_putnetobj(struct kvec *resv, struct xdr_netobj *o)
 }
 
 /*
- * Verify the checksum on the header and return SVC_OK on success.
- * Otherwise, return SVC_DROP (in the case of a bad sequence number)
- * or return SVC_DENIED and indicate error in rqstp->rq_auth_stat.
+ * Decode and verify a Call's verifier field. For RPC_AUTH_GSS Calls,
+ * the body of this field contains a variable length checksum.
+ *
+ * GSS-specific auth_stat values are mandated by RFC 2203 Section
+ * 5.3.3.3.
  */
 static int
-gss_verify_header(struct svc_rqst *rqstp, struct rsc *rsci,
-		  __be32 *rpcstart, struct rpc_gss_wire_cred *gc)
+svcauth_gss_verify_header(struct svc_rqst *rqstp, struct rsc *rsci,
+			  __be32 *rpcstart, struct rpc_gss_wire_cred *gc)
 {
+	struct xdr_stream	*xdr = &rqstp->rq_arg_stream;
 	struct gss_ctx		*ctx_id = rsci->mechctx;
+	u32			flavor, maj_stat;
 	struct xdr_buf		rpchdr;
 	struct xdr_netobj	checksum;
-	u32			flavor = 0;
-	struct kvec		*argv = &rqstp->rq_arg.head[0];
 	struct kvec		iov;
 
-	/* data to compute the checksum over: */
+	/*
+	 * Compute the checksum of the incoming Call from the
+	 * XID field to credential field:
+	 */
 	iov.iov_base = rpcstart;
-	iov.iov_len = (u8 *)argv->iov_base - (u8 *)rpcstart;
+	iov.iov_len = (u8 *)xdr->p - (u8 *)rpcstart;
 	xdr_buf_from_iov(&iov, &rpchdr);
 
-	rqstp->rq_auth_stat = rpc_autherr_badverf;
-	if (argv->iov_len < 4)
-		return SVC_DENIED;
-	flavor = svc_getnl(argv);
-	if (flavor != RPC_AUTH_GSS)
+	/* Call's verf field: */
+	if (xdr_stream_decode_opaque_auth(xdr, &flavor,
+					  (void **)&checksum.data,
+					  &checksum.len) < 0) {
+		rqstp->rq_auth_stat = rpc_autherr_badverf;
 		return SVC_DENIED;
-	if (svc_safe_getnetobj(argv, &checksum))
+	}
+	if (flavor != RPC_AUTH_GSS) {
+		rqstp->rq_auth_stat = rpc_autherr_badverf;
 		return SVC_DENIED;
+	}
 
-	if (rqstp->rq_deferred) /* skip verification of revisited request */
+	if (rqstp->rq_deferred)
 		return SVC_OK;
-	if (gss_verify_mic(ctx_id, &rpchdr, &checksum) != GSS_S_COMPLETE) {
+	maj_stat = gss_verify_mic(ctx_id, &rpchdr, &checksum);
+	if (maj_stat != GSS_S_COMPLETE) {
+		trace_rpcgss_svc_mic(rqstp, maj_stat);
 		rqstp->rq_auth_stat = rpcsec_gsserr_credproblem;
 		return SVC_DENIED;
 	}
@@ -1431,8 +1441,6 @@ svcauth_gss_proc_init(struct svc_rqst *rqstp, struct rpc_gss_wire_cred *gc)
 	u32 flavor, len;
 	void *body;
 
-	svcxdr_init_decode(rqstp);
-
 	/* Call's verf field: */
 	if (xdr_stream_decode_opaque_auth(xdr, &flavor, &body, &len) < 0)
 		return SVC_GARBAGE;
@@ -1603,6 +1611,7 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 	if ((gc->gc_proc != RPC_GSS_PROC_DATA) && (rqstp->rq_proc != 0))
 		goto auth_err;
 
+	svcxdr_init_decode(rqstp);
 	rqstp->rq_auth_stat = rpc_autherr_badverf;
 	switch (gc->gc_proc) {
 	case RPC_GSS_PROC_INIT:
@@ -1615,7 +1624,7 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 		rsci = gss_svc_searchbyctx(sn->rsc_cache, &gc->gc_ctx);
 		if (!rsci)
 			goto auth_err;
-		switch (gss_verify_header(rqstp, rsci, rpcstart, gc)) {
+		switch (svcauth_gss_verify_header(rqstp, rsci, rpcstart, gc)) {
 		case SVC_OK:
 			break;
 		case SVC_DENIED:
@@ -1650,13 +1659,11 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 		rqstp->rq_auth_stat = rpc_autherr_badcred;
 		switch (gc->gc_svc) {
 		case RPC_GSS_SVC_NONE:
-			svcxdr_init_decode(rqstp);
 			break;
 		case RPC_GSS_SVC_INTEGRITY:
 			/* placeholders for length and seq. number: */
 			svc_putnl(resv, 0);
 			svc_putnl(resv, 0);
-			svcxdr_init_decode(rqstp);
 			if (svcauth_gss_unwrap_integ(rqstp, gc->gc_seq,
 						     rsci->mechctx))
 				goto garbage_args;
@@ -1666,7 +1673,6 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 			/* placeholders for length and seq. number: */
 			svc_putnl(resv, 0);
 			svc_putnl(resv, 0);
-			svcxdr_init_decode(rqstp);
 			if (svcauth_gss_unwrap_priv(rqstp, gc->gc_seq,
 						    rsci->mechctx))
 				goto garbage_args;

[PATCH v1 18/25] SUNRPC: Clean up svcauth_gss_accept's NULL procedure check

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Micro-optimizations:

1. The value of rqstp->rq_auth_stat is replaced no matter which
   arm of the switch is taken, so the initial assignment can be
   safely removed.

2. Avoid checking the value of gc->gc_proc twice in the I/O
   (RPC_GSS_PROC_DATA) path.

The cost is a little extra code redundancy.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 8362fc143754..363c25198547 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1608,17 +1608,19 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 	if (crlen != round_up_to_quad(gc->gc_ctx.len) + 5 * 4)
 		goto auth_err;
 
-	if ((gc->gc_proc != RPC_GSS_PROC_DATA) && (rqstp->rq_proc != 0))
-		goto auth_err;
-
 	svcxdr_init_decode(rqstp);
-	rqstp->rq_auth_stat = rpc_autherr_badverf;
+
 	switch (gc->gc_proc) {
 	case RPC_GSS_PROC_INIT:
 	case RPC_GSS_PROC_CONTINUE_INIT:
+		if (rqstp->rq_proc != 0)
+			goto auth_err;
 		return svcauth_gss_proc_init(rqstp, gc);
-	case RPC_GSS_PROC_DATA:
 	case RPC_GSS_PROC_DESTROY:
+		if (rqstp->rq_proc != 0)
+			goto auth_err;
+		fallthrough;
+	case RPC_GSS_PROC_DATA:
 		/* Look up the context, and check the verifier: */
 		rqstp->rq_auth_stat = rpcsec_gsserr_credproblem;
 		rsci = gss_svc_searchbyctx(sn->rsc_cache, &gc->gc_ctx);
@@ -1634,6 +1636,8 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 		}
 		break;
 	default:
+		if (rqstp->rq_proc != 0)
+			goto auth_err;
 		rqstp->rq_auth_stat = rpc_autherr_rejectedcred;
 		goto auth_err;
 	}

[PATCH v1 19/25] SUNRPC: Convert the svcauth_gss_accept() pre-amble to use xdr_stream

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Done as part of hardening the server-side RPC header decoding path.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |  125 ++++++++++++++++++++++---------------
 1 file changed, 75 insertions(+), 50 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 363c25198547..557de28127fe 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -697,23 +697,6 @@ static inline u32 round_up_to_quad(u32 i)
 	return (i + 3 ) & ~3;
 }
 
-static inline int
-svc_safe_getnetobj(struct kvec *argv, struct xdr_netobj *o)
-{
-	int l;
-
-	if (argv->iov_len < 4)
-		return -1;
-	o->len = svc_getnl(argv);
-	l = round_up_to_quad(o->len);
-	if (argv->iov_len < l)
-		return -1;
-	o->data = argv->iov_base;
-	argv->iov_base += l;
-	argv->iov_len -= l;
-	return 0;
-}
-
 static inline int
 svc_safe_putnetobj(struct kvec *resv, struct xdr_netobj *o)
 {
@@ -1553,27 +1536,91 @@ static void destroy_use_gss_proxy_proc_entry(struct net *net) {}
 #endif /* CONFIG_PROC_FS */
 
 /*
- * Accept an rpcsec packet.
- * If context establishment, punt to user space
- * If data exchange, verify/decrypt
- * If context destruction, handle here
- * In the context establishment and destruction case we encode
- * response here and return SVC_COMPLETE.
+ * The Call's credential body should contain a struct rpc_gss_cred_t.
+ *
+ * RFC 2203 Section 5
+ *
+ *	struct rpc_gss_cred_t {
+ *		union switch (unsigned int version) {
+ *		case RPCSEC_GSS_VERS_1:
+ *			struct {
+ *				rpc_gss_proc_t gss_proc;
+ *				unsigned int seq_num;
+ *				rpc_gss_service_t service;
+ *				opaque handle<>;
+ *			} rpc_gss_cred_vers_1_t;
+ *		}
+ *	};
+ */
+static bool
+svcauth_gss_decode_credbody(struct xdr_stream *xdr,
+			    struct rpc_gss_wire_cred *gc,
+			    __be32 **rpcstart)
+{
+	ssize_t handle_len;
+	u32 body_len;
+	__be32 *p;
+
+	p = xdr_inline_decode(xdr, XDR_UNIT);
+	if (!p)
+		return false;
+	/*
+	 * start of rpc packet is 7 u32's back from here:
+	 * xid direction rpcversion prog vers proc flavour
+	 */
+	*rpcstart = p - 7;
+	body_len = be32_to_cpup(p);
+	if (body_len > RPC_MAX_AUTH_SIZE)
+		return false;
+
+	/* struct rpc_gss_cred_t */
+	if (xdr_stream_decode_u32(xdr, &gc->gc_v) < 0)
+		return false;
+	if (xdr_stream_decode_u32(xdr, &gc->gc_proc) < 0)
+		return false;
+	if (xdr_stream_decode_u32(xdr, &gc->gc_seq) < 0)
+		return false;
+	if (xdr_stream_decode_u32(xdr, &gc->gc_svc) < 0)
+		return false;
+	handle_len = xdr_stream_decode_opaque_inline(xdr,
+						     (void **)&gc->gc_ctx.data,
+						     body_len);
+	if (handle_len < 0)
+		return false;
+	if (body_len != XDR_UNIT * 5 + xdr_align_size(handle_len))
+		return false;
+
+	gc->gc_ctx.len = handle_len;
+	return true;
+}
+
+/**
+ * svcauth_gss_accept - Decode and validate incoming RPC_AUTH_GSS credential
+ * @rqstp: RPC transaction
+ *
+ * Return values:
+ *   %SVC_OK: Success
+ *   %SVC_COMPLETE: GSS context lifetime event
+ *   %SVC_DENIED: Credential or verifier is not valid
+ *   %SVC_GARBAGE: Failed to decode credential or verifier
+ *   %SVC_CLOSE: Temporary failure
+ *
+ * The rqstp->rq_auth_stat field is also set (see RFCs 2203 and 5531).
  */
 static int
 svcauth_gss_accept(struct svc_rqst *rqstp)
 {
-	struct kvec	*argv = &rqstp->rq_arg.head[0];
 	struct kvec	*resv = &rqstp->rq_res.head[0];
-	u32		crlen;
 	struct gss_svc_data *svcdata = rqstp->rq_auth_data;
+	__be32		*rpcstart;
 	struct rpc_gss_wire_cred *gc;
 	struct rsc	*rsci = NULL;
-	__be32		*rpcstart;
 	__be32		*reject_stat = resv->iov_base + resv->iov_len;
 	int		ret;
 	struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
 
+	svcxdr_init_decode(rqstp);
+
 	rqstp->rq_auth_stat = rpc_autherr_badcred;
 	if (!svcdata)
 		svcdata = kmalloc(sizeof(*svcdata), GFP_KERNEL);
@@ -1584,31 +1631,10 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 	svcdata->rsci = NULL;
 	gc = &svcdata->clcred;
 
-	/* start of rpc packet is 7 u32's back from here:
-	 * xid direction rpcversion prog vers proc flavour
-	 */
-	rpcstart = argv->iov_base;
-	rpcstart -= 7;
-
-	/* credential is:
-	 *   version(==1), proc(0,1,2,3), seq, service (1,2,3), handle
-	 * at least 5 u32s, and is preceded by length, so that makes 6.
-	 */
-
-	if (argv->iov_len < 5 * 4)
+	if (!svcauth_gss_decode_credbody(&rqstp->rq_arg_stream, gc, &rpcstart))
 		goto auth_err;
-	crlen = svc_getnl(argv);
-	if (svc_getnl(argv) != RPC_GSS_VERSION)
+	if (gc->gc_v != RPC_GSS_VERSION)
 		goto auth_err;
-	gc->gc_proc = svc_getnl(argv);
-	gc->gc_seq = svc_getnl(argv);
-	gc->gc_svc = svc_getnl(argv);
-	if (svc_safe_getnetobj(argv, &gc->gc_ctx))
-		goto auth_err;
-	if (crlen != round_up_to_quad(gc->gc_ctx.len) + 5 * 4)
-		goto auth_err;
-
-	svcxdr_init_decode(rqstp);
 
 	switch (gc->gc_proc) {
 	case RPC_GSS_PROC_INIT:
@@ -1621,7 +1647,6 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 			goto auth_err;
 		fallthrough;
 	case RPC_GSS_PROC_DATA:
-		/* Look up the context, and check the verifier: */
 		rqstp->rq_auth_stat = rpcsec_gsserr_credproblem;
 		rsci = gss_svc_searchbyctx(sn->rsc_cache, &gc->gc_ctx);
 		if (!rsci)

[PATCH v1 20/25] SUNRPC: Hoist init_decode out of svc_authenticate()

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Now that each ->accept method has been converted to use xdr_stream,
the svcxdr_init_decode() calls can be hoisted back up into the
generic RPC server code.

The dprintk in svc_authenticate() is removed, since
trace_svc_authenticate() reports the same information.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |    2 --
 net/sunrpc/svc.c                  |    1 +
 net/sunrpc/svcauth.c              |   13 ++++++++-----
 net/sunrpc/svcauth_unix.c         |    6 ------
 4 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 557de28127fe..2e603358fae1 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -1619,8 +1619,6 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
 	int		ret;
 	struct sunrpc_net *sn = net_generic(SVC_NET(rqstp), sunrpc_net_id);
 
-	svcxdr_init_decode(rqstp);
-
 	rqstp->rq_auth_stat = rpc_autherr_badcred;
 	if (!svcdata)
 		svcdata = kmalloc(sizeof(*svcdata), GFP_KERNEL);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 85f0c3cfc877..e4f8a177763e 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1276,6 +1276,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	 * We do this before anything else in order to get a decent
 	 * auth verifier.
 	 */
+	svcxdr_init_decode(rqstp);
 	auth_res = svc_authenticate(rqstp);
 	/* Also give the program a chance to reject this call: */
 	if (auth_res == SVC_OK && progp)
diff --git a/net/sunrpc/svcauth.c b/net/sunrpc/svcauth.c
index e72ba2f13f6c..67d8245a08af 100644
--- a/net/sunrpc/svcauth.c
+++ b/net/sunrpc/svcauth.c
@@ -63,14 +63,17 @@ svc_put_auth_ops(struct auth_ops *aops)
 int
 svc_authenticate(struct svc_rqst *rqstp)
 {
-	rpc_authflavor_t	flavor;
-	struct auth_ops		*aops;
+	struct auth_ops *aops;
+	u32 flavor;
 
 	rqstp->rq_auth_stat = rpc_auth_ok;
 
-	flavor = svc_getnl(&rqstp->rq_arg.head[0]);
-
-	dprintk("svc: svc_authenticate (%d)\n", flavor);
+	/*
+	 * Decode the Call credential's flavor field. The credential's
+	 * body field is decoded in the chosen ->accept method below.
+	 */
+	if (xdr_stream_decode_u32(&rqstp->rq_arg_stream, &flavor) < 0)
+		return SVC_GARBAGE;
 
 	aops = svc_get_auth_ops(flavor);
 	if (aops == NULL) {
diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index 168e12137754..f09a148aa0c1 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -750,8 +750,6 @@ svcauth_null_accept(struct svc_rqst *rqstp)
 	u32 flavor, len;
 	void *body;
 
-	svcxdr_init_decode(rqstp);
-
 	/* Length of Call's credential body field: */
 	if (xdr_stream_decode_u32(xdr, &len) < 0)
 		return SVC_GARBAGE;
@@ -828,8 +826,6 @@ svcauth_tls_accept(struct svc_rqst *rqstp)
 	u32 flavor, len;
 	void *body;
 
-	svcxdr_init_decode(rqstp);
-
 	/* Length of Call's credential body field: */
 	if (xdr_stream_decode_u32(xdr, &len) < 0)
 		return SVC_GARBAGE;
@@ -905,8 +901,6 @@ svcauth_unix_accept(struct svc_rqst *rqstp)
 	void *body;
 	__be32 *p;
 
-	svcxdr_init_decode(rqstp);
-
 	/*
 	 * This implementation ignores the length of the Call's
 	 * credential body field and the timestamp and machinename

[PATCH v1 21/25] SUNRPC: Re-order construction of the first reply fields

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Clean up: Group these together for legibility.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/svc.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index e4f8a177763e..e473c19b2f8d 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1248,19 +1248,15 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	__set_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);
 	__clear_bit(RQ_DROPME, &rqstp->rq_flags);
 
+	/* Construct the first words of the reply: */
 	svc_putu32(resv, rqstp->rq_xid);
+	svc_putnl(resv, RPC_REPLY);
+	reply_statp = resv->iov_base + resv->iov_len;
 
 	vers = svc_getnl(argv);
-
-	/* First words of reply: */
-	svc_putnl(resv, 1);		/* REPLY */
-
 	if (vers != 2)		/* RPC version number */
 		goto err_bad_rpc;
 
-	/* Save position in case we later decide to reject: */
-	reply_statp = resv->iov_base + resv->iov_len;
-
 	svc_putnl(resv, 0);		/* ACCEPT */
 
 	rqstp->rq_prog = prog = svc_getnl(argv);	/* program number */

[PATCH v1 22/25] SUNRPC: Eliminate unneeded variable

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Clean up: Saving the RPC program number in two places is
unnecessary.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/svc.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index e473c19b2f8d..3c1574f362fe 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1232,7 +1232,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	struct svc_serv		*serv = rqstp->rq_server;
 	struct svc_process_info process;
 	__be32			*statp;
-	u32			prog, vers;
+	u32			vers;
 	__be32			rpc_stat;
 	int			auth_res, rc;
 	__be32			*reply_statp;
@@ -1259,12 +1259,12 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 
 	svc_putnl(resv, 0);		/* ACCEPT */
 
-	rqstp->rq_prog = prog = svc_getnl(argv);	/* program number */
+	rqstp->rq_prog = svc_getnl(argv);	/* program number */
 	rqstp->rq_vers = svc_getnl(argv);	/* version number */
 	rqstp->rq_proc = svc_getnl(argv);	/* procedure number */
 
 	for (progp = serv->sv_program; progp; progp = progp->pg_next)
-		if (prog == progp->pg_prog)
+		if (rqstp->rq_prog == progp->pg_prog)
 			break;
 
 	/*
@@ -1389,7 +1389,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	goto sendit;
 
 err_bad_prog:
-	dprintk("svc: unknown program %d\n", prog);
+	dprintk("svc: unknown program %d\n", rqstp->rq_prog);
 	serv->sv_stats->rpcbadfmt++;
 	svc_putnl(resv, RPC_PROG_UNAVAIL);
 	goto sendit;

[PATCH v1 23/25] SUNRPC: Decode most of RPC header with xdr_stream

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Done as part of hardening the server-side RPC header decoding path.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/svc.c |   20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 3c1574f362fe..f292b898f200 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1231,17 +1231,13 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	const struct svc_procedure *procp = NULL;
 	struct svc_serv		*serv = rqstp->rq_server;
 	struct svc_process_info process;
-	__be32			*statp;
-	u32			vers;
+	__be32			*p, *statp;
 	__be32			rpc_stat;
 	int			auth_res, rc;
 	__be32			*reply_statp;
 
 	rpc_stat = rpc_success;
 
-	if (argv->iov_len < 6*4)
-		goto err_short_len;
-
 	/* Will be turned off by GSS integrity and privacy services */
 	__set_bit(RQ_SPLICE_OK, &rqstp->rq_flags);
 	/* Will be turned off only when NFSv4 Sessions are used */
@@ -1253,15 +1249,18 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	svc_putnl(resv, RPC_REPLY);
 	reply_statp = resv->iov_base + resv->iov_len;
 
-	vers = svc_getnl(argv);
-	if (vers != 2)		/* RPC version number */
+	svcxdr_init_decode(rqstp);
+	p = xdr_inline_decode(&rqstp->rq_arg_stream, XDR_UNIT * 4);
+	if (unlikely(!p))
+		goto err_short_len;
+	if (*p++ != cpu_to_be32(RPC_VERSION))
 		goto err_bad_rpc;
 
 	svc_putnl(resv, 0);		/* ACCEPT */
 
-	rqstp->rq_prog = svc_getnl(argv);	/* program number */
-	rqstp->rq_vers = svc_getnl(argv);	/* version number */
-	rqstp->rq_proc = svc_getnl(argv);	/* procedure number */
+	rqstp->rq_prog = be32_to_cpup(p++);
+	rqstp->rq_vers = be32_to_cpup(p++);
+	rqstp->rq_proc = be32_to_cpup(p);
 
 	for (progp = serv->sv_program; progp; progp = progp->pg_next)
 		if (rqstp->rq_prog == progp->pg_prog)
@@ -1272,7 +1271,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	 * We do this before anything else in order to get a decent
 	 * auth verifier.
 	 */
-	svcxdr_init_decode(rqstp);
 	auth_res = svc_authenticate(rqstp);
 	/* Also give the program a chance to reject this call: */
 	if (auth_res == SVC_OK && progp)

[PATCH v1 24/25] SUNRPC: Remove svc_process_common's argv parameter

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Clean up: With xdr_stream decoding, the @argv parameter is no longer
used.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/svc.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index f292b898f200..571151a17e87 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1225,7 +1225,7 @@ EXPORT_SYMBOL_GPL(svc_generic_init_request);
  * Common routine for processing the RPC request.
  */
 static int
-svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
+svc_process_common(struct svc_rqst *rqstp, struct kvec *resv)
 {
 	struct svc_program	*progp;
 	const struct svc_procedure *procp = NULL;
@@ -1363,8 +1363,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv)
 	return 0;
 
 err_short_len:
-	svc_printk(rqstp, "short len %zd, dropping request\n",
-			argv->iov_len);
+	svc_printk(rqstp, "short len %u, dropping request\n",
+		   rqstp->rq_arg.len);
 	goto close_xprt;
 
 err_bad_rpc:
@@ -1453,7 +1453,7 @@ svc_process(struct svc_rqst *rqstp)
 	dir = svc_getu32(argv);
 	if (dir != rpc_call)
 		goto out_baddir;
-	if (!svc_process_common(rqstp, argv, resv))
+	if (!svc_process_common(rqstp, resv))
 		goto out_drop;
 	return svc_send(rqstp);
 
@@ -1519,7 +1519,7 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
 	svc_getnl(argv);	/* CALLDIR */
 
 	/* Parse and execute the bc call */
-	proc_error = svc_process_common(rqstp, argv, resv);
+	proc_error = svc_process_common(rqstp, resv);
 
 	atomic_dec(&req->rq_xprt->bc_slot_count);
 	if (!proc_error) {

[PATCH v1 25/25] SUNRPC: Hoist svcxdr_init_decode() into svc_process()

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Now the entire RPC Call header parsing path is handled via struct
xdr_stream-based decoders.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/svc.c      |   28 +++++++++++++++++-----------
 net/sunrpc/svc_xprt.c |    1 -
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 571151a17e87..94f7efca60fc 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1249,7 +1249,6 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *resv)
 	svc_putnl(resv, RPC_REPLY);
 	reply_statp = resv->iov_base + resv->iov_len;
 
-	svcxdr_init_decode(rqstp);
 	p = xdr_inline_decode(&rqstp->rq_arg_stream, XDR_UNIT * 4);
 	if (unlikely(!p))
 		goto err_short_len;
@@ -1425,9 +1424,8 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *resv)
 int
 svc_process(struct svc_rqst *rqstp)
 {
-	struct kvec		*argv = &rqstp->rq_arg.head[0];
 	struct kvec		*resv = &rqstp->rq_res.head[0];
-	__be32			dir;
+	__be32 *p;
 
 #if IS_ENABLED(CONFIG_FAIL_SUNRPC)
 	if (!fail_sunrpc.ignore_server_disconnect &&
@@ -1450,16 +1448,21 @@ svc_process(struct svc_rqst *rqstp)
 	rqstp->rq_res.tail[0].iov_base = NULL;
 	rqstp->rq_res.tail[0].iov_len = 0;
 
-	dir = svc_getu32(argv);
-	if (dir != rpc_call)
+	svcxdr_init_decode(rqstp);
+	p = xdr_inline_decode(&rqstp->rq_arg_stream, XDR_UNIT * 2);
+	if (unlikely(!p))
+		goto out_drop;
+	rqstp->rq_xid = *p++;
+	if (unlikely(*p != rpc_call))
 		goto out_baddir;
+
 	if (!svc_process_common(rqstp, resv))
 		goto out_drop;
 	return svc_send(rqstp);
 
 out_baddir:
 	svc_printk(rqstp, "bad direction 0x%08x, dropping request\n",
-		   be32_to_cpu(dir));
+		   be32_to_cpu(*p));
 	rqstp->rq_server->sv_stats->rpcbadfmt++;
 out_drop:
 	svc_drop(rqstp);
@@ -1476,7 +1479,6 @@ int
 bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
 	       struct svc_rqst *rqstp)
 {
-	struct kvec	*argv = &rqstp->rq_arg.head[0];
 	struct kvec	*resv = &rqstp->rq_res.head[0];
 	struct rpc_task *task;
 	int proc_error;
@@ -1511,12 +1513,16 @@ bc_svc_process(struct svc_serv *serv, struct rpc_rqst *req,
 	/* reset result send buffer "put" position */
 	resv->iov_len = 0;
 
+	svcxdr_init_decode(rqstp);
+
 	/*
-	 * Skip the next two words because they've already been
-	 * processed in the transport
+	 * Skip the XID and calldir fields because they've already
+	 * been processed by the caller.
 	 */
-	svc_getu32(argv);	/* XID */
-	svc_getnl(argv);	/* CALLDIR */
+	if (!xdr_inline_decode(&rqstp->rq_arg_stream, XDR_UNIT * 2)) {
+		error = -EINVAL;
+		goto out;
+	}
 
 	/* Parse and execute the bc call */
 	proc_error = svc_process_common(rqstp, resv);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 2106003645a7..b4697984b4b2 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -890,7 +890,6 @@ int svc_recv(struct svc_rqst *rqstp, long timeout)
 
 	xprt->xpt_ops->xpo_secure_port(rqstp);
 	rqstp->rq_chandle.defer = svc_defer;
-	rqstp->rq_xid = svc_getu32(&rqstp->rq_arg.head[0]);
 
 	if (serv->sv_stats)
 		serv->sv_stats->netcnt++;

Re: [PATCH v1 00/25] Server-side RPC call header parsing overhaul

[Jeff Layton]

On Mon, 2023-01-02 at 12:05 -0500, Chuck Lever wrote:
> Happy new year, campers.
> 
> The following series has been percolating for quite a while, thanks
> to the extended 6.1-rc cycle. I'd like to get this work reviewed
> for possible inclusion in v6.3, so I'm starting early.
> 
> The purpose of this series is to replace the svc_get* macros in the
> Linux kernel server's RPC call header parsing code with xdr_stream
> helpers. I've measured no change in CPU utilization after the
> overhaul; svc_recv() and friends remain the highest CPU consumers by
> an order of magnitude.
> 
> Memory safety: Buffer bounds checking after decoding each XDR item
> is more memory-safe than the current decoding mechanism. Subsequent
> memory safety improvements to the xdr_stream helpers will benefit
> all who use them.
> 
> Audit friendliness: The new code has lots of comments and other
> clean-up to help align it with the relevant RPC specifications. The
> use of common helpers also makes the decoders easier to audit.
> 
> I've split the full series in half to make it easier to review. The
> patches posted here handle RPC call header decoding. Remaining
> patches, to be posted later, deal with RPC reply header encoding.
> 
> Yes, there are a lot of patches, but they are each small, easily
> chewed mechanical changes.
> 
> ---
> 
> Chuck Lever (25):
>       SUNRPC: Push svcxdr_init_decode() into svc_process_common()
>       SUNRPC: Move svcxdr_init_decode() into ->accept methods
>       SUNRPC: Add an XDR decoding helper for struct opaque_auth
>       SUNRPC: Convert svcauth_null_accept() to use xdr_stream
>       SUNRPC: Convert svcauth_unix_accept() to use xdr_stream
>       SUNRPC: Convert svcauth_tls_accept() to use xdr_stream
>       SUNRPC: Move the server-side GSS upcall to a noinline function
>       SUNRPC: Hoist common verifier decoding code into svcauth_gss_proc_init()
>       SUNRPC: Remove gss_read_common_verf()
>       SUNRPC: Remove gss_read_verf()
>       SUNRPC: Convert server-side GSS upcall helpers to use xdr_stream
>       SUNRPC: Replace read_u32_from_xdr_buf() with existing XDR helper
>       SUNRPC: Rename automatic variables in unwrap_integ_data()
>       SUNRPC: Convert unwrap_integ_data() to use xdr_stream
>       SUNRPC: Rename automatic variables in unwrap_priv_data()
>       SUNRPC: Convert unwrap_priv_data() to use xdr_stream
>       SUNRPC: Convert gss_verify_header() to use xdr_stream
>       SUNRPC: Clean up svcauth_gss_accept's NULL procedure check
>       SUNRPC: Convert the svcauth_gss_accept() pre-amble to use xdr_stream
>       SUNRPC: Hoist init_decode out of svc_authenticate()
>       SUNRPC: Re-order construction of the first reply fields
>       SUNRPC: Eliminate unneeded variable
>       SUNRPC: Decode most of RPC header with xdr_stream
>       SUNRPC: Remove svc_process_common's argv parameter
>       SUNRPC: Hoist svcxdr_init_decode() into svc_process()
> 
> 
>  fs/lockd/svc.c                    |   1 -
>  fs/nfs/callback_xdr.c             |   1 -
>  fs/nfsd/nfssvc.c                  |   1 -
>  include/linux/sunrpc/msg_prot.h   |   5 +
>  include/linux/sunrpc/xdr.h        |   5 +-
>  net/sunrpc/auth_gss/svcauth_gss.c | 512 ++++++++++++++++--------------
>  net/sunrpc/svc.c                  |  69 ++--
>  net/sunrpc/svc_xprt.c             |   1 -
>  net/sunrpc/svcauth.c              |  13 +-
>  net/sunrpc/svcauth_unix.c         | 132 +++++---
>  net/sunrpc/xdr.c                  |  50 ++-
>  11 files changed, 468 insertions(+), 322 deletions(-)
> 
> --
> Chuck Lever
> 

I went through the whole set and it's all a mostly logical set of
methodical changes. You can add:

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH v1 00/25] Server-side RPC call header parsing overhaul

[Chuck Lever III]

> On Jan 3, 2023, at 9:52 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Mon, 2023-01-02 at 12:05 -0500, Chuck Lever wrote:
>> Happy new year, campers.
>> 
>> The following series has been percolating for quite a while, thanks
>> to the extended 6.1-rc cycle. I'd like to get this work reviewed
>> for possible inclusion in v6.3, so I'm starting early.
>> 
>> The purpose of this series is to replace the svc_get* macros in the
>> Linux kernel server's RPC call header parsing code with xdr_stream
>> helpers. I've measured no change in CPU utilization after the
>> overhaul; svc_recv() and friends remain the highest CPU consumers by
>> an order of magnitude.
>> 
>> Memory safety: Buffer bounds checking after decoding each XDR item
>> is more memory-safe than the current decoding mechanism. Subsequent
>> memory safety improvements to the xdr_stream helpers will benefit
>> all who use them.
>> 
>> Audit friendliness: The new code has lots of comments and other
>> clean-up to help align it with the relevant RPC specifications. The
>> use of common helpers also makes the decoders easier to audit.
>> 
>> I've split the full series in half to make it easier to review. The
>> patches posted here handle RPC call header decoding. Remaining
>> patches, to be posted later, deal with RPC reply header encoding.
>> 
>> Yes, there are a lot of patches, but they are each small, easily
>> chewed mechanical changes.
>> 
>> ---
>> 
>> Chuck Lever (25):
>>      SUNRPC: Push svcxdr_init_decode() into svc_process_common()
>>      SUNRPC: Move svcxdr_init_decode() into ->accept methods
>>      SUNRPC: Add an XDR decoding helper for struct opaque_auth
>>      SUNRPC: Convert svcauth_null_accept() to use xdr_stream
>>      SUNRPC: Convert svcauth_unix_accept() to use xdr_stream
>>      SUNRPC: Convert svcauth_tls_accept() to use xdr_stream
>>      SUNRPC: Move the server-side GSS upcall to a noinline function
>>      SUNRPC: Hoist common verifier decoding code into svcauth_gss_proc_init()
>>      SUNRPC: Remove gss_read_common_verf()
>>      SUNRPC: Remove gss_read_verf()
>>      SUNRPC: Convert server-side GSS upcall helpers to use xdr_stream
>>      SUNRPC: Replace read_u32_from_xdr_buf() with existing XDR helper
>>      SUNRPC: Rename automatic variables in unwrap_integ_data()
>>      SUNRPC: Convert unwrap_integ_data() to use xdr_stream
>>      SUNRPC: Rename automatic variables in unwrap_priv_data()
>>      SUNRPC: Convert unwrap_priv_data() to use xdr_stream
>>      SUNRPC: Convert gss_verify_header() to use xdr_stream
>>      SUNRPC: Clean up svcauth_gss_accept's NULL procedure check
>>      SUNRPC: Convert the svcauth_gss_accept() pre-amble to use xdr_stream
>>      SUNRPC: Hoist init_decode out of svc_authenticate()
>>      SUNRPC: Re-order construction of the first reply fields
>>      SUNRPC: Eliminate unneeded variable
>>      SUNRPC: Decode most of RPC header with xdr_stream
>>      SUNRPC: Remove svc_process_common's argv parameter
>>      SUNRPC: Hoist svcxdr_init_decode() into svc_process()
>> 
>> 
>> fs/lockd/svc.c                    |   1 -
>> fs/nfs/callback_xdr.c             |   1 -
>> fs/nfsd/nfssvc.c                  |   1 -
>> include/linux/sunrpc/msg_prot.h   |   5 +
>> include/linux/sunrpc/xdr.h        |   5 +-
>> net/sunrpc/auth_gss/svcauth_gss.c | 512 ++++++++++++++++--------------
>> net/sunrpc/svc.c                  |  69 ++--
>> net/sunrpc/svc_xprt.c             |   1 -
>> net/sunrpc/svcauth.c              |  13 +-
>> net/sunrpc/svcauth_unix.c         | 132 +++++---
>> net/sunrpc/xdr.c                  |  50 ++-
>> 11 files changed, 468 insertions(+), 322 deletions(-)
>> 
>> --
>> Chuck Lever
>> 
> 
> I went through the whole set and it's all a mostly logical set of
> methodical changes. You can add:
> 
> Reviewed-by: Jeff Layton <jlayton@kernel.org>

Thanks Jeff! I've applied these and pushed them to nfsd's for-next,
but comments are still open, and testing results are welcome.


--
Chuck Lever