[PATCH 00/10] Update rpcbind client's XDR functions

[PATCH 00/10] Update rpcbind client's XDR functions

[Chuck Lever]

Hi Trond-

Here, I update the kernel rpcbind client's XDR functions to use the
NFSv4-style encoding and decoding, use the new universal address API
added in the previous series, and introduce support for using the
whole address returned by RPCB_GETADDR instead of just the port
number.

This series of patches is for 2.6.32, and depends on the last series I
just sent.  These patches have seen light testing here, and are also
available in the cel-2.6.git linux.nfs.org repository under
the cel-ipv6-07152009 tag.

The last patch in this series is a one-off, not dependent on any of
the others.

---

Chuck Lever (10):
      SUNRPC: Add documenting comments in net/sunrpc/timer.c
      SUNRPC: Use address returned by rpcbind when connecting
      SUNRPC: Rename sock_xprt.addr as sock_xprt.srcaddr
      SUNRPC: Pass full bind address to transports after GETPORT/GETADDR
      SUNRPC: Eliminate PROC macro from rpcb_clnt
      SUNRPC: Clean up: Remove unused XDR decoder functions from rpcb_clnt.c
      SUNRPC: Introduce new xdr_stream-based decoders to rpcb_clnt.c
      SUNRPC: Introduce xdr_stream-based decoders for RPCB_UNSET
      SUNRPC: Clean up: Remove unused XDR encoder functions from rpcb_clnt.c
      SUNRPC: Introduce new xdr_stream-based encoders to rpcb_clnt.c


 include/linux/sunrpc/xprt.h     |    4 
 net/sunrpc/rpcb_clnt.c          |  384 ++++++++++++++++++++++++++++-----------
 net/sunrpc/timer.c              |   45 +++--
 net/sunrpc/xprtrdma/transport.c |   23 ++
 net/sunrpc/xprtsock.c           |  173 ++++++++++++------
 5 files changed, 451 insertions(+), 178 deletions(-)

-- 
Chuck Lever <chuck.lever@oracle.com>

[PATCH 01/10] SUNRPC: Introduce new xdr_stream-based encoders to rpcb_clnt.c

[Chuck Lever]

Replace the open-coded encode logic for rpcbind arguments with an
xdr_stream-based implementation, similar to what NFSv4 uses, to
better protect against buffer overflows.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/rpcb_clnt.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 75 insertions(+), 1 deletions(-)

diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index 1fb1c07..2e8be8f 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -708,6 +708,30 @@ static int rpcb_encode_mapping(struct rpc_rqst *req, __be32 *p,
 	return 0;
 }
 
