From owner-freebsd-bugs@FreeBSD.ORG Thu Jul 14 16:00:35 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 EB91E16A41C for ; Thu, 14 Jul 2005 16:00:35 +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 6E50443D53 for ; Thu, 14 Jul 2005 16:00:35 +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 j6EG0Znj028378 for ; Thu, 14 Jul 2005 16:00:35 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.3/8.13.1/Submit) id j6EG0ZXQ028377; Thu, 14 Jul 2005 16:00:35 GMT (envelope-from gnats) Resent-Date: Thu, 14 Jul 2005 16:00:35 GMT Resent-Message-Id: <200507141600.j6EG0ZXQ028377@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, Dan Lukes Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 4F7A116A41C for ; Thu, 14 Jul 2005 15:59:13 +0000 (GMT) (envelope-from dan@kulesh.obluda.cz) Received: from kulesh.obluda.cz (kulesh.obluda.cz [193.179.22.243]) by mx1.FreeBSD.org (Postfix) with ESMTP id DDE5D43D48 for ; Thu, 14 Jul 2005 15:59:11 +0000 (GMT) (envelope-from dan@kulesh.obluda.cz) Received: from kulesh.obluda.cz (localhost.eunet.cz [127.0.0.1]) by kulesh.obluda.cz (8.13.3/8.13.3) with ESMTP id j6EFx9El034222 for ; Thu, 14 Jul 2005 17:59:10 +0200 (CEST) (envelope-from dan@kulesh.obluda.cz) Received: (from root@localhost) by kulesh.obluda.cz (8.13.3/8.13.1/Submit) id j6EFx9NC034221; Thu, 14 Jul 2005 17:59:09 +0200 (CEST) (envelope-from dan) Message-Id: <200507141559.j6EFx9NC034221@kulesh.obluda.cz> Date: Thu, 14 Jul 2005 17:59:09 +0200 (CEST) From: Dan Lukes To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Subject: bin/83467: [ PATCH ] Unhandled malloc errors within libbsnmp pf_snmp.c X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Dan Lukes List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Jul 2005 16:00:36 -0000 >Number: 83467 >Category: bin >Synopsis: [ PATCH ] Unhandled malloc errors within libbsnmp pf_snmp.c >Confidential: no >Severity: serious >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Jul 14 16:00:34 GMT 2005 >Closed-Date: >Last-Modified: >Originator: Dan Lukes >Release: FreeBSD 5.4-STABLE i386 >Organization: Obludarium >Environment: System: FreeBSD kulesh.obluda.cz 5.4-STABLE FreeBSD 5.4-STABLE #8: Sat Jul 9 16:31:08 CEST 2005 dan@kulesh.obluda.cz:/usr/obj/usr/src/sys/KULESH i386 lib/libbsnmp/modules/snmp_pf/pf_snmp.c,v 1.1.2.1 2005/04/20 08:45:13 philip >Description: See synopsis >How-To-Repeat: >Fix: Please note the realloc(NULL, ...) is correct construct, so we can delete initial malloc within pfi_refresh() and pft_refresh() instead of error handling. Original error handling of ioctl() errors cause memory leakage (p resp. t isn't free()d ) I delete partially build pfi_table/pft_table on error also. --- patch begins here --- --- lib/libbsnmp/modules/snmp_pf/pf_snmp.c.ORIG Wed Apr 20 10:45:13 2005 +++ lib/libbsnmp/modules/snmp_pf/pf_snmp.c Thu Jul 14 17:47:43 2005 @@ -899,7 +899,7 @@ pfi_refresh(void) { struct pfioc_iface io; - struct pfi_if *p; + struct pfi_if *p = NULL; struct pfi_entry *e; int i, numifs = 1; @@ -913,19 +913,23 @@ } bzero(&io, sizeof(io)); - p = malloc(sizeof(struct pfi_if)); io.pfiio_flags = PFI_FLAG_INSTANCE; io.pfiio_esize = sizeof(struct pfi_if); for (;;) { - p = realloc(p, numifs * sizeof(struct pfi_if)); + p = reallocf(p, numifs * sizeof(struct pfi_if)); + if (p == NULL) { + syslog(LOG_ERR, "pfi_refresh(): reallocf() numifs=%d: %s", + numifs, strerror(errno)); + goto err2; + } io.pfiio_size = numifs; io.pfiio_buffer = p; if (ioctl(dev, DIOCIGETIFACES, &io)) { syslog(LOG_ERR, "pfi_refresh(): ioctl(): %s", strerror(errno)); - return (-1); + goto err2; } if (numifs >= io.pfiio_size) @@ -936,6 +940,8 @@ for (i = 0; i < numifs; i++) { e = malloc(sizeof(struct pfi_entry)); + if (e == NULL) + goto err1; e->index = i + 1; memcpy(&e->pfi, p+i, sizeof(struct pfi_if)); TAILQ_INSERT_TAIL(&pfi_table, e, link); @@ -947,6 +953,16 @@ free(p); return (0); + +err1: + while (!TAILQ_EMPTY(&pfi_table)) { + e = TAILQ_FIRST(&pfi_table); + TAILQ_REMOVE(&pfi_table, e, link); + free(e); + } +err2: + free(p); + return(-1); } static int @@ -978,6 +994,12 @@ for (i = 0; i < numqs; i++) { e = malloc(sizeof(struct pfq_entry)); + if (e == NULL) { + syslog(LOG_ERR, "pfq_refresh(): " + "malloc(): %s", + strerror(errno)); + goto err; + } pa.ticket = ticket; pa.nr = i; @@ -985,7 +1007,7 @@ syslog(LOG_ERR, "pfq_refresh(): " "ioctl(DIOCGETALTQ): %s", strerror(errno)); - return (-1); + goto err; } if (pa.altq.qid > 0) { @@ -1000,6 +1022,14 @@ pf_tick = this_tick; return (0); +err: + free(e); + while (!TAILQ_EMPTY(&pfq_table)) { + e = TAILQ_FIRST(&pfq_table); + TAILQ_REMOVE(&pfq_table, e, link); + free(e); + } + return(-1); } static int @@ -1024,7 +1054,7 @@ pft_refresh(void) { struct pfioc_table io; - struct pfr_tstats *t; + struct pfr_tstats *t = NULL; struct pft_entry *e; int i, numtbls = 1; @@ -1038,18 +1068,22 @@ } bzero(&io, sizeof(io)); - t = malloc(sizeof(struct pfr_tstats)); io.pfrio_esize = sizeof(struct pfr_tstats); for (;;) { - t = realloc(t, numtbls * sizeof(struct pfr_tstats)); + t = reallocf(t, numtbls * sizeof(struct pfr_tstats)); + if (t == NULL) { + syslog(LOG_ERR, "pft_refresh(): reallocf() numtbls=%d: %s", + numtbls, strerror(errno)); + goto err2; + } io.pfrio_size = numtbls; io.pfrio_buffer = t; if (ioctl(dev, DIOCRGETTSTATS, &io)) { syslog(LOG_ERR, "pft_refresh(): ioctl(): %s", strerror(errno)); - return (-1); + goto err2; } if (numtbls >= io.pfrio_size) @@ -1060,6 +1094,8 @@ for (i = 0; i < numtbls; i++) { e = malloc(sizeof(struct pfr_tstats)); + if (e == NULL) + goto err1; e->index = i + 1; memcpy(&e->pft, t+i, sizeof(struct pfr_tstats)); TAILQ_INSERT_TAIL(&pft_table, e, link); @@ -1071,6 +1107,15 @@ free(t); return (0); +err1: + while (!TAILQ_EMPTY(&pft_table)) { + e = TAILQ_FIRST(&pft_table); + TAILQ_REMOVE(&pft_table, e, link); + free(e); + } +err2: + free(t); + return(-1); } /* --- patch ends here --- >Release-Note: >Audit-Trail: >Unformatted: