All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.