+static int rpcb_enc_mapping(struct rpc_rqst *req, __be32 *p,
+			    const struct rpcbind_args *rpcb)
+{
+	struct rpc_task *task = req->rq_task;
+	struct xdr_stream xdr;
+
+	dprintk("RPC: %5u encoding PMAP_%s call (%u, %u, %d, %u)\n",
+			task->tk_pid, task->tk_msg.rpc_proc->p_name,
+			rpcb->r_prog, rpcb->r_vers, rpcb->r_prot, rpcb->r_port);
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+
+	p = xdr_reserve_space(&xdr, sizeof(u32) * RPCB_mappingargs_sz);
+	if (unlikely(p == NULL))
+		return -EIO;
+
+	*p++ = htonl(rpcb->r_prog);
+	*p++ = htonl(rpcb->r_vers);
+	*p++ = htonl(rpcb->r_prot);
+	*p   = htonl(rpcb->r_port);
+
+	return 0;
+}
+
 static int rpcb_decode_getport(struct rpc_rqst *req, __be32 *p,
 			       unsigned short *portp)
 {
@@ -746,6 +770,56 @@ static int rpcb_encode_getaddr(struct rpc_rqst *req, __be32 *p,
 	return 0;
 }
 
+static int encode_rpcb_string(struct xdr_stream *xdr, const char *string,
+				const u32 maxstrlen)
+{
+	u32 len;
+	__be32 *p;
+
+	if (unlikely(string == NULL))
+		return -EIO;
+	len = strlen(string);
+	if (unlikely(len > maxstrlen))
+		return -EIO;
+
+	p = xdr_reserve_space(xdr, sizeof(u32) + len);
+	if (unlikely(p == NULL))
+		return -EIO;
+	xdr_encode_opaque(p, string, len);
+
+	return 0;
+}
+
+static int rpcb_enc_getaddr(struct rpc_rqst *req, __be32 *p,
+			    const struct rpcbind_args *rpcb)
+{
+	struct rpc_task *task = req->rq_task;
+	struct xdr_stream xdr;
+
+	dprintk("RPC: %5u encoding RPCB_%s call (%u, %u, '%s', '%s')\n",
+			task->tk_pid, task->tk_msg.rpc_proc->p_name,
+			rpcb->r_prog, rpcb->r_vers,
+			rpcb->r_netid, rpcb->r_addr);
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+
+	p = xdr_reserve_space(&xdr,
+			sizeof(u32) * (RPCB_program_sz + RPCB_version_sz));
+	if (unlikely(p == NULL))
+		return -EIO;
+	*p++ = htonl(rpcb->r_prog);
+	*p = htonl(rpcb->r_vers);
+
+	if (encode_rpcb_string(&xdr, rpcb->r_netid, RPCBIND_MAXNETIDLEN))
+		return -EIO;
+	if (encode_rpcb_string(&xdr, rpcb->r_addr, RPCBIND_MAXUADDRLEN))
+		return -EIO;
+	if (encode_rpcb_string(&xdr, rpcb->r_owner, RPCB_MAXOWNERLEN))
+		return -EIO;
+
+	return 0;
+}
+
 static int rpcb_decode_getaddr(struct rpc_rqst *req, __be32 *p,
 			       unsigned short *portp)
 {
@@ -812,7 +886,7 @@ out_err:
 #define PROC(proc, argtype, restype)					\
 	[RPCBPROC_##proc] = {						\
 		.p_proc		= RPCBPROC_##proc,			\
-		.p_encode	= (kxdrproc_t) rpcb_encode_##argtype,	\
+		.p_encode	= (kxdrproc_t) rpcb_enc_##argtype,	\
 		.p_decode	= (kxdrproc_t) rpcb_decode_##restype,	\
 		.p_arglen	= RPCB_##argtype##args_sz,		\
 		.p_replen	= RPCB_##restype##res_sz,		\

[PATCH 02/10] SUNRPC: Clean up: Remove unused XDR encoder functions from rpcb_clnt.c

[Chuck Lever]

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/rpcb_clnt.c |   34 ----------------------------------
 1 files changed, 0 insertions(+), 34 deletions(-)

diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index 2e8be8f..8da6a9b 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -694,20 +694,6 @@ static void rpcb_getport_done(struct rpc_task *child, void *data)
  * XDR functions for rpcbind
  */
 
-static int rpcb_encode_mapping(struct rpc_rqst *req, __be32 *p,
-			       struct rpcbind_args *rpcb)
-{
-	dprintk("RPC:       encoding rpcb request (%u, %u, %d, %u)\n",
-			rpcb->r_prog, rpcb->r_vers, rpcb->r_prot, rpcb->r_port);
-	*p++ = htonl(rpcb->r_prog);
-	*p++ = htonl(rpcb->r_vers);
-	*p++ = htonl(rpcb->r_prot);
-	*p++ = htonl(rpcb->r_port);
-
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
 static int rpcb_enc_mapping(struct rpc_rqst *req, __be32 *p,
 			    const struct rpcbind_args *rpcb)
 {
@@ -750,26 +736,6 @@ static int rpcb_decode_set(struct rpc_rqst *req, __be32 *p,
 	return 0;
 }
 
-static int rpcb_encode_getaddr(struct rpc_rqst *req, __be32 *p,
-			       struct rpcbind_args *rpcb)
-{
-	if (rpcb->r_addr == NULL)
-		return -EIO;
-
-	dprintk("RPC:       encoding rpcb request (%u, %u, %s)\n",
-			rpcb->r_prog, rpcb->r_vers, rpcb->r_addr);
-	*p++ = htonl(rpcb->r_prog);
-	*p++ = htonl(rpcb->r_vers);
-
-	p = xdr_encode_string(p, rpcb->r_netid);
-	p = xdr_encode_string(p, rpcb->r_addr);
-	p = xdr_encode_string(p, rpcb->r_owner);
-
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-
-	return 0;
-}
-
 static int encode_rpcb_string(struct xdr_stream *xdr, const char *string,
 				const u32 maxstrlen)
 {

[PATCH 03/10] SUNRPC: Introduce xdr_stream-based decoders for RPCB_UNSET

[Chuck Lever]

Replace the open-coded decode logic for rpcbind UNSET results with an
xdr_stream-based implementation, similar to what NFSv4 uses, to
protect against buffer overflows.

The new function is unused for the moment.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/rpcb_clnt.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index 8da6a9b..d52bb0f 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -736,6 +736,28 @@ static int rpcb_decode_set(struct rpc_rqst *req, __be32 *p,
 	return 0;
 }
 
+static int rpcb_dec_set(struct rpc_rqst *req, __be32 *p,
+			unsigned int *boolp)
+{
+	struct rpc_task *task = req->rq_task;
+	struct xdr_stream xdr;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+
+	p = xdr_inline_decode(&xdr, sizeof(u32));
+	if (unlikely(p == NULL))
+		return -EIO;
+
+	*boolp = 0;
+	if (*p)
+		*boolp = 1;
+
+	dprintk("RPC: %5u RPCB_%s call %s\n",
+			task->tk_pid, task->tk_msg.rpc_proc->p_name,
+			(*boolp ? "succeeded" : "failed"));
+	return 0;
+}
+
 static int encode_rpcb_string(struct xdr_stream *xdr, const char *string,
 				const u32 maxstrlen)
 {

[PATCH 04/10] SUNRPC: Introduce new xdr_stream-based decoders to rpcb_clnt.c

[Chuck Lever]

Replace the open-coded decode logic for PMAP_GETPORT/RPCB_GETADDR with
an xdr_stream-based implementation, similar to what NFSv4 uses, to
protect against buffer overflows.  The new implementation also checks
that the incoming port number is reasonable.

In the age of TI-RPC, rpcbind RPCB_GETADDR replies return a full
address to which the client should bind.  Introduce support in the
PMAP_GETPORT and RPCB_GETADDR XDR decoders for handing back a
full address for successful rpcbind operations.  Subsequent patches
will pass this address to the parent transport's ->set_port method.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/rpcb_clnt.c |  150 +++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 142 insertions(+), 8 deletions(-)

diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index d52bb0f..91af1f8 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -14,6 +14,7 @@
 
 #include <linux/module.h>
 
+#include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/socket.h>
 #include <linux/in.h>
@@ -122,6 +123,8 @@ struct rpcbind_args {
 	const char *		r_owner;
 
 	int			r_status;
+	struct sockaddr_storage	r_raddr;
+	size_t			r_raddrlen;
 };
 
 static struct rpc_procinfo rpcb_procedures2[];
@@ -157,6 +160,18 @@ static void rpcb_map_release(void *data)
 	kfree(map);
 }
 
+static const struct sockaddr rpcb_inaddr_unspec = {
+	.sa_family		= AF_UNSPEC,
+};
+
+static void rpcb_set_unspec_raddr(struct rpcbind_args *rpcb)
+{
+	struct sockaddr *sap = (struct sockaddr *)&rpcb->r_raddr;
+
+	rpcb->r_raddrlen = sizeof(rpcb_inaddr_unspec);
+	memcpy(sap, &rpcb_inaddr_unspec, rpcb->r_raddrlen);
+}
+
 static const struct sockaddr_in rpcb_inaddr_loopback = {
 	.sin_family		= AF_INET,
 	.sin_addr.s_addr	= htonl(INADDR_LOOPBACK),
@@ -454,7 +469,7 @@ int rpcb_getport_sync(struct sockaddr_in *sin, u32 prog, u32 vers, int prot)
 	struct rpc_message msg = {
 		.rpc_proc	= &rpcb_procedures2[RPCBPROC_GETPORT],
 		.rpc_argp	= &map,
-		.rpc_resp	= &map.r_port,
+		.rpc_resp	= &map,
 	};
 	struct rpc_clnt	*rpcb_clnt;
 	int status;
@@ -467,12 +482,14 @@ int rpcb_getport_sync(struct sockaddr_in *sin, u32 prog, u32 vers, int prot)
 	if (IS_ERR(rpcb_clnt))
 		return PTR_ERR(rpcb_clnt);
 
+	memcpy(&map.r_raddr, sin, sizeof(*sin));
 	status = rpc_call_sync(rpcb_clnt, &msg, 0);
 	rpc_shutdown_client(rpcb_clnt);
 
 	if (status >= 0) {
-		if (map.r_port != 0)
-			return map.r_port;
+		sin = (struct sockaddr_in *)&map.r_raddr;
+		if (sin->sin_family == AF_INET)
+			return ntohs(sin->sin_port);
 		status = -EACCES;
 	}
 	return status;
@@ -484,7 +501,7 @@ static struct rpc_task *rpcb_call_async(struct rpc_clnt *rpcb_clnt, struct rpcbi
 	struct rpc_message msg = {
 		.rpc_proc = proc,
 		.rpc_argp = map,
-		.rpc_resp = &map->r_port,
+		.rpc_resp = map,
 	};
 	struct rpc_task_setup task_setup_data = {
 		.rpc_client = rpcb_clnt,
@@ -627,6 +644,8 @@ void rpcb_getport_async(struct rpc_task *task)
 		break;
 	case RPCBVERS_2:
 		map->r_addr = NULL;
+		memcpy(&map->r_raddr, sap, salen);
+		map->r_raddrlen = salen;
 		break;
 	default:
 		BUG();
@@ -659,8 +678,10 @@ EXPORT_SYMBOL_GPL(rpcb_getport_async);
 static void rpcb_getport_done(struct rpc_task *child, void *data)
 {
 	struct rpcbind_args *map = data;
+	struct sockaddr *sap = (struct sockaddr *)&map->r_raddr;
 	struct rpc_xprt *xprt = map->r_xprt;
 	int status = child->tk_status;
+	unsigned short port = 0;
 
 	/* Garbage reply: retry with a lesser rpcbind version */
 	if (status == -EIO)
@@ -673,19 +694,30 @@ static void rpcb_getport_done(struct rpc_task *child, void *data)
 	if (status < 0) {
 		/* rpcbind server not available on remote host? */
 		xprt->ops->set_port(xprt, 0);
-	} else if (map->r_port == 0) {
+		xprt_clear_bound(xprt);
+	} else if (sap->sa_family == AF_UNSPEC) {
 		/* Requested RPC service wasn't registered on remote host */
 		xprt->ops->set_port(xprt, 0);
+		xprt_clear_bound(xprt);
 		status = -EACCES;
 	} else {
 		/* Succeeded */
-		xprt->ops->set_port(xprt, map->r_port);
+		switch (sap->sa_family) {
+		case AF_INET:
+			port = ntohs(((struct sockaddr_in *)sap)->sin_port);
+			break;
+		case AF_INET6:
+			port = ntohs(((struct sockaddr_in6 *)sap)->sin6_port);
+			break;
+		}
+
+		xprt->ops->set_port(xprt, port);
 		xprt_set_bound(xprt);
 		status = 0;
 	}
 
 	dprintk("RPC: %5u rpcb_getport_done(status %d, port %u)\n",
-			child->tk_pid, status, map->r_port);
+			child->tk_pid, status, port);
 
 	map->r_status = status;
 }
@@ -727,6 +759,37 @@ static int rpcb_decode_getport(struct rpc_rqst *req, __be32 *p,
 	return 0;
 }
 
+static int rpcb_dec_getport(struct rpc_rqst *req, __be32 *p,
+			    struct rpcbind_args *rpcb)
+{
+	struct sockaddr *sap = (struct sockaddr *)&rpcb->r_raddr;
+	struct rpc_task *task = req->rq_task;
+	struct xdr_stream xdr;
+	unsigned long port;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+
+	p = xdr_inline_decode(&xdr, sizeof(u32));
+	if (unlikely(p == NULL))
+		return -EIO;
+	port = ntohl(*p);
+	dprintk("RPC: %5u PMAP_%s result: %lu\n", task->tk_pid,
+			task->tk_msg.rpc_proc->p_name, port);
+
+	if (unlikely(port > USHORT_MAX))
+		return -EIO;
+
+	if (sap->sa_family != AF_INET)
+		return -EIO;
+
+	if (port == 0)
+		rpcb_set_unspec_raddr(rpcb);
+	else
+		((struct sockaddr_in *)sap)->sin_port = htons(port);
+
+	return 0;
+}
+
 static int rpcb_decode_set(struct rpc_rqst *req, __be32 *p,
 			   unsigned int *boolp)
 {
@@ -871,11 +934,82 @@ out_err:
 	return -EIO;
 }
 
+static int rpcb_dec_getaddr(struct rpc_rqst *req, __be32 *p,
+			    struct rpcbind_args *rpcb)
+{
+	struct sockaddr *sap = (struct sockaddr *)&rpcb->r_raddr;
+	size_t salen = sizeof(rpcb->r_raddr);
+	struct rpc_task *task = req->rq_task;
+	struct sockaddr_in6 sin6;
+	struct xdr_stream xdr;
+	u32 len;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+
+	p = xdr_inline_decode(&xdr, sizeof(u32));
+	if (unlikely(p == NULL))
+		goto out_fail;
+	len = ntohl(*p);
+
+	/*
+	 * Special case: if the returned universal address is a null
+	 * string, the requested RPC service was not registered.
+	 */
+	if (len == 0) {
+		rpcb_set_unspec_raddr(rpcb);
+		dprintk("RPC:       RPCB reply: program not registered\n");
+		return 0;
+	}
+
+	if (unlikely(len > RPCBIND_MAXUADDRLEN))
+		goto out_fail;
+
+	p = xdr_inline_decode(&xdr, len);
+	if (unlikely(p == NULL))
+		goto out_fail;
+
+	/*
+	 * If both the parent's destination address and the returned
+	 * address are site-local, or both are link-local, copy the
+	 * scope ID from the parent's address.  Universal addresses
+	 * do not support scope IDs.
+	 */
+	if (sap->sa_family == AF_INET6)
+		memcpy(&sin6, sap, sizeof(sin6));
+
+	salen = rpc_uaddr2sockaddr((char *)p, len, sap, salen);
+	if (unlikely(salen == 0))
+		goto out_fail;
+	rpcb->r_raddrlen = salen;
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	/* NB: sap now contains the returned address */
+	if (sap->sa_family == AF_INET6) {
+		struct sockaddr_in6 *recvd = (struct sockaddr_in6 *)sap;
+
+		if ((ipv6_addr_type(&sin6.sin6_addr) & IPV6_ADDR_LINKLOCAL &&
+		    ipv6_addr_type(&recvd->sin6_addr) & IPV6_ADDR_LINKLOCAL) ||
+		    (ipv6_addr_type(&sin6.sin6_addr) & IPV6_ADDR_SITELOCAL &&
+		    ipv6_addr_type(&recvd->sin6_addr) & IPV6_ADDR_SITELOCAL))
+			recvd->sin6_scope_id = sin6.sin6_scope_id;
+	}
+#endif
+
+	dprintk("RPC: %5u RPCB_%s reply: %s\n", task->tk_pid,
+			task->tk_msg.rpc_proc->p_name, (char *)p);
+	return 0;
+
+out_fail:
+	dprintk("RPC: %5u malformed RPCB_%s reply\n",
+			task->tk_pid, task->tk_msg.rpc_proc->p_name);
+	return -EIO;
+}
+
 #define PROC(proc, argtype, restype)					\
 	[RPCBPROC_##proc] = {						\
 		.p_proc		= RPCBPROC_##proc,			\
 		.p_encode	= (kxdrproc_t) rpcb_enc_##argtype,	\
-		.p_decode	= (kxdrproc_t) rpcb_decode_##restype,	\
+		.p_decode	= (kxdrproc_t) rpcb_dec_##restype,	\
 		.p_arglen	= RPCB_##argtype##args_sz,		\
 		.p_replen	= RPCB_##restype##res_sz,		\
 		.p_statidx	= RPCBPROC_##proc,			\

Re: [PATCH 04/10] SUNRPC: Introduce new xdr_stream-based decoders to rpcb_clnt.c

[Trond Myklebust]

On Wed, 2009-07-15 at 17:42 -0400, Chuck Lever wrote:
> Replace the open-coded decode logic for PMAP_GETPORT/RPCB_GETADDR with
> an xdr_stream-based implementation, similar to what NFSv4 uses, to
> protect against buffer overflows.  The new implementation also checks
> that the incoming port number is reasonable.
> 
> In the age of TI-RPC, rpcbind RPCB_GETADDR replies return a full
> address to which the client should bind.  Introduce support in the
> PMAP_GETPORT and RPCB_GETADDR XDR decoders for handing back a
> full address for successful rpcbind operations.  Subsequent patches
> will pass this address to the parent transport's ->set_port method.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  net/sunrpc/rpcb_clnt.c |  150 +++++++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 142 insertions(+), 8 deletions(-)
> 
> diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
> index d52bb0f..91af1f8 100644
> --- a/net/sunrpc/rpcb_clnt.c
> +++ b/net/sunrpc/rpcb_clnt.c
> @@ -14,6 +14,7 @@
>  
>  #include <linux/module.h>
>  
> +#include <linux/kernel.h>
>  #include <linux/types.h>
>  #include <linux/socket.h>
>  #include <linux/in.h>
> @@ -122,6 +123,8 @@ struct rpcbind_args {
>  	const char *		r_owner;
>  
>  	int			r_status;
> +	struct sockaddr_storage	r_raddr;
> +	size_t			r_raddrlen;
>  };
>  
>  static struct rpc_procinfo rpcb_procedures2[];
> @@ -157,6 +160,18 @@ static void rpcb_map_release(void *data)
>  	kfree(map);
>  }
>  
> +static const struct sockaddr rpcb_inaddr_unspec = {
> +	.sa_family		= AF_UNSPEC,
> +};
> +
> +static void rpcb_set_unspec_raddr(struct rpcbind_args *rpcb)
> +{
> +	struct sockaddr *sap = (struct sockaddr *)&rpcb->r_raddr;
> +
> +	rpcb->r_raddrlen = sizeof(rpcb_inaddr_unspec);
> +	memcpy(sap, &rpcb_inaddr_unspec, rpcb->r_raddrlen);
> +}
> +
>  static const struct sockaddr_in rpcb_inaddr_loopback = {
>  	.sin_family		= AF_INET,
>  	.sin_addr.s_addr	= htonl(INADDR_LOOPBACK),
> @@ -454,7 +469,7 @@ int rpcb_getport_sync(struct sockaddr_in *sin, u32 prog, u32 vers, int prot)
>  	struct rpc_message msg = {
>  		.rpc_proc	= &rpcb_procedures2[RPCBPROC_GETPORT],
>  		.rpc_argp	= &map,
> -		.rpc_resp	= &map.r_port,
> +		.rpc_resp	= &map,
>  	};
>  	struct rpc_clnt	*rpcb_clnt;
>  	int status;
> @@ -467,12 +482,14 @@ int rpcb_getport_sync(struct sockaddr_in *sin, u32 prog, u32 vers, int prot)
>  	if (IS_ERR(rpcb_clnt))
>  		return PTR_ERR(rpcb_clnt);
>  
> +	memcpy(&map.r_raddr, sin, sizeof(*sin));
>  	status = rpc_call_sync(rpcb_clnt, &msg, 0);
>  	rpc_shutdown_client(rpcb_clnt);
>  
>  	if (status >= 0) {
> -		if (map.r_port != 0)
> -			return map.r_port;
> +		sin = (struct sockaddr_in *)&map.r_raddr;
> +		if (sin->sin_family == AF_INET)
> +			return ntohs(sin->sin_port);
>  		status = -EACCES;
>  	}
>  	return status;
> @@ -484,7 +501,7 @@ static struct rpc_task *rpcb_call_async(struct rpc_clnt *rpcb_clnt, struct rpcbi
>  	struct rpc_message msg = {
>  		.rpc_proc = proc,
>  		.rpc_argp = map,
> -		.rpc_resp = &map->r_port,
> +		.rpc_resp = map,
>  	};
>  	struct rpc_task_setup task_setup_data = {
>  		.rpc_client = rpcb_clnt,
> @@ -627,6 +644,8 @@ void rpcb_getport_async(struct rpc_task *task)
>  		break;
>  	case RPCBVERS_2:
>  		map->r_addr = NULL;
> +		memcpy(&map->r_raddr, sap, salen);
> +		map->r_raddrlen = salen;
>  		break;
>  	default:
>  		BUG();
> @@ -659,8 +678,10 @@ EXPORT_SYMBOL_GPL(rpcb_getport_async);
>  static void rpcb_getport_done(struct rpc_task *child, void *data)
>  {
>  	struct rpcbind_args *map = data;
> +	struct sockaddr *sap = (struct sockaddr *)&map->r_raddr;
>  	struct rpc_xprt *xprt = map->r_xprt;
>  	int status = child->tk_status;
> +	unsigned short port = 0;
>  
>  	/* Garbage reply: retry with a lesser rpcbind version */
>  	if (status == -EIO)
> @@ -673,19 +694,30 @@ static void rpcb_getport_done(struct rpc_task *child, void *data)
>  	if (status < 0) {
>  		/* rpcbind server not available on remote host? */
>  		xprt->ops->set_port(xprt, 0);
> -	} else if (map->r_port == 0) {
> +		xprt_clear_bound(xprt);
> +	} else if (sap->sa_family == AF_UNSPEC) {
>  		/* Requested RPC service wasn't registered on remote host */
>  		xprt->ops->set_port(xprt, 0);
> +		xprt_clear_bound(xprt);

Why do we need these two calls to xprt_clear_bound? In fact, why do we
need to call set_port() at all in the AF_UNSPEC case?

>  		status = -EACCES;
>  	} else {
>  		/* Succeeded */
> -		xprt->ops->set_port(xprt, map->r_port);
> +		switch (sap->sa_family) {
> +		case AF_INET:
> +			port = ntohs(((struct sockaddr_in *)sap)->sin_port);
> +			break;
> +		case AF_INET6:
> +			port = ntohs(((struct sockaddr_in6 *)sap)->sin6_port);
> +			break;
> +		}
> +
> +		xprt->ops->set_port(xprt, port);
>  		xprt_set_bound(xprt);
>  		status = 0;
>  	}
>  
>  	dprintk("RPC: %5u rpcb_getport_done(status %d, port %u)\n",
> -			child->tk_pid, status, map->r_port);
> +			child->tk_pid, status, port);
>  
>  	map->r_status = status;
>  }
> @@ -727,6 +759,37 @@ static int rpcb_decode_getport(struct rpc_rqst *req, __be32 *p,
>  	return 0;
>  }
>  
> +static int rpcb_dec_getport(struct rpc_rqst *req, __be32 *p,
> +			    struct rpcbind_args *rpcb)
> +{
> +	struct sockaddr *sap = (struct sockaddr *)&rpcb->r_raddr;
> +	struct rpc_task *task = req->rq_task;
> +	struct xdr_stream xdr;
> +	unsigned long port;
> +
> +	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
> +
> +	p = xdr_inline_decode(&xdr, sizeof(u32));

Technically, it is sizeof(__be32). In practice, though, '4' is good
enough.

> +	if (unlikely(p == NULL))
> +		return -EIO;
> +	port = ntohl(*p);
> +	dprintk("RPC: %5u PMAP_%s result: %lu\n", task->tk_pid,
> +			task->tk_msg.rpc_proc->p_name, port);
> +
> +	if (unlikely(port > USHORT_MAX))
> +		return -EIO;
> +
> +	if (sap->sa_family != AF_INET)
> +		return -EIO;
> +
> +	if (port == 0)
> +		rpcb_set_unspec_raddr(rpcb);
> +	else
> +		((struct sockaddr_in *)sap)->sin_port = htons(port);
> +
> +	return 0;
> +}
> +
>  static int rpcb_decode_set(struct rpc_rqst *req, __be32 *p,
>  			   unsigned int *boolp)
>  {
> @@ -871,11 +934,82 @@ out_err:
>  	return -EIO;
>  }
>  
> +static int rpcb_dec_getaddr(struct rpc_rqst *req, __be32 *p,
> +			    struct rpcbind_args *rpcb)
> +{
> +	struct sockaddr *sap = (struct sockaddr *)&rpcb->r_raddr;
> +	size_t salen = sizeof(rpcb->r_raddr);
> +	struct rpc_task *task = req->rq_task;
> +	struct sockaddr_in6 sin6;
> +	struct xdr_stream xdr;
> +	u32 len;
> +
> +	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
> +
> +	p = xdr_inline_decode(&xdr, sizeof(u32));

Ditto.

> +	if (unlikely(p == NULL))
> +		goto out_fail;
> +	len = ntohl(*p);
> +
> +	/*
> +	 * Special case: if the returned universal address is a null
> +	 * string, the requested RPC service was not registered.
> +	 */
> +	if (len == 0) {
> +		rpcb_set_unspec_raddr(rpcb);
> +		dprintk("RPC:       RPCB reply: program not registered\n");
> +		return 0;
> +	}
> +
> +	if (unlikely(len > RPCBIND_MAXUADDRLEN))
> +		goto out_fail;
> +
> +	p = xdr_inline_decode(&xdr, len);
> +	if (unlikely(p == NULL))
> +		goto out_fail;
> +
> +	/*
> +	 * If both the parent's destination address and the returned
> +	 * address are site-local, or both are link-local, copy the
> +	 * scope ID from the parent's address.  Universal addresses
> +	 * do not support scope IDs.
> +	 */
> +	if (sap->sa_family == AF_INET6)
> +		memcpy(&sin6, sap, sizeof(sin6));
> +
> +	salen = rpc_uaddr2sockaddr((char *)p, len, sap, salen);
> +	if (unlikely(salen == 0))
> +		goto out_fail;
> +	rpcb->r_raddrlen = salen;
> +
> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)

Ugh... No embedded ifdefs please....

> +	/* NB: sap now contains the returned address */
> +	if (sap->sa_family == AF_INET6) {
> +		struct sockaddr_in6 *recvd = (struct sockaddr_in6 *)sap;
> +
> +		if ((ipv6_addr_type(&sin6.sin6_addr) & IPV6_ADDR_LINKLOCAL &&
> +		    ipv6_addr_type(&recvd->sin6_addr) & IPV6_ADDR_LINKLOCAL) ||
> +		    (ipv6_addr_type(&sin6.sin6_addr) & IPV6_ADDR_SITELOCAL &&
> +		    ipv6_addr_type(&recvd->sin6_addr) & IPV6_ADDR_SITELOCAL))
> +			recvd->sin6_scope_id = sin6.sin6_scope_id;
> +	}

So, why are we filling in a full struct sockaddr here? We know which IP
address we've been told to connect to. We just want to know the port
number.

> +#endif
> +
> +	dprintk("RPC: %5u RPCB_%s reply: %s\n", task->tk_pid,
> +			task->tk_msg.rpc_proc->p_name, (char *)p);
> +	return 0;
> +
> +out_fail:
> +	dprintk("RPC: %5u malformed RPCB_%s reply\n",
> +			task->tk_pid, task->tk_msg.rpc_proc->p_name);
> +	return -EIO;
> +}
> +
>  #define PROC(proc, argtype, restype)					\
>  	[RPCBPROC_##proc] = {						\
>  		.p_proc		= RPCBPROC_##proc,			\
>  		.p_encode	= (kxdrproc_t) rpcb_enc_##argtype,	\
> -		.p_decode	= (kxdrproc_t) rpcb_decode_##restype,	\
> +		.p_decode	= (kxdrproc_t) rpcb_dec_##restype,	\
>  		.p_arglen	= RPCB_##argtype##args_sz,		\
>  		.p_replen	= RPCB_##restype##res_sz,		\
>  		.p_statidx	= RPCBPROC_##proc,			\
> 

[PATCH 05/10] SUNRPC: Clean up: Remove unused XDR decoder functions from rpcb_clnt.c

[Chuck Lever]

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/rpcb_clnt.c |   81 ------------------------------------------------
 1 files changed, 0 insertions(+), 81 deletions(-)

diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index 91af1f8..627d281 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -750,15 +750,6 @@ static int rpcb_enc_mapping(struct rpc_rqst *req, __be32 *p,
 	return 0;
 }
 
-static int rpcb_decode_getport(struct rpc_rqst *req, __be32 *p,
-			       unsigned short *portp)
-{
-	*portp = (unsigned short) ntohl(*p++);
-	dprintk("RPC:       rpcb getport result: %u\n",
-			*portp);
-	return 0;
-}
-
 static int rpcb_dec_getport(struct rpc_rqst *req, __be32 *p,
 			    struct rpcbind_args *rpcb)
 {
@@ -790,15 +781,6 @@ static int rpcb_dec_getport(struct rpc_rqst *req, __be32 *p,
 	return 0;
 }
 
-static int rpcb_decode_set(struct rpc_rqst *req, __be32 *p,
-			   unsigned int *boolp)
-{
-	*boolp = (unsigned int) ntohl(*p++);
-	dprintk("RPC:       rpcb set/unset call %s\n",
-			(*boolp ? "succeeded" : "failed"));
-	return 0;
-}
-
 static int rpcb_dec_set(struct rpc_rqst *req, __be32 *p,
 			unsigned int *boolp)
 {
@@ -871,69 +853,6 @@ static int rpcb_enc_getaddr(struct rpc_rqst *req, __be32 *p,
 	return 0;
 }
 
-static int rpcb_decode_getaddr(struct rpc_rqst *req, __be32 *p,
-			       unsigned short *portp)
-{
-	char *addr;
-	u32 addr_len;
-	int c, i, f, first, val;
-
-	*portp = 0;
-	addr_len = ntohl(*p++);
-
-	if (addr_len == 0) {
-		dprintk("RPC:       rpcb_decode_getaddr: "
-					"service is not registered\n");
-		return 0;
-	}
-
-	/*
-	 * Simple sanity check.
-	 */
-	if (addr_len > RPCBIND_MAXUADDRLEN)
-		goto out_err;
-
-	/*
-	 * Start at the end and walk backwards until the first dot
-	 * is encountered.  When the second dot is found, we have
-	 * both parts of the port number.
-	 */
-	addr = (char *)p;
-	val = 0;
-	first = 1;
-	f = 1;
-	for (i = addr_len - 1; i > 0; i--) {
-		c = addr[i];
-		if (c >= '0' && c <= '9') {
-			val += (c - '0') * f;
-			f *= 10;
-		} else if (c == '.') {
-			if (first) {
-				*portp = val;
-				val = first = 0;
-				f = 1;
-			} else {
-				*portp |= (val << 8);
-				break;
-			}
-		}
-	}
-
-	/*
-	 * Simple sanity check.  If we never saw a dot in the reply,
-	 * then this was probably just garbage.
-	 */
-	if (first)
-		goto out_err;
-
-	dprintk("RPC:       rpcb_decode_getaddr port=%u\n", *portp);
-	return 0;
-
-out_err:
-	dprintk("RPC:       rpcbind server returned malformed reply\n");
-	return -EIO;
-}
-
 static int rpcb_dec_getaddr(struct rpc_rqst *req, __be32 *p,
 			    struct rpcbind_args *rpcb)
 {

[PATCH 06/10] SUNRPC: Eliminate PROC macro from rpcb_clnt

[Chuck Lever]

Clean up: Replace PROC macro with open coded C99 structure
initializers to improve readability.

The rpcbind v4 GETVERSADDR procedure is never sent by the current
implementation, so it is not copied to the new structures.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/rpcb_clnt.c |  113 +++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 91 insertions(+), 22 deletions(-)

diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index 627d281..4cc2c58 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -924,39 +924,108 @@ out_fail:
 	return -EIO;
 }
 
-#define PROC(proc, argtype, restype)					\
-	[RPCBPROC_##proc] = {						\
-		.p_proc		= RPCBPROC_##proc,			\
-		.p_encode	= (kxdrproc_t) rpcb_enc_##argtype,	\
-		.p_decode	= (kxdrproc_t) rpcb_dec_##restype,	\
-		.p_arglen	= RPCB_##argtype##args_sz,		\
-		.p_replen	= RPCB_##restype##res_sz,		\
-		.p_statidx	= RPCBPROC_##proc,			\
-		.p_timer	= 0,					\
-		.p_name		= #proc,				\
-	}
-
 /*
  * Not all rpcbind procedures described in RFC 1833 are implemented
  * since the Linux kernel RPC code requires only these.
  */
+
 static struct rpc_procinfo rpcb_procedures2[] = {
-	PROC(SET,		mapping,	set),
-	PROC(UNSET,		mapping,	set),
-	PROC(GETPORT,		mapping,	getport),
+	[RPCBPROC_SET] = {
+		.p_proc		= RPCBPROC_SET,
+		.p_encode	= (kxdrproc_t)rpcb_enc_mapping,
+		.p_decode	= (kxdrproc_t)rpcb_dec_set,
+		.p_arglen	= RPCB_mappingargs_sz,
+		.p_replen	= RPCB_setres_sz,
+		.p_statidx	= RPCBPROC_SET,
+		.p_timer	= 0,
+		.p_name		= "SET",
+	},
+	[RPCBPROC_UNSET] = {
+		.p_proc		= RPCBPROC_UNSET,
+		.p_encode	= (kxdrproc_t)rpcb_enc_mapping,
+		.p_decode	= (kxdrproc_t)rpcb_dec_set,
+		.p_arglen	= RPCB_mappingargs_sz,
+		.p_replen	= RPCB_setres_sz,
+		.p_statidx	= RPCBPROC_UNSET,
+		.p_timer	= 0,
+		.p_name		= "UNSET",
+	},
+	[RPCBPROC_GETPORT] = {
+		.p_proc		= RPCBPROC_GETPORT,
+		.p_encode	= (kxdrproc_t)rpcb_enc_mapping,
+		.p_decode	= (kxdrproc_t)rpcb_dec_getport,
+		.p_arglen	= RPCB_mappingargs_sz,
+		.p_replen	= RPCB_getportres_sz,
+		.p_statidx	= RPCBPROC_GETPORT,
+		.p_timer	= 0,
+		.p_name		= "GETPORT",
+	},
 };
 
 static struct rpc_procinfo rpcb_procedures3[] = {
-	PROC(SET,		getaddr,	set),
-	PROC(UNSET,		getaddr,	set),
-	PROC(GETADDR,		getaddr,	getaddr),
+	[RPCBPROC_SET] = {
+		.p_proc		= RPCBPROC_SET,
+		.p_encode	= (kxdrproc_t)rpcb_enc_getaddr,
+		.p_decode	= (kxdrproc_t)rpcb_dec_set,
+		.p_arglen	= RPCB_getaddrargs_sz,
+		.p_replen	= RPCB_setres_sz,
+		.p_statidx	= RPCBPROC_SET,
+		.p_timer	= 0,
+		.p_name		= "SET",
+	},
+	[RPCBPROC_UNSET] = {
+		.p_proc		= RPCBPROC_UNSET,
+		.p_encode	= (kxdrproc_t)rpcb_enc_getaddr,
+		.p_decode	= (kxdrproc_t)rpcb_dec_set,
+		.p_arglen	= RPCB_getaddrargs_sz,
+		.p_replen	= RPCB_setres_sz,
+		.p_statidx	= RPCBPROC_UNSET,
+		.p_timer	= 0,
+		.p_name		= "UNSET",
+	},
+	[RPCBPROC_GETADDR] = {
+		.p_proc		= RPCBPROC_GETADDR,
+		.p_encode	= (kxdrproc_t)rpcb_enc_getaddr,
+		.p_decode	= (kxdrproc_t)rpcb_dec_getaddr,
+		.p_arglen	= RPCB_getaddrargs_sz,
+		.p_replen	= RPCB_getaddrres_sz,
+		.p_statidx	= RPCBPROC_GETADDR,
+		.p_timer	= 0,
+		.p_name		= "GETADDR",
+	},
 };
 
 static struct rpc_procinfo rpcb_procedures4[] = {
-	PROC(SET,		getaddr,	set),
-	PROC(UNSET,		getaddr,	set),
-	PROC(GETADDR,		getaddr,	getaddr),
-	PROC(GETVERSADDR,	getaddr,	getaddr),
+	[RPCBPROC_SET] = {
+		.p_proc		= RPCBPROC_SET,
+		.p_encode	= (kxdrproc_t)rpcb_enc_getaddr,
+		.p_decode	= (kxdrproc_t)rpcb_dec_set,
+		.p_arglen	= RPCB_getaddrargs_sz,
+		.p_replen	= RPCB_setres_sz,
+		.p_statidx	= RPCBPROC_SET,
+		.p_timer	= 0,
+		.p_name		= "SET",
+	},
+	[RPCBPROC_UNSET] = {
+		.p_proc		= RPCBPROC_UNSET,
+		.p_encode	= (kxdrproc_t)rpcb_enc_getaddr,
+		.p_decode	= (kxdrproc_t)rpcb_dec_set,
+		.p_arglen	= RPCB_getaddrargs_sz,
+		.p_replen	= RPCB_setres_sz,
+		.p_statidx	= RPCBPROC_UNSET,
+		.p_timer	= 0,
+		.p_name		= "UNSET",
+	},
+	[RPCBPROC_GETADDR] = {
+		.p_proc		= RPCBPROC_GETADDR,
+		.p_encode	= (kxdrproc_t)rpcb_enc_getaddr,
+		.p_decode	= (kxdrproc_t)rpcb_dec_getaddr,
+		.p_arglen	= RPCB_getaddrargs_sz,
+		.p_replen	= RPCB_getaddrres_sz,
+		.p_statidx	= RPCBPROC_GETADDR,
+		.p_timer	= 0,
+		.p_name		= "GETADDR",
+	},
 };
 
 static struct rpcb_info rpcb_next_version[] = {

[PATCH 07/10] SUNRPC: Pass full bind address to transports after GETPORT/GETADDR

[Chuck Lever]

TI-RPC rpcbind operations provide not just a port number, but a full
socket address the client should connect to.  This allows rpcbind to
redirect RPC traffic to specific network interfaces or servers.  The
Linux kernel rpcbind client implementation currently ignores the
address.

Expand the ->set_port transport method so an address is passed to
transports during an RPC bind operation.  Additional changes to
individual client transports will be required to replace the peer
address after an rpcbind operation.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 include/linux/sunrpc/xprt.h     |    4 +++-
 net/sunrpc/rpcb_clnt.c          |   22 +++++++---------------
 net/sunrpc/xprtrdma/transport.c |   23 +++++++++++++++++++----
 net/sunrpc/xprtsock.c           |   35 +++++++++++++++++++++++++++--------
 4 files changed, 56 insertions(+), 28 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index 65fad95..deb39d5 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -112,7 +112,9 @@ struct rpc_xprt_ops {
 	int		(*reserve_xprt)(struct rpc_task *task);
 	void		(*release_xprt)(struct rpc_xprt *xprt, struct rpc_task *task);
 	void		(*rpcbind)(struct rpc_task *task);
-	void		(*set_port)(struct rpc_xprt *xprt, unsigned short port);
+	void		(*set_address)(struct rpc_xprt *xprt,
+					const struct sockaddr *sap,
+					const size_t salen);
 	void		(*connect)(struct rpc_task *task);
 	void *		(*buf_alloc)(struct rpc_task *task, size_t size);
 	void		(*buf_free)(void *buffer);
diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index 4cc2c58..68046df 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -681,7 +681,6 @@ static void rpcb_getport_done(struct rpc_task *child, void *data)
 	struct sockaddr *sap = (struct sockaddr *)&map->r_raddr;
 	struct rpc_xprt *xprt = map->r_xprt;
 	int status = child->tk_status;
-	unsigned short port = 0;
 
 	/* Garbage reply: retry with a lesser rpcbind version */
 	if (status == -EIO)
@@ -693,31 +692,24 @@ static void rpcb_getport_done(struct rpc_task *child, void *data)
 
 	if (status < 0) {
 		/* rpcbind server not available on remote host? */
-		xprt->ops->set_port(xprt, 0);
+		xprt->ops->set_address(xprt, &rpcb_inaddr_unspec,
+					sizeof(rpcb_inaddr_unspec));
 		xprt_clear_bound(xprt);
 	} else if (sap->sa_family == AF_UNSPEC) {
 		/* Requested RPC service wasn't registered on remote host */
-		xprt->ops->set_port(xprt, 0);
+		xprt->ops->set_address(xprt, &rpcb_inaddr_unspec,
+					sizeof(rpcb_inaddr_unspec));
 		xprt_clear_bound(xprt);
 		status = -EACCES;
 	} else {
 		/* Succeeded */
-		switch (sap->sa_family) {
-		case AF_INET:
-			port = ntohs(((struct sockaddr_in *)sap)->sin_port);
-			break;
-		case AF_INET6:
-			port = ntohs(((struct sockaddr_in6 *)sap)->sin6_port);
-			break;
-		}
-
-		xprt->ops->set_port(xprt, port);
+		xprt->ops->set_address(xprt, sap, map->r_raddrlen);
 		xprt_set_bound(xprt);
 		status = 0;
 	}
 
-	dprintk("RPC: %5u rpcb_getport_done(status %d, port %u)\n",
-			child->tk_pid, status, port);
+	dprintk("RPC: %5u rpcb_getport_done(status %d)\n",
+			child->tk_pid, status);
 
 	map->r_status = status;
 }
diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
index 5f9b867..780385f 100644
--- a/net/sunrpc/xprtrdma/transport.c
+++ b/net/sunrpc/xprtrdma/transport.c
@@ -453,14 +453,29 @@ xprt_rdma_close(struct rpc_xprt *xprt)
 }
 
 static void
-xprt_rdma_set_port(struct rpc_xprt *xprt, u16 port)
+xprt_rdma_set_address(struct rpc_xprt *xprt, const struct sockaddr *bindaddr,
+			const size_t bindaddr_len)
 {
 	struct sockaddr_in *sap;
+	__be16 port;
+
+	switch (bindaddr->sa_family) {
+	case AF_UNSPEC:
+		port = 0;
+		break;
+	case AF_INET:
+		port = ((struct sockaddr_in *)bindaddr)->sin_port;
+		break;
+	default:
+		dprintk("RPC:       %s: address family not supported\n",
+				__func__);
+		return;
+	}
 
 	sap = (struct sockaddr_in *)&xprt->addr;
-	sap->sin_port = htons(port);
+	sap->sin_port = port;
 	sap = (struct sockaddr_in *)&rpcx_to_rdmad(xprt).addr;
-	sap->sin_port = htons(port);
+	sap->sin_port = port;
 	dprintk("RPC:       %s: %u\n", __func__, port);
 }
 
@@ -752,7 +767,7 @@ static struct rpc_xprt_ops xprt_rdma_procs = {
 	.release_request	= xprt_release_rqst_cong,       /* ditto */
 	.set_retrans_timeout	= xprt_set_retrans_timeout_def, /* ditto */
 	.rpcbind		= rpcb_getport_async,	/* sunrpc/rpcb_clnt.c */
-	.set_port		= xprt_rdma_set_port,
+	.set_address		= xprt_rdma_set_address,
 	.connect		= xprt_rdma_connect,
 	.buf_alloc		= xprt_rdma_allocate,
 	.buf_free		= xprt_rdma_free,
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 302a409..000ddd9 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1521,23 +1521,42 @@ static unsigned short xs_get_random_port(void)
 }
 
 /**
- * xs_set_port - reset the port number in the remote endpoint address
+ * xs_set_address - reset the port number in the remote endpoint address
  * @xprt: generic transport
- * @port: new port number
+ * @bindaddr: socket address to connect to
+ * @bindaddr_len: length of socket address
  *
  */
-static void xs_set_port(struct rpc_xprt *xprt, unsigned short port)
+static void xs_set_address(struct rpc_xprt *xprt,
+			   const struct sockaddr *bindaddr,
+			   const size_t bindaddr_len)
 {
 	struct sockaddr *addr = xs_addr(xprt);
+	__be16 port;
 
-	dprintk("RPC:       setting port for xprt %p to %u\n", xprt, port);
+	switch (bindaddr->sa_family) {
+	case AF_UNSPEC:
+		port = 0;
+		break;
+	case AF_INET:
+		port = ((struct sockaddr_in *)bindaddr)->sin_port;
+		break;
+	case AF_INET6:
+		port = ((struct sockaddr_in6 *)bindaddr)->sin6_port;
+		break;
+	default:
+		BUG();
+	}
+
+	dprintk("RPC:       setting port for xprt %p to %u\n",
+			xprt, ntohs(port));
 
 	switch (addr->sa_family) {
 	case AF_INET:
-		((struct sockaddr_in *)addr)->sin_port = htons(port);
+		((struct sockaddr_in *)addr)->sin_port = port;
 		break;
 	case AF_INET6:
-		((struct sockaddr_in6 *)addr)->sin6_port = htons(port);
+		((struct sockaddr_in6 *)addr)->sin6_port = port;
 		break;
 	default:
 		BUG();
@@ -2102,7 +2121,7 @@ static struct rpc_xprt_ops xs_udp_ops = {
 	.reserve_xprt		= xprt_reserve_xprt_cong,
 	.release_xprt		= xprt_release_xprt_cong,
 	.rpcbind		= rpcb_getport_async,
-	.set_port		= xs_set_port,
+	.set_address		= xs_set_address,
 	.connect		= xs_connect,
 	.buf_alloc		= rpc_malloc,
 	.buf_free		= rpc_free,
@@ -2119,7 +2138,7 @@ static struct rpc_xprt_ops xs_tcp_ops = {
 	.reserve_xprt		= xprt_reserve_xprt,
 	.release_xprt		= xs_tcp_release_xprt,
 	.rpcbind		= rpcb_getport_async,
-	.set_port		= xs_set_port,
+	.set_address		= xs_set_address,
 	.connect		= xs_tcp_connect,
 	.buf_alloc		= rpc_malloc,
 	.buf_free		= rpc_free,

Re: [PATCH 07/10] SUNRPC: Pass full bind address to transports after GETPORT/GETADDR

[Trond Myklebust]

On Wed, 2009-07-15 at 17:42 -0400, Chuck Lever wrote:
> TI-RPC rpcbind operations provide not just a port number, but a full
> socket address the client should connect to.  This allows rpcbind to
> redirect RPC traffic to specific network interfaces or servers.  The
> Linux kernel rpcbind client implementation currently ignores the
> address.
> 
> Expand the ->set_port transport method so an address is passed to
> transports during an RPC bind operation.  Additional changes to
> individual client transports will be required to replace the peer
> address after an rpcbind operation.

Now I'm worried. We've just spent a lot of time implementing RPCSEC_GSS
security, and yet we're going allow an AUTH_SYS-based RPC call to tell
us to change an IP address that the user supplied us with? It was bad
enough when we allowed it to set the port number...

     Trond

Re: [PATCH 07/10] SUNRPC: Pass full bind address to transports after GETPORT/GETADDR

[J. Bruce Fields]

On Thu, Jul 16, 2009 at 05:10:44PM -0400, Trond Myklebust wrote:
> On Wed, 2009-07-15 at 17:42 -0400, Chuck Lever wrote:
> > TI-RPC rpcbind operations provide not just a port number, but a full
> > socket address the client should connect to.  This allows rpcbind to
> > redirect RPC traffic to specific network interfaces or servers.  The
> > Linux kernel rpcbind client implementation currently ignores the
> > address.
> > 
> > Expand the ->set_port transport method so an address is passed to
> > transports during an RPC bind operation.  Additional changes to
> > individual client transports will be required to replace the peer
> > address after an rpcbind operation.
> 
> Now I'm worried. We've just spent a lot of time implementing RPCSEC_GSS
> security, and yet we're going allow an AUTH_SYS-based RPC call to tell
> us to change an IP address that the user supplied us with? It was bad
> enough when we allowed it to set the port number...

The authentication of the server will use whatever hostname was supplied
on the commandline, so should still provide some protection.

On the other hand: with krb5 (as opposed to krb5i or krb5p), a
man-in-the-middle attack that keeps the rpc headers and replaces the
body is always possible.  But if you can redirect the client to a
port/ip address under your control, that might simplify the attack
significantly.  Similarly, sniffing non-krb5p traffic might become
simpler.

It might also simplify denial of service attacks (possibly with the goal
of making the user give up on gss and downgrade to auth_sys?).

An attacker could do the same stuff with dns, I suppose.

--b.

[PATCH 08/10] SUNRPC: Rename sock_xprt.addr as sock_xprt.srcaddr

[Chuck Lever]

Rename the addr and port field in the sock_xprt struct.  I'm about to add
another address field, so let's avoid confusion.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/xprtsock.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 000ddd9..4804d88 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -248,8 +248,8 @@ struct sock_xprt {
 	 * Connection of transports
 	 */
 	struct delayed_work	connect_worker;
-	struct sockaddr_storage	addr;
-	unsigned short		port;
+	struct sockaddr_storage	srcaddr;
+	unsigned short		srcport;
 
 	/*
 	 * UDP socket buffer size parameters
@@ -1565,7 +1565,7 @@ static void xs_set_address(struct rpc_xprt *xprt,
 
 static unsigned short xs_get_srcport(struct sock_xprt *transport, struct socket *sock)
 {
-	unsigned short port = transport->port;
+	unsigned short port = transport->srcport;
 
 	if (port == 0 && transport->xprt.resvport)
 		port = xs_get_random_port();
@@ -1574,8 +1574,8 @@ static unsigned short xs_get_srcport(struct sock_xprt *transport, struct socket
 
 static unsigned short xs_next_srcport(struct sock_xprt *transport, struct socket *sock, unsigned short port)
 {
-	if (transport->port != 0)
-		transport->port = 0;
+	if (transport->srcport != 0)
+		transport->srcport = 0;
 	if (!transport->xprt.resvport)
 		return 0;
 	if (port <= xprt_min_resvport || port > xprt_max_resvport)
@@ -1593,7 +1593,7 @@ static int xs_bind4(struct sock_xprt *transport, struct socket *sock)
 	unsigned short port = xs_get_srcport(transport, sock);
 	unsigned short last;
 
-	sa = (struct sockaddr_in *)&transport->addr;
+	sa = (struct sockaddr_in *)&transport->srcaddr;
 	myaddr.sin_addr = sa->sin_addr;
 	do {
 		myaddr.sin_port = htons(port);
@@ -1602,7 +1602,7 @@ static int xs_bind4(struct sock_xprt *transport, struct socket *sock)
 		if (port == 0)
 			break;
 		if (err == 0) {
-			transport->port = port;
+			transport->srcport = port;
 			break;
 		}
 		last = port;
@@ -1626,7 +1626,7 @@ static int xs_bind6(struct sock_xprt *transport, struct socket *sock)
 	unsigned short port = xs_get_srcport(transport, sock);
 	unsigned short last;
 
-	sa = (struct sockaddr_in6 *)&transport->addr;
+	sa = (struct sockaddr_in6 *)&transport->srcaddr;
 	myaddr.sin6_addr = sa->sin6_addr;
 	do {
 		myaddr.sin6_port = htons(port);
@@ -1635,7 +1635,7 @@ static int xs_bind6(struct sock_xprt *transport, struct socket *sock)
 		if (port == 0)
 			break;
 		if (err == 0) {
-			transport->port = port;
+			transport->srcport = port;
 			break;
 		}
 		last = port;
@@ -2080,7 +2080,7 @@ static void xs_udp_print_stats(struct rpc_xprt *xprt, struct seq_file *seq)
 	struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
 
 	seq_printf(seq, "\txprt:\tudp %u %lu %lu %lu %lu %Lu %Lu\n",
-			transport->port,
+			transport->srcport,
 			xprt->stat.bind_count,
 			xprt->stat.sends,
 			xprt->stat.recvs,
@@ -2104,7 +2104,7 @@ static void xs_tcp_print_stats(struct rpc_xprt *xprt, struct seq_file *seq)
 		idle_time = (long)(jiffies - xprt->last_used) / HZ;
 
 	seq_printf(seq, "\txprt:\ttcp %u %lu %lu %lu %ld %lu %lu %lu %Lu %Lu\n",
-			transport->port,
+			transport->srcport,
 			xprt->stat.bind_count,
 			xprt->stat.connect_count,
 			xprt->stat.connect_time,
@@ -2183,7 +2183,7 @@ static struct rpc_xprt *xs_setup_xprt(struct xprt_create *args,
 	memcpy(&xprt->addr, args->dstaddr, args->addrlen);
 	xprt->addrlen = args->addrlen;
 	if (args->srcaddr)
-		memcpy(&new->addr, args->srcaddr, args->addrlen);
+		memcpy(&new->srcaddr, args->srcaddr, args->addrlen);
 
 	return xprt;
 }

[PATCH 09/10] SUNRPC: Use address returned by rpcbind when connecting

[Chuck Lever]

rpcbind provides an RPC service address and port number.  Currently
the transport capabilities defined in net/sunrpc/xprtsock.c use only
the port number.  Teach them to use the returned address as well.

After a bind completes, update the transport instance's address
strings so debugging messages display the current port and address
it's connected to.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/xprtsock.c |  148 ++++++++++++++++++++++++++++++++-----------------
 1 files changed, 98 insertions(+), 50 deletions(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 4804d88..df3d4af 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -250,6 +250,8 @@ struct sock_xprt {
 	struct delayed_work	connect_worker;
 	struct sockaddr_storage	srcaddr;
 	unsigned short		srcport;
+	struct sockaddr_storage rpcbindaddr;
+	size_t			rpcbindaddr_len;
 
 	/*
 	 * UDP socket buffer size parameters
@@ -296,9 +298,39 @@ static inline struct sockaddr_in6 *xs_addr_in6(struct rpc_xprt *xprt)
 	return (struct sockaddr_in6 *) &xprt->addr;
 }
 
+static inline struct sockaddr *xs_rpcbindaddr(struct sock_xprt *trans)
+{
+	return (struct sockaddr *)&trans->rpcbindaddr;
+}
+
+static inline struct sockaddr_in *xs_rpcbindaddr_in(struct sock_xprt *trans)
+{
+	return (struct sockaddr_in *)&trans->rpcbindaddr;
+}
+
+static inline struct sockaddr_in6 *xs_rpcbindaddr_in6(struct sock_xprt *trans)
+{
+	return (struct sockaddr_in6 *)&trans->rpcbindaddr;
+}
+
+static void xs_free_peer_addresses(struct rpc_xprt *xprt)
+{
+	unsigned int i;
+
+	for (i = 0; i < RPC_DISPLAY_MAX; i++)
+		switch (i) {
+		case RPC_DISPLAY_PROTO:
+		case RPC_DISPLAY_NETID:
+			continue;
+		default:
+			kfree(xprt->address_strings[i]);
+		}
+}
+
 static void xs_format_common_peer_addresses(struct rpc_xprt *xprt)
 {
-	struct sockaddr *sap = xs_addr(xprt);
+	struct sock_xprt *trans = container_of(xprt, struct sock_xprt, xprt);
+	struct sockaddr *sap = xs_rpcbindaddr(trans);
 	char buf[128];
 
 	(void)rpc_ntop(sap, buf, sizeof(buf));
@@ -321,7 +353,8 @@ static void xs_format_ipv4_peer_addresses(struct rpc_xprt *xprt,
 					  const char *protocol,
 					  const char *netid)
 {
-	struct sockaddr_in *sin = xs_addr_in(xprt);
+	struct sock_xprt *trans = container_of(xprt, struct sock_xprt, xprt);
+	struct sockaddr_in *sin = xs_rpcbindaddr_in(trans);
 	char buf[16];
 
 	xprt->address_strings[RPC_DISPLAY_PROTO] = protocol;
@@ -334,11 +367,27 @@ static void xs_format_ipv4_peer_addresses(struct rpc_xprt *xprt,
 	xs_format_common_peer_addresses(xprt);
 }
 
+static void xs_update_ipv4_peer_addresses(struct rpc_xprt *xprt)
+{
+	struct sock_xprt *trans = container_of(xprt, struct sock_xprt, xprt);
+	struct sockaddr_in *sin = xs_rpcbindaddr_in(trans);
+	char buf[16];
+
+	xs_free_peer_addresses(xprt);
+
+	(void)snprintf(buf, sizeof(buf), "%02x%02x%02x%02x",
+				NIPQUAD(sin->sin_addr.s_addr));
+	xprt->address_strings[RPC_DISPLAY_HEX_ADDR] = kstrdup(buf, GFP_KERNEL);
+
+	xs_format_common_peer_addresses(xprt);
+}
+
 static void xs_format_ipv6_peer_addresses(struct rpc_xprt *xprt,
 					  const char *protocol,
 					  const char *netid)
 {
-	struct sockaddr_in6 *sin6 = xs_addr_in6(xprt);
+	struct sock_xprt *trans = container_of(xprt, struct sock_xprt, xprt);
+	struct sockaddr_in6 *sin6 = xs_rpcbindaddr_in6(trans);
 	char buf[48];
 
 	xprt->address_strings[RPC_DISPLAY_PROTO] = protocol;
@@ -350,18 +399,18 @@ static void xs_format_ipv6_peer_addresses(struct rpc_xprt *xprt,
 	xs_format_common_peer_addresses(xprt);
 }
 
-static void xs_free_peer_addresses(struct rpc_xprt *xprt)
+static void xs_update_ipv6_peer_addresses(struct rpc_xprt *xprt)
 {
-	unsigned int i;
+	struct sock_xprt *trans = container_of(xprt, struct sock_xprt, xprt);
+	struct sockaddr_in6 *sin6 = xs_rpcbindaddr_in6(trans);
+	char buf[48];
 
-	for (i = 0; i < RPC_DISPLAY_MAX; i++)
-		switch (i) {
-		case RPC_DISPLAY_PROTO:
-		case RPC_DISPLAY_NETID:
-			continue;
-		default:
-			kfree(xprt->address_strings[i]);
-		}
+	xs_free_peer_addresses(xprt);
+
+	(void)snprintf(buf, sizeof(buf), "%pi6", &sin6->sin6_addr);
+	xprt->address_strings[RPC_DISPLAY_HEX_ADDR] = kstrdup(buf, GFP_KERNEL);
+
+	xs_format_common_peer_addresses(xprt);
 }
 
 #define XS_SENDMSG_FLAGS	(MSG_DONTWAIT | MSG_NOSIGNAL)
@@ -545,9 +594,9 @@ static int xs_udp_send_request(struct rpc_task *task)
 	if (!xprt_bound(xprt))
 		return -ENOTCONN;
 	status = xs_sendpages(transport->sock,
-			      xs_addr(xprt),
-			      xprt->addrlen, xdr,
-			      req->rq_bytes_sent);
+			      xs_rpcbindaddr(transport),
+			      transport->rpcbindaddr_len,
+			      xdr, req->rq_bytes_sent);
 
 	dprintk("RPC:       xs_udp_send_request(%u) = %d\n",
 			xdr->len - req->rq_bytes_sent, status);
@@ -1531,36 +1580,28 @@ static void xs_set_address(struct rpc_xprt *xprt,
 			   const struct sockaddr *bindaddr,
 			   const size_t bindaddr_len)
 {
-	struct sockaddr *addr = xs_addr(xprt);
-	__be16 port;
+	struct sock_xprt *transport = container_of(xprt, struct sock_xprt,
+							xprt);
+
+	memcpy(&transport->rpcbindaddr, bindaddr, bindaddr_len);
+	transport->rpcbindaddr_len = bindaddr_len;
 
 	switch (bindaddr->sa_family) {
 	case AF_UNSPEC:
-		port = 0;
-		break;
+		dprintk("RPC:       xprt %p not bound\n", xprt);
+		return;
 	case AF_INET:
-		port = ((struct sockaddr_in *)bindaddr)->sin_port;
+		xs_update_ipv4_peer_addresses(xprt);
 		break;
 	case AF_INET6:
-		port = ((struct sockaddr_in6 *)bindaddr)->sin6_port;
+		xs_update_ipv6_peer_addresses(xprt);
 		break;
 	default:
 		BUG();
 	}
 
-	dprintk("RPC:       setting port for xprt %p to %u\n",
-			xprt, ntohs(port));
-
-	switch (addr->sa_family) {
-	case AF_INET:
-		((struct sockaddr_in *)addr)->sin_port = port;
-		break;
-	case AF_INET6:
-		((struct sockaddr_in6 *)addr)->sin6_port = port;
-		break;
-	default:
-		BUG();
-	}
+	dprintk("RPC:       xprt %p bound to %s\n",
+			xprt, xprt->address_strings[RPC_DISPLAY_ALL]);
 }
 
 static unsigned short xs_get_srcport(struct sock_xprt *transport, struct socket *sock)
@@ -1867,7 +1908,8 @@ static int xs_tcp_finish_connecting(struct rpc_xprt *xprt, struct socket *sock)
 	/* Tell the socket layer to start connecting... */
 	xprt->stat.connect_count++;
 	xprt->stat.connect_start = jiffies;
-	return kernel_connect(sock, xs_addr(xprt), xprt->addrlen, O_NONBLOCK);
+	return kernel_connect(sock, xs_rpcbindaddr(transport),
+				transport->rpcbindaddr_len, O_NONBLOCK);
 }
 
 /**
@@ -2227,29 +2269,32 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
 
 	switch (addr->sa_family) {
 	case AF_INET:
-		if (((struct sockaddr_in *)addr)->sin_port != htons(0))
+		xs_format_ipv4_peer_addresses(xprt, "udp", RPCBIND_NETID_UDP);
+		if (((struct sockaddr_in *)addr)->sin_port != htons(0)) {
+			xs_set_address(xprt, addr,
+					sizeof(struct sockaddr_in));
 			xprt_set_bound(xprt);
+		}
 
 		INIT_DELAYED_WORK(&transport->connect_worker,
 					xs_udp_connect_worker4);
-		xs_format_ipv4_peer_addresses(xprt, "udp", RPCBIND_NETID_UDP);
 		break;
 	case AF_INET6:
-		if (((struct sockaddr_in6 *)addr)->sin6_port != htons(0))
+		xs_format_ipv6_peer_addresses(xprt, "udp", RPCBIND_NETID_UDP6);
+		if (((struct sockaddr_in6 *)addr)->sin6_port != htons(0)) {
+			xs_set_address(xprt, addr,
+					sizeof(struct sockaddr_in6));
 			xprt_set_bound(xprt);
+		}
 
 		INIT_DELAYED_WORK(&transport->connect_worker,
 					xs_udp_connect_worker6);
-		xs_format_ipv6_peer_addresses(xprt, "udp", RPCBIND_NETID_UDP6);
 		break;
 	default:
 		kfree(xprt);
 		return ERR_PTR(-EAFNOSUPPORT);
 	}
 
-	dprintk("RPC:       set up transport to address %s\n",
-			xprt->address_strings[RPC_DISPLAY_ALL]);
-
 	if (try_module_get(THIS_MODULE))
 		return xprt;
 
@@ -2294,27 +2339,30 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
 
 	switch (addr->sa_family) {
 	case AF_INET:
-		if (((struct sockaddr_in *)addr)->sin_port != htons(0))
+		xs_format_ipv4_peer_addresses(xprt, "tcp", RPCBIND_NETID_TCP);
+		if (((struct sockaddr_in *)addr)->sin_port != htons(0)) {
+			xs_set_address(xprt, addr,
+					sizeof(struct sockaddr_in));
 			xprt_set_bound(xprt);
+		}
 
 		INIT_DELAYED_WORK(&transport->connect_worker, xs_tcp_connect_worker4);
-		xs_format_ipv4_peer_addresses(xprt, "tcp", RPCBIND_NETID_TCP);
 		break;
 	case AF_INET6:
-		if (((struct sockaddr_in6 *)addr)->sin6_port != htons(0))
+		xs_format_ipv6_peer_addresses(xprt, "tcp", RPCBIND_NETID_TCP6);
+		if (((struct sockaddr_in6 *)addr)->sin6_port != htons(0)) {
+			xs_set_address(xprt, addr,
+					sizeof(struct sockaddr_in6));
 			xprt_set_bound(xprt);
+		}
 
 		INIT_DELAYED_WORK(&transport->connect_worker, xs_tcp_connect_worker6);
-		xs_format_ipv6_peer_addresses(xprt, "tcp", RPCBIND_NETID_TCP6);
 		break;
 	default:
 		kfree(xprt);
 		return ERR_PTR(-EAFNOSUPPORT);
 	}
 
-	dprintk("RPC:       set up transport to address %s\n",
-			xprt->address_strings[RPC_DISPLAY_ALL]);
-
 	if (try_module_get(THIS_MODULE))
 		return xprt;
 

[PATCH 10/10] SUNRPC: Add documenting comments in net/sunrpc/timer.c

[Chuck Lever]

Clean up: provide documenting comments for the functions in
net/sunrpc/timer.c.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/timer.c |   45 +++++++++++++++++++++++++++++----------------
 1 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/net/sunrpc/timer.c b/net/sunrpc/timer.c
index 31becbf..14ac71c 100644
--- a/net/sunrpc/timer.c
+++ b/net/sunrpc/timer.c
@@ -25,8 +25,13 @@
 #define RPC_RTO_INIT (HZ/5)
 #define RPC_RTO_MIN (HZ/10)
 
-void
-rpc_init_rtt(struct rpc_rtt *rt, unsigned long timeo)
+/**
+ * rpc_init_rtt - Initialize an RPC RTT estimator context
+ * @rt: context to initialize
+ * @timeo: initial timeout value, in jiffies
+ *
+ */
+void rpc_init_rtt(struct rpc_rtt *rt, unsigned long timeo)
 {
 	unsigned long init = 0;
 	unsigned i;
@@ -43,12 +48,16 @@ rpc_init_rtt(struct rpc_rtt *rt, unsigned long timeo)
 }
 EXPORT_SYMBOL_GPL(rpc_init_rtt);
 
-/*
+/**
+ * rpc_update_rtt - Update an RPC RTT estimator context
+ * @rt: context to update
+ * @timer: timer array index (request type)
+ * @m: recent actual RTT, in jiffies
+ *
  * NB: When computing the smoothed RTT and standard deviation,
  *     be careful not to produce negative intermediate results.
  */
-void
-rpc_update_rtt(struct rpc_rtt *rt, unsigned timer, long m)
+void rpc_update_rtt(struct rpc_rtt *rt, unsigned timer, long m)
 {
 	long *srtt, *sdrtt;
 
@@ -79,21 +88,25 @@ rpc_update_rtt(struct rpc_rtt *rt, unsigned timer, long m)
 }
 EXPORT_SYMBOL_GPL(rpc_update_rtt);
 
-/*
- * Estimate rto for an nfs rpc sent via. an unreliable datagram.
- * Use the mean and mean deviation of rtt for the appropriate type of rpc
- * for the frequent rpcs and a default for the others.
- * The justification for doing "other" this way is that these rpcs
- * happen so infrequently that timer est. would probably be stale.
- * Also, since many of these rpcs are
- * non-idempotent, a conservative timeout is desired.
+/**
+ * rpc_calc_rto - Provide an estimated timeout value
+ * @rt: context to use for calculation
+ * @timer: timer array index (request type)
+ *
+ * Estimate RTO for an NFS RPC sent via an unreliable datagram.  Use
+ * the mean and mean deviation of RTT for the appropriate type of RPC
+ * for frequently issued RPCs, and a fixed default for the others.
+ *
+ * The justification for doing "other" this way is that these RPCs
+ * happen so infrequently that timer estimation would probably be
+ * stale.  Also, since many of these rpcs are non-idempotent, a
+ * conservative timeout is desired.
+ *
  * getattr, lookup,
  * read, write, commit     - A+4D
  * other                   - timeo
  */
-
-unsigned long
-rpc_calc_rto(struct rpc_rtt *rt, unsigned timer)
+unsigned long rpc_calc_rto(struct rpc_rtt *rt, unsigned timer)
 {
 	unsigned long res;