Linux NFS development
 help / color / mirror / Atom feed
* [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer.
@ 2017-05-25 20:35 Steve Dickson
  2017-05-25 20:35 ` [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer Steve Dickson
  2017-05-30 16:45 ` [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson
  0 siblings, 2 replies; 4+ messages in thread
From: Steve Dickson @ 2017-05-25 20:35 UTC (permalink / raw)
  To: Linux NFS Mailing list

commit 7ea36ee introduced a svc_freeargs() call
that ended up freeing static pointer.

It turns out the allocations for the rmt_args
is not necessary . The xdr routines (xdr_bytes) will
handle the memory management and the largest
possible message size is UDPMSGSIZE (due to UDP only)
which is smaller than RPC_BUF_MAX

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 src/rpcb_svc_com.c | 39 ++++++---------------------------------
 1 file changed, 6 insertions(+), 33 deletions(-)

diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c
index cb63afd..1fc2229 100644
--- a/src/rpcb_svc_com.c
+++ b/src/rpcb_svc_com.c
@@ -612,9 +612,9 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 	struct netconfig *nconf;
 	struct netbuf *caller;
 	struct r_rmtcall_args a;
-	char *buf_alloc = NULL, *outbufp;
+	char *outbufp;
 	char *outbuf_alloc = NULL;
-	char buf[RPC_BUF_MAX], outbuf[RPC_BUF_MAX];
+	char  outbuf[RPC_BUF_MAX];
 	struct netbuf *na = (struct netbuf *) NULL;
 	struct rpc_msg call_msg;
 	int outlen;
@@ -635,36 +635,10 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 	}
 	if (si.si_socktype != SOCK_DGRAM)
 		return;	/* Only datagram type accepted */
-	sendsz = __rpc_get_t_size(si.si_af, si.si_proto, UDPMSGSIZE);
-	if (sendsz == 0) {	/* data transfer not supported */
-		if (reply_type == RPCBPROC_INDIRECT)
-			svcerr_systemerr(transp);
-		return;
-	}
-	/*
-	 * Should be multiple of 4 for XDR.
-	 */
-	sendsz = ((sendsz + 3) / 4) * 4;
-	if (sendsz > RPC_BUF_MAX) {
-#ifdef	notyet
-		buf_alloc = alloca(sendsz);		/* not in IDR2? */
-#else
-		buf_alloc = malloc(sendsz);
-#endif	/* notyet */
-		if (buf_alloc == NULL) {
-			if (debugging)
-				xlog(LOG_DEBUG,
-					"rpcbproc_callit_com:  No Memory!\n");
-			if (reply_type == RPCBPROC_INDIRECT)
-				svcerr_systemerr(transp);
-			return;
-		}
-		a.rmt_args.args = buf_alloc;
-	} else {
-		a.rmt_args.args = buf;
-	}
+	sendsz = UDPMSGSIZE;
 
 	call_msg.rm_xid = 0;	/* For error checking purposes */
+	memset(&a, 0, sizeof(a)); /* Zero out the input buffer */
 	if (!svc_getargs(transp, (xdrproc_t) xdr_rmtcall_args, (char *) &a)) {
 		if (reply_type == RPCBPROC_INDIRECT)
 			svcerr_decode(transp);
@@ -704,7 +678,8 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 	if (rbl == (rpcblist_ptr)NULL) {
 #ifdef RPCBIND_DEBUG
 		if (debugging)
-			xlog(LOG_DEBUG, "not found\n");
+			xlog(LOG_DEBUG, "prog %lu vers %lu: not found\n", 
+				a.rmt_prog, a.rmt_vers);
 #endif
 		if (reply_type == RPCBPROC_INDIRECT)
 			svcerr_noprog(transp);
@@ -937,8 +912,6 @@ out:
 	}
 	if (local_uaddr)
 		free(local_uaddr);
-	if (buf_alloc)
-		free(buf_alloc);
 	if (outbuf_alloc)
 		free(outbuf_alloc);
 	if (na) {
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer
  2017-05-25 20:35 [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson
@ 2017-05-25 20:35 ` Steve Dickson
  2017-05-30 16:45   ` Steve Dickson
  2017-05-30 16:45 ` [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson
  1 sibling, 1 reply; 4+ messages in thread
From: Steve Dickson @ 2017-05-25 20:35 UTC (permalink / raw)
  To: Linux NFS Mailing list

Now that sendz is a fixed size (UDPMSGSIZE) which
is small then RPC_BUF_MAX, no need to check the
sendz size.

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 src/rpcb_svc_com.c | 33 +++++----------------------------
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c
index 1fc2229..d36b090 100644
--- a/src/rpcb_svc_com.c
+++ b/src/rpcb_svc_com.c
@@ -612,8 +612,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 	struct netconfig *nconf;
 	struct netbuf *caller;
 	struct r_rmtcall_args a;
-	char *outbufp;
-	char *outbuf_alloc = NULL;
 	char  outbuf[RPC_BUF_MAX];
 	struct netbuf *na = (struct netbuf *) NULL;
 	struct rpc_msg call_msg;
@@ -674,7 +672,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 
 	rpcbs_rmtcall(versnum - 2, reply_type, a.rmt_prog, a.rmt_vers,
 			a.rmt_proc, transp->xp_netid, rbl);
-
 	if (rbl == (rpcblist_ptr)NULL) {
 #ifdef RPCBIND_DEBUG
 		if (debugging)
@@ -793,24 +790,10 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 	call_msg.rm_call.cb_rpcvers = RPC_MSG_VERSION;
 	call_msg.rm_call.cb_prog = a.rmt_prog;
 	call_msg.rm_call.cb_vers = a.rmt_vers;
-	if (sendsz > RPC_BUF_MAX) {
-#ifdef	notyet
-		outbuf_alloc = alloca(sendsz);	/* not in IDR2? */
-#else
-		outbuf_alloc = malloc(sendsz);
-#endif	/* notyet */
-		if (outbuf_alloc == NULL) {
-			if (reply_type == RPCBPROC_INDIRECT)
-				svcerr_systemerr(transp);
-			if (debugging)
-				xlog(LOG_DEBUG,
-				"rpcbproc_callit_com:  No memory!\n");
-			goto error;
-		}
-		xdrmem_create(&outxdr, outbuf_alloc, sendsz, XDR_ENCODE);
-	} else {
-		xdrmem_create(&outxdr, outbuf, sendsz, XDR_ENCODE);
-	}
+
+	memset(outbuf, '\0', sendsz); /* Zero out the output buffer */
+	xdrmem_create(&outxdr, outbuf, sendsz, XDR_ENCODE);
+
 	if (!xdr_callhdr(&outxdr, &call_msg)) {
 		if (reply_type == RPCBPROC_INDIRECT)
 			svcerr_systemerr(transp);
@@ -875,10 +858,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 		goto error;
 	}
 	outlen = (int) XDR_GETPOS(&outxdr);
-	if (outbuf_alloc)
-		outbufp = outbuf_alloc;
-	else
-		outbufp = outbuf;
 
 	na = uaddr2taddr(nconf, local_uaddr);
 	if (!na) {
@@ -887,7 +866,7 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
 		goto error;
 	}
 
-	if (sendto(fd, outbufp, outlen, 0, (struct sockaddr *)na->buf, na->len)
+	if (sendto(fd, outbuf, outlen, 0, (struct sockaddr *)na->buf, na->len)
 	    != outlen) {
 		if (debugging)
 			xlog(LOG_DEBUG,
@@ -912,8 +891,6 @@ out:
 	}
 	if (local_uaddr)
 		free(local_uaddr);
-	if (outbuf_alloc)
-		free(outbuf_alloc);
 	if (na) {
 		free(na->buf);
 		free(na);
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer.
  2017-05-25 20:35 [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson
  2017-05-25 20:35 ` [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer Steve Dickson
@ 2017-05-30 16:45 ` Steve Dickson
  1 sibling, 0 replies; 4+ messages in thread
From: Steve Dickson @ 2017-05-30 16:45 UTC (permalink / raw)
  To: Linux NFS Mailing list



On 05/25/2017 04:35 PM, Steve Dickson wrote:
> commit 7ea36ee introduced a svc_freeargs() call
> that ended up freeing static pointer.
> 
> It turns out the allocations for the rmt_args
> is not necessary . The xdr routines (xdr_bytes) will
> handle the memory management and the largest
> possible message size is UDPMSGSIZE (due to UDP only)
> which is smaller than RPC_BUF_MAX
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
Committed.... 

steved.
> ---
>  src/rpcb_svc_com.c | 39 ++++++---------------------------------
>  1 file changed, 6 insertions(+), 33 deletions(-)
> 
> diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c
> index cb63afd..1fc2229 100644
> --- a/src/rpcb_svc_com.c
> +++ b/src/rpcb_svc_com.c
> @@ -612,9 +612,9 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
>  	struct netconfig *nconf;
>  	struct netbuf *caller;
>  	struct r_rmtcall_args a;
> -	char *buf_alloc = NULL, *outbufp;
> +	char *outbufp;
>  	char *outbuf_alloc = NULL;
> -	char buf[RPC_BUF_MAX], outbuf[RPC_BUF_MAX];
> +	char  outbuf[RPC_BUF_MAX];
>  	struct netbuf *na = (struct netbuf *) NULL;
>  	struct rpc_msg call_msg;
>  	int outlen;
> @@ -635,36 +635,10 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
>  	}
>  	if (si.si_socktype != SOCK_DGRAM)
>  		return;	/* Only datagram type accepted */
> -	sendsz = __rpc_get_t_size(si.si_af, si.si_proto, UDPMSGSIZE);
> -	if (sendsz == 0) {	/* data transfer not supported */
> -		if (reply_type == RPCBPROC_INDIRECT)
> -			svcerr_systemerr(transp);
> -		return;
> -	}
> -	/*
> -	 * Should be multiple of 4 for XDR.
> -	 */
> -	sendsz = ((sendsz + 3) / 4) * 4;
> -	if (sendsz > RPC_BUF_MAX) {
> -#ifdef	notyet
> -		buf_alloc = alloca(sendsz);		/* not in IDR2? */
> -#else
> -		buf_alloc = malloc(sendsz);
> -#endif	/* notyet */
> -		if (buf_alloc == NULL) {
> -			if (debugging)
> -				xlog(LOG_DEBUG,
> -					"rpcbproc_callit_com:  No Memory!\n");
> -			if (reply_type == RPCBPROC_INDIRECT)
> -				svcerr_systemerr(transp);
> -			return;
> -		}
> -		a.rmt_args.args = buf_alloc;
> -	} else {
> -		a.rmt_args.args = buf;
> -	}
> +	sendsz = UDPMSGSIZE;
>  
>  	call_msg.rm_xid = 0;	/* For error checking purposes */
> +	memset(&a, 0, sizeof(a)); /* Zero out the input buffer */
>  	if (!svc_getargs(transp, (xdrproc_t) xdr_rmtcall_args, (char *) &a)) {
>  		if (reply_type == RPCBPROC_INDIRECT)
>  			svcerr_decode(transp);
> @@ -704,7 +678,8 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
>  	if (rbl == (rpcblist_ptr)NULL) {
>  #ifdef RPCBIND_DEBUG
>  		if (debugging)
> -			xlog(LOG_DEBUG, "not found\n");
> +			xlog(LOG_DEBUG, "prog %lu vers %lu: not found\n", 
> +				a.rmt_prog, a.rmt_vers);
>  #endif
>  		if (reply_type == RPCBPROC_INDIRECT)
>  			svcerr_noprog(transp);
> @@ -937,8 +912,6 @@ out:
>  	}
>  	if (local_uaddr)
>  		free(local_uaddr);
> -	if (buf_alloc)
> -		free(buf_alloc);
>  	if (outbuf_alloc)
>  		free(outbuf_alloc);
>  	if (na) {
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer
  2017-05-25 20:35 ` [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer Steve Dickson
@ 2017-05-30 16:45   ` Steve Dickson
  0 siblings, 0 replies; 4+ messages in thread
From: Steve Dickson @ 2017-05-30 16:45 UTC (permalink / raw)
  To: Linux NFS Mailing list



On 05/25/2017 04:35 PM, Steve Dickson wrote:
> Now that sendz is a fixed size (UDPMSGSIZE) which
> is small then RPC_BUF_MAX, no need to check the
> sendz size.
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
Committed...

steved.

> ---
>  src/rpcb_svc_com.c | 33 +++++----------------------------
>  1 file changed, 5 insertions(+), 28 deletions(-)
> 
> diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c
> index 1fc2229..d36b090 100644
> --- a/src/rpcb_svc_com.c
> +++ b/src/rpcb_svc_com.c
> @@ -612,8 +612,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
>  	struct netconfig *nconf;
>  	struct netbuf *caller;
>  	struct r_rmtcall_args a;
> -	char *outbufp;
> -	char *outbuf_alloc = NULL;
>  	char  outbuf[RPC_BUF_MAX];
>  	struct netbuf *na = (struct netbuf *) NULL;
>  	struct rpc_msg call_msg;
> @@ -674,7 +672,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
>  
>  	rpcbs_rmtcall(versnum - 2, reply_type, a.rmt_prog, a.rmt_vers,
>  			a.rmt_proc, transp->xp_netid, rbl);
> -
>  	if (rbl == (rpcblist_ptr)NULL) {
>  #ifdef RPCBIND_DEBUG
>  		if (debugging)
> @@ -793,24 +790,10 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
>  	call_msg.rm_call.cb_rpcvers = RPC_MSG_VERSION;
>  	call_msg.rm_call.cb_prog = a.rmt_prog;
>  	call_msg.rm_call.cb_vers = a.rmt_vers;
> -	if (sendsz > RPC_BUF_MAX) {
> -#ifdef	notyet
> -		outbuf_alloc = alloca(sendsz);	/* not in IDR2? */
> -#else
> -		outbuf_alloc = malloc(sendsz);
> -#endif	/* notyet */
> -		if (outbuf_alloc == NULL) {
> -			if (reply_type == RPCBPROC_INDIRECT)
> -				svcerr_systemerr(transp);
> -			if (debugging)
> -				xlog(LOG_DEBUG,
> -				"rpcbproc_callit_com:  No memory!\n");
> -			goto error;
> -		}
> -		xdrmem_create(&outxdr, outbuf_alloc, sendsz, XDR_ENCODE);
> -	} else {
> -		xdrmem_create(&outxdr, outbuf, sendsz, XDR_ENCODE);
> -	}
> +
> +	memset(outbuf, '\0', sendsz); /* Zero out the output buffer */
> +	xdrmem_create(&outxdr, outbuf, sendsz, XDR_ENCODE);
> +
>  	if (!xdr_callhdr(&outxdr, &call_msg)) {
>  		if (reply_type == RPCBPROC_INDIRECT)
>  			svcerr_systemerr(transp);
> @@ -875,10 +858,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
>  		goto error;
>  	}
>  	outlen = (int) XDR_GETPOS(&outxdr);
> -	if (outbuf_alloc)
> -		outbufp = outbuf_alloc;
> -	else
> -		outbufp = outbuf;
>  
>  	na = uaddr2taddr(nconf, local_uaddr);
>  	if (!na) {
> @@ -887,7 +866,7 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp,
>  		goto error;
>  	}
>  
> -	if (sendto(fd, outbufp, outlen, 0, (struct sockaddr *)na->buf, na->len)
> +	if (sendto(fd, outbuf, outlen, 0, (struct sockaddr *)na->buf, na->len)
>  	    != outlen) {
>  		if (debugging)
>  			xlog(LOG_DEBUG,
> @@ -912,8 +891,6 @@ out:
>  	}
>  	if (local_uaddr)
>  		free(local_uaddr);
> -	if (outbuf_alloc)
> -		free(outbuf_alloc);
>  	if (na) {
>  		free(na->buf);
>  		free(na);
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-05-30 16:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-25 20:35 [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson
2017-05-25 20:35 ` [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer Steve Dickson
2017-05-30 16:45   ` Steve Dickson
2017-05-30 16:45 ` [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox