From owner-freebsd-bugs@FreeBSD.ORG Tue Jun 7 11:40:28 2005 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2A5C016A422 for ; Tue, 7 Jun 2005 11:40:28 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6080F43D5C for ; Tue, 7 Jun 2005 11:40:27 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.3/8.13.3) with ESMTP id j57BeR79089380 for ; Tue, 7 Jun 2005 11:40:27 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.3/8.13.1/Submit) id j57BeR0g089379; Tue, 7 Jun 2005 11:40:27 GMT (envelope-from gnats) Resent-Date: Tue, 7 Jun 2005 11:40:27 GMT Resent-Message-Id: <200506071140.j57BeR0g089379@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Andrey Simonenko Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id BCE8316A42A for ; Tue, 7 Jun 2005 11:30:56 +0000 (GMT) (envelope-from simon@comsys.ntu-kpi.kiev.ua) Received: from comsys.ntu-kpi.kiev.ua (comsys.ntu-kpi.kiev.ua [195.245.194.142]) by mx1.FreeBSD.org (Postfix) with ESMTP id 9FCB143D55 for ; Tue, 7 Jun 2005 11:30:52 +0000 (GMT) (envelope-from simon@comsys.ntu-kpi.kiev.ua) Received: from pm514-9.comsys.ntu-kpi.kiev.ua (pm514-9.comsys.ntu-kpi.kiev.ua [10.18.54.109]) (authenticated bits=0) by comsys.ntu-kpi.kiev.ua (8.12.10/8.12.10) with ESMTP id j57BaZQk070296 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 7 Jun 2005 14:36:36 +0300 (EEST) Received: by pm514-9.comsys.ntu-kpi.kiev.ua (Postfix, from userid 1000) id 54F5D228; Tue, 7 Jun 2005 14:29:41 +0300 (EEST) Message-Id: <20050607112941.GA433@pm514-9.comsys.ntu-kpi.kiev.ua> Date: Tue, 7 Jun 2005 14:29:41 +0300 From: Andrey Simonenko To: FreeBSD-gnats-submit@FreeBSD.org Cc: Subject: bin/81987: [patch] memory leaks in libc/rpc X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Jun 2005 11:40:28 -0000 >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: