bin/81987: [patch] memory leaks in libc/rpc
Andrey Simonenko
simon at comsys.ntu-kpi.kiev.ua
Tue Jun 7 11:40:28 GMT 2005
>Number: 81987
>Category: bin
>Synopsis: [patch] memory leaks in libc/rpc
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Tue Jun 07 11:40:26 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator: Andrey Simonenko
>Release: FreeBSD 5.4-STABLE i386
>Organization:
>Environment:
>Description:
Several memory leaks and memory allocations without checking
return values in libc/rpc code were found and fixed.
Also several memory allocations without checking return values
are not fixed, firstly I want to hear opinion about this patch.
Why to strdup "tcp", "udp" and "inet" (see patch)? May be it
would be better to use const char * in struct endpoint{} for
family and proto?
>How-To-Repeat:
>Fix:
diff -ruN rpc.orig/auth_time.c rpc/auth_time.c
--- rpc.orig/auth_time.c Tue Jan 6 20:45:58 2004
+++ rpc/auth_time.c Fri May 20 17:31:02 2005
@@ -156,6 +156,7 @@
struct hostent *he;
struct hostent dummy;
char *ptr[2];
+ endpoint *ep;
if (host == NULL && sin == NULL)
return (NULL);
@@ -175,26 +176,34 @@
* This is lame. We go around once for TCP, then again
* for UDP.
*/
- for (i = 0; (he->h_addr_list[i] != NULL) && (num_ep < maxep);
- i++, num_ep++) {
+ for (i = 0, ep = eps; (he->h_addr_list[i] != NULL) && (num_ep < maxep);
+ i++, ep++, num_ep++) {
struct in_addr *a;
a = (struct in_addr *)he->h_addr_list[i];
snprintf(hname, sizeof(hname), "%s.0.111", inet_ntoa(*a));
- eps[num_ep].uaddr = strdup(hname);
- eps[num_ep].family = strdup("inet");
- eps[num_ep].proto = strdup("tcp");
+ ep->uaddr = strdup(hname);
+ ep->family = strdup("inet");
+ ep->proto = strdup("tcp");
+ if (ep->uaddr == NULL || ep->family == NULL || ep->proto == NULL) {
+ free_eps(eps, num_ep + 1);
+ return NULL;
+ }
}
for (i = 0; (he->h_addr_list[i] != NULL) && (num_ep < maxep);
- i++, num_ep++) {
+ i++, ep++, num_ep++) {
struct in_addr *a;
a = (struct in_addr *)he->h_addr_list[i];
snprintf(hname, sizeof(hname), "%s.0.111", inet_ntoa(*a));
- eps[num_ep].uaddr = strdup(hname);
- eps[num_ep].family = strdup("inet");
- eps[num_ep].proto = strdup("udp");
+ ep->uaddr = strdup(hname);
+ ep->family = strdup("inet");
+ ep->proto = strdup("udp");
+ if (ep->uaddr == NULL || ep->family == NULL || ep->proto == NULL) {
+ free_eps(eps, num_ep + 1);
+ return NULL;
+ }
}
srv->name = (nis_name) host;
diff -ruN rpc.orig/getnetconfig.c rpc/getnetconfig.c
--- rpc.orig/getnetconfig.c Fri Mar 5 10:10:17 2004
+++ rpc/getnetconfig.c Mon May 23 16:35:30 2005
@@ -536,6 +536,7 @@
{
char *tokenp; /* for processing tokens */
char *lasts;
+ char **nc_lookups;
nc_error = NC_BADFILE; /* nearly anything that breaks is for this reason */
stringp[strlen(stringp)-1] = '\0'; /* get rid of newline */
@@ -601,14 +602,18 @@
if (ncp->nc_lookups != NULL) /* from last visit */
free(ncp->nc_lookups);
- /* preallocate one string pointer */
- ncp->nc_lookups = (char **)malloc(sizeof (char *));
+ ncp->nc_lookups = NULL;
ncp->nc_nlookups = 0;
while ((cp = tokenp) != NULL) {
+ if ((nc_lookups = realloc(ncp->nc_lookups,
+ (ncp->nc_nlookups + 1) * sizeof *ncp->nc_lookups)) == NULL) {
+ free(ncp->nc_lookups);
+ ncp->nc_lookups = NULL;
+ return (-1);
+ }
tokenp = _get_next_token(cp, ',');
- ncp->nc_lookups[(size_t)ncp->nc_nlookups++] = cp;
- ncp->nc_lookups = (char **)realloc(ncp->nc_lookups,
- (size_t)(ncp->nc_nlookups+1) *sizeof(char *)); /* for next loop */
+ ncp->nc_lookups = nc_lookups;
+ ncp->nc_lookups[ncp->nc_nlookups++] = cp;
}
}
return (0);
@@ -693,6 +698,7 @@
p->nc_lookups = (char **)malloc((size_t)(p->nc_nlookups+1) * sizeof(char *));
if (p->nc_lookups == NULL) {
free(p->nc_netid);
+ free(p);
return(NULL);
}
for (i=0; i < p->nc_nlookups; i++) {
diff -ruN rpc.orig/getnetpath.c rpc/getnetpath.c
--- rpc.orig/getnetpath.c Tue Jan 28 00:36:53 2003
+++ rpc/getnetpath.c Fri May 20 17:16:27 2005
@@ -102,7 +102,7 @@
}
if ((np_sessionp->nc_handlep = setnetconfig()) == NULL) {
syslog (LOG_ERR, "rpc: failed to open " NETCONFIG);
- return (NULL);
+ goto failed;
}
np_sessionp->valid = NP_VALID;
np_sessionp->ncp_list = NULL;
@@ -111,15 +111,18 @@
} else {
(void) endnetconfig(np_sessionp->nc_handlep);/* won't need nc session*/
np_sessionp->nc_handlep = NULL;
- if ((np_sessionp->netpath = malloc(strlen(npp)+1)) == NULL) {
- free(np_sessionp);
- return (NULL);
- } else {
+ if ((np_sessionp->netpath = malloc(strlen(npp)+1)) == NULL)
+ goto failed;
+ else {
(void) strcpy(np_sessionp->netpath, npp);
}
}
np_sessionp->netpath_start = np_sessionp->netpath;
return ((void *)np_sessionp);
+
+failed:
+ free(np_sessionp);
+ return (NULL);
}
/*
diff -ruN rpc.orig/rpc_generic.c rpc/rpc_generic.c
--- rpc.orig/rpc_generic.c Wed Oct 29 11:20:33 2003
+++ rpc/rpc_generic.c Wed May 18 11:29:59 2005
@@ -319,10 +319,8 @@
case _RPC_NETPATH:
case _RPC_CIRCUIT_N:
case _RPC_DATAGRAM_N:
- if (!(handle->nhandle = setnetpath())) {
- free(handle);
- return (NULL);
- }
+ if (!(handle->nhandle = setnetpath()))
+ goto failed;
handle->nflag = TRUE;
break;
case _RPC_VISIBLE:
@@ -332,16 +330,19 @@
case _RPC_UDP:
if (!(handle->nhandle = setnetconfig())) {
syslog (LOG_ERR, "rpc: failed to open " NETCONFIG);
- free(handle);
- return (NULL);
+ goto failed;
}
handle->nflag = FALSE;
break;
default:
- return (NULL);
+ goto failed;
}
return (handle);
+
+failed:
+ free(handle);
+ return (NULL);
}
/*
diff -ruN rpc.orig/rpcb_clnt.c rpc/rpcb_clnt.c
--- rpc.orig/rpcb_clnt.c Wed Oct 29 11:22:49 2003
+++ rpc/rpcb_clnt.c Wed May 18 12:45:55 2005
@@ -237,15 +237,16 @@
ad_cache->ac_netid = strdup(netid);
ad_cache->ac_uaddr = uaddr ? strdup(uaddr) : NULL;
ad_cache->ac_taddr = (struct netbuf *)malloc(sizeof (struct netbuf));
+ if (ad_cache->ac_taddr != NULL) {
+ ad_cache->ac_taddr->len = ad_cache->ac_taddr->maxlen = taddr->len;
+ if ((ad_cache->ac_taddr->buf = malloc(taddr->len)) == NULL)
+ goto failed;
+ }
if (!ad_cache->ac_host || !ad_cache->ac_netid || !ad_cache->ac_taddr ||
(uaddr && !ad_cache->ac_uaddr)) {
- return;
- }
- ad_cache->ac_taddr->len = ad_cache->ac_taddr->maxlen = taddr->len;
- ad_cache->ac_taddr->buf = (char *) malloc(taddr->len);
- if (ad_cache->ac_taddr->buf == NULL) {
- return;
+ goto failed;
}
+
memcpy(ad_cache->ac_taddr->buf, taddr->buf, taddr->len);
#ifdef ND_DEBUG
fprintf(stderr, "Added to cache: %s : %s\n", host, netid);
@@ -289,6 +290,16 @@
free(cptr);
}
rwlock_unlock(&rpcbaddr_cache_lock);
+ return;
+
+failed:
+ free(ad_cache->ac_host);
+ free(ad_cache->ac_netid);
+ free(ad_cache->ac_uaddr);
+ if (ad_cache->ac_taddr != NULL) {
+ free(ad_cache->ac_taddr->buf);
+ free(ad_cache->ac_taddr);
+ }
}
/*
>Release-Note:
>Audit-Trail:
>Unformatted:
More information about the freebsd-bugs
mailing list