There patches close a number of memory leaks that
were found using a covscan. Most them were in error
recovery code but a few of them were in main line code.
Steve Dickson (13):
auth_gss.c: resource_leak
auth_gss.c: buffer_size_warning
clnt_bcast.c: resource_leak
clnt_vc.c: resource_leak
getnetconfig.c: cppcheck_warning
getnetpath.c: resource_leak
rpc_generic.c: resource_leak
rpc_soc.c: resource_leak
rpc_soc.c: buffer_size_warning
rpcb_clnt.c: resource_leak
rtime.c: resource_leak
svc_generic.c: resource_leak
svc_simple.c: resource_leak
src/auth_gss.c | 3 ++-
src/clnt_bcast.c | 1 +
src/clnt_vc.c | 2 ++
src/getnetconfig.c | 2 ++
src/getnetpath.c | 1 +
src/rpc_generic.c | 1 +
src/rpc_soc.c | 8 ++++++--
src/rpcb_clnt.c | 1 +
src/rtime.c | 1 +
src/svc_generic.c | 1 +
src/svc_simple.c | 1 +
11 files changed, 19 insertions(+), 3 deletions(-)
--
2.17.1
Variable "gd" going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
src/auth_gss.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/auth_gss.c b/src/auth_gss.c
index 5959893..289bd5b 100644
--- a/src/auth_gss.c
+++ b/src/auth_gss.c
@@ -207,6 +207,7 @@ authgss_create(CLIENT *clnt, gss_name_t name, struct rpc_gss_sec *sec)
rpc_createerr.cf_stat = RPC_SYSTEMERROR;
rpc_createerr.cf_error.re_errno = ENOMEM;
free(auth);
+ free(gd);
return (NULL);
}
}
--
2.17.1
Variable "sys_auth" going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
src/clnt_bcast.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/clnt_bcast.c b/src/clnt_bcast.c
index 98cf061..2ad6c89 100644
--- a/src/clnt_bcast.c
+++ b/src/clnt_bcast.c
@@ -330,6 +330,7 @@ rpc_broadcast_exp(prog, vers, proc, xargs, argsp, xresults, resultsp,
if (nettype == NULL)
nettype = "datagram_n";
if ((handle = __rpc_setconf(nettype)) == NULL) {
+ AUTH_DESTROY(sys_auth);
return (RPC_UNKNOWNPROTO);
}
while ((nconf = __rpc_getconf(handle)) != NULL) {
--
2.17.1
Calling strncpy with a maximum size argument of 128 bytes on
destination array "options_ret->actual_mechanism" of size 128
bytes might leave the destination string unterminated
Signed-off-by: Steve Dickson <[email protected]>
---
src/auth_gss.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/auth_gss.c b/src/auth_gss.c
index 289bd5b..7d08262 100644
--- a/src/auth_gss.c
+++ b/src/auth_gss.c
@@ -593,7 +593,7 @@ _rpc_gss_refresh(AUTH *auth, rpc_gss_options_ret_t *options_ret)
if (rpc_gss_oid_to_mech(actual_mech_type, &mechanism)) {
strncpy(options_ret->actual_mechanism,
mechanism,
- sizeof(options_ret->actual_mechanism));
+ (sizeof(options_ret->actual_mechanism)-1));
}
gd->established = TRUE;
--
2.17.1
Variable "handle" going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
src/svc_generic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/svc_generic.c b/src/svc_generic.c
index 52a56c2..20abaa2 100644
--- a/src/svc_generic.c
+++ b/src/svc_generic.c
@@ -113,6 +113,7 @@ svc_create(dispatch, prognum, versnum, nettype)
if (l == NULL) {
warnx("svc_create: no memory");
mutex_unlock(&xprtlist_lock);
+ __rpc_endconf(handle);
return (0);
}
l->xprt = xprt;
--
2.17.1
Memory leak: p
Memory leak: tmp
Signed-off-by: Steve Dickson <[email protected]>
---
src/getnetconfig.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/getnetconfig.c b/src/getnetconfig.c
index 92e7c43..d67d97d 100644
--- a/src/getnetconfig.c
+++ b/src/getnetconfig.c
@@ -709,6 +709,8 @@ struct netconfig *ncp;
p->nc_lookups = (char **)malloc((size_t)(p->nc_nlookups+1) * sizeof(char *));
if (p->nc_lookups == NULL) {
free(p->nc_netid);
+ free(p);
+ free(tmp);
return(NULL);
}
for (i=0; i < p->nc_nlookups; i++) {
--
2.17.1
Calling strncpy with a maximum size argument of 108 bytes on
destination array "sun.sun_path" of size 108 bytes might
leave the destination string unterminated.
Signed-off-by: Steve Dickson <[email protected]>
---
src/rpc_soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/rpc_soc.c b/src/rpc_soc.c
index 59e0882..a85cb17 100644
--- a/src/rpc_soc.c
+++ b/src/rpc_soc.c
@@ -673,7 +673,7 @@ svcunix_create(sock, sendsize, recvsize, path)
memset(&sun, 0, sizeof sun);
sun.sun_family = AF_LOCAL;
- strncpy(sun.sun_path, path, sizeof(sun.sun_path));
+ strncpy(sun.sun_path, path, (sizeof(sun.sun_path)-1));
addrlen = sizeof(struct sockaddr_un);
sa = (struct sockaddr *)&sun;
--
2.17.1
Variable "handle" going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
src/rpc_generic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/rpc_generic.c b/src/rpc_generic.c
index 589cbd5..51f36ac 100644
--- a/src/rpc_generic.c
+++ b/src/rpc_generic.c
@@ -319,6 +319,7 @@ __rpc_setconf(nettype)
handle->nflag = FALSE;
break;
default:
+ free(handle);
return (NULL);
}
--
2.17.1
Variable "localhandle" going out of scope leaks the storage it points to.
Returning without closing handle "sock" leaks it.
Signed-off-by: Steve Dickson <[email protected]>
---
src/rpc_soc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/rpc_soc.c b/src/rpc_soc.c
index 5a6eeb7..59e0882 100644
--- a/src/rpc_soc.c
+++ b/src/rpc_soc.c
@@ -663,8 +663,10 @@ svcunix_create(sock, sendsize, recvsize, path)
strcmp(nconf->nc_protofmly, NC_LOOPBACK) == 0)
break;
}
- if (nconf == NULL)
+ if (nconf == NULL) {
+ endnetconfig(localhandle);
return(xprt);
+ }
if ((sock = __rpc_nconf2fd(nconf)) < 0)
goto done;
@@ -692,6 +694,8 @@ svcunix_create(sock, sendsize, recvsize, path)
}
xprt = (SVCXPRT *)svc_tli_create(sock, nconf, &taddr, sendsize, recvsize);
+ if (xprt == NULL)
+ close(sock);
done:
endnetconfig(localhandle);
--
2.17.1
Variable "nc_handle" going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
src/rpcb_clnt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/rpcb_clnt.c b/src/rpcb_clnt.c
index e45736a..0c34cb7 100644
--- a/src/rpcb_clnt.c
+++ b/src/rpcb_clnt.c
@@ -547,6 +547,7 @@ try_nconf:
if (tmpnconf == NULL) {
rpc_createerr.cf_stat = RPC_UNKNOWNPROTO;
mutex_unlock(&loopnconf_lock);
+ endnetconfig(nc_handle);
return (NULL);
}
loopnconf = getnetconfigent(tmpnconf->nc_netid);
--
2.17.1
Variable "ct" going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
src/clnt_vc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/clnt_vc.c b/src/clnt_vc.c
index 3d775c7..10ee91a 100644
--- a/src/clnt_vc.c
+++ b/src/clnt_vc.c
@@ -325,6 +325,8 @@ clnt_vc_create(fd, raddr, prog, vers, sendsz, recvsz)
recvsz = __rpc_get_t_size(si.si_af, si.si_proto, (int)recvsz);
xdrrec_create(&(ct->ct_xdrs), sendsz, recvsz,
cl->cl_private, read_vc, write_vc);
+ mem_free(ct->ct_addr.buf, ct->ct_addr.len);
+ mem_free(ct, sizeof (struct ct_data));
return (cl);
err:
--
2.17.1
Handle variable "s" going out of scope leaks the handle.
Signed-off-by: Steve Dickson <[email protected]>
---
src/rtime.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/rtime.c b/src/rtime.c
index b642840..29fbf0a 100644
--- a/src/rtime.c
+++ b/src/rtime.c
@@ -90,6 +90,7 @@ rtime(addrp, timep, timeout)
/* TCP and UDP port are the same in this case */
if ((serv = getservbyname("time", "tcp")) == NULL) {
+ do_close(s);
return(-1);
}
--
2.17.1
Variable "xdrbuf" going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
src/svc_simple.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/svc_simple.c b/src/svc_simple.c
index cb58002..c32fe0a 100644
--- a/src/svc_simple.c
+++ b/src/svc_simple.c
@@ -157,6 +157,7 @@ rpc_reg(prognum, versnum, procnum, progname, inproc, outproc, nettype)
((netid = strdup(nconf->nc_netid)) == NULL)) {
warnx(rpc_reg_err, rpc_reg_msg, __no_mem_str);
SVC_DESTROY(svcxprt);
+ free(xdrbuf);
break;
}
madenow = TRUE;
--
2.17.1
Variable "np_sessionp" going out of scope leaks the storage it points to.
Signed-off-by: Steve Dickson <[email protected]>
---
src/getnetpath.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/getnetpath.c b/src/getnetpath.c
index 7c19932..ea1a18c 100644
--- a/src/getnetpath.c
+++ b/src/getnetpath.c
@@ -88,6 +88,7 @@ setnetpath()
}
if ((np_sessionp->nc_handlep = setnetconfig()) == NULL) {
syslog (LOG_ERR, "rpc: failed to open " NETCONFIG);
+ free(np_sessionp);
return (NULL);
}
np_sessionp->valid = NP_VALID;
--
2.17.1
On 09/07/2018 02:01 PM, Steve Dickson wrote:
> There patches close a number of memory leaks that
> were found using a covscan. Most them were in error
> recovery code but a few of them were in main line code.
>
>
> Steve Dickson (13):
> auth_gss.c: resource_leak
> auth_gss.c: buffer_size_warning
> clnt_bcast.c: resource_leak
> clnt_vc.c: resource_leak
> getnetconfig.c: cppcheck_warning
> getnetpath.c: resource_leak
> rpc_generic.c: resource_leak
> rpc_soc.c: resource_leak
> rpc_soc.c: buffer_size_warning
> rpcb_clnt.c: resource_leak
> rtime.c: resource_leak
> svc_generic.c: resource_leak
> svc_simple.c: resource_leak
>
> src/auth_gss.c | 3 ++-
> src/clnt_bcast.c | 1 +
> src/clnt_vc.c | 2 ++
> src/getnetconfig.c | 2 ++
> src/getnetpath.c | 1 +
> src/rpc_generic.c | 1 +
> src/rpc_soc.c | 8 ++++++--
> src/rpcb_clnt.c | 1 +
> src/rtime.c | 1 +
> src/svc_generic.c | 1 +
> src/svc_simple.c | 1 +
> 11 files changed, 19 insertions(+), 3 deletions(-)
>
Committed (tag: libtirpc-1-1-5-rc1)
On Fri, 2018-09-07 at 14:01 -0400, Steve Dickson wrote:
> Variable "ct" going out of scope leaks the storage it points to.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> src/clnt_vc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/src/clnt_vc.c b/src/clnt_vc.c
> index 3d775c7..10ee91a 100644
> --- a/src/clnt_vc.c
> +++ b/src/clnt_vc.c
> @@ -325,6 +325,8 @@ clnt_vc_create(fd, raddr, prog, vers, sendsz, recvsz)
> recvsz = __rpc_get_t_size(si.si_af, si.si_proto, (int)recvsz);
> xdrrec_create(&(ct->ct_xdrs), sendsz, recvsz,
> cl->cl_private, read_vc, write_vc);
> + mem_free(ct->ct_addr.buf, ct->ct_addr.len);
> + mem_free(ct, sizeof (struct ct_data));
> return (cl);
>
> err:
Are you sure about this one Steve?
aka:
/*
* Create a client handle which uses xdrrec for serialization
* and authnone for authentication.
*/
cl->cl_ops = clnt_vc_ops();
cl->cl_private = ct; <------?
cl->cl_auth = authnone_create();
sendsz = __rpc_get_t_size(si.si_af, si.si_proto, (int)sendsz);
recvsz = __rpc_get_t_size(si.si_af, si.si_proto, (int)recvsz);
xdrrec_create(&(ct->ct_xdrs), sendsz, recvsz,
cl->cl_private, read_vc, write_vc);
return (cl);
On Mon, 2018-10-29 at 14:24 +0800, Ian Kent wrote:
> On Fri, 2018-09-07 at 14:01 -0400, Steve Dickson wrote:
> > Variable "ct" going out of scope leaks the storage it points to.
> >
> > Signed-off-by: Steve Dickson <[email protected]>
> > ---
> > src/clnt_vc.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/src/clnt_vc.c b/src/clnt_vc.c
> > index 3d775c7..10ee91a 100644
> > --- a/src/clnt_vc.c
> > +++ b/src/clnt_vc.c
> > @@ -325,6 +325,8 @@ clnt_vc_create(fd, raddr, prog, vers, sendsz, recvsz)
> > recvsz = __rpc_get_t_size(si.si_af, si.si_proto, (int)recvsz);
> > xdrrec_create(&(ct->ct_xdrs), sendsz, recvsz,
> > cl->cl_private, read_vc, write_vc);
> > + mem_free(ct->ct_addr.buf, ct->ct_addr.len);
> > + mem_free(ct, sizeof (struct ct_data));
> > return (cl);
> >
> > err:
>
> Are you sure about this one Steve?
>
> aka:
> /*
> * Create a client handle which uses xdrrec for serialization
> * and authnone for authentication.
> */
> cl->cl_ops = clnt_vc_ops();
> cl->cl_private = ct; <------?
> cl->cl_auth = authnone_create();
> sendsz = __rpc_get_t_size(si.si_af, si.si_proto, (int)sendsz);
> recvsz = __rpc_get_t_size(si.si_af, si.si_proto, (int)recvsz);
> xdrrec_create(&(ct->ct_xdrs), sendsz, recvsz,
> cl->cl_private, read_vc, write_vc);
> return (cl);
Oh!
My bad, reverted in commit e49077d2fa.