From owner-freebsd-bugs@FreeBSD.ORG Thu Jun 28 21:50:02 2007 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 [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id C374516A46C for ; Thu, 28 Jun 2007 21:50:02 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [69.147.83.40]) by mx1.freebsd.org (Postfix) with ESMTP id 52AF013C487 for ; Thu, 28 Jun 2007 21:50:02 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id l5SLo2Me028116 for ; Thu, 28 Jun 2007 21:50:02 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id l5SLo2LI028115; Thu, 28 Jun 2007 21:50:02 GMT (envelope-from gnats) Resent-Date: Thu, 28 Jun 2007 21:50:02 GMT Resent-Message-Id: <200706282150.l5SLo2LI028115@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, David Sanderson Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id BE0D616A400 for ; Thu, 28 Jun 2007 21:43:53 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [69.147.83.33]) by mx1.freebsd.org (Postfix) with ESMTP id 9698513C480 for ; Thu, 28 Jun 2007 21:43:53 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.13.1/8.13.1) with ESMTP id l5SLhrvv065122 for ; Thu, 28 Jun 2007 21:43:53 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.13.1/8.13.1/Submit) id l5SLhrY9065121; Thu, 28 Jun 2007 21:43:53 GMT (envelope-from nobody) Message-Id: <200706282143.l5SLhrY9065121@www.freebsd.org> Date: Thu, 28 Jun 2007 21:43:53 GMT From: David Sanderson To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.0 Cc: Subject: misc/114110: Disk_Names() returns a vector that is unsafe to free 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: Thu, 28 Jun 2007 21:50:02 -0000 >Number: 114110 >Category: misc >Synopsis: Disk_Names() returns a vector that is unsafe to free >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: Thu Jun 28 21:50:01 GMT 2007 >Closed-Date: >Last-Modified: >Originator: David Sanderson >Release: FreeBSD 6.2 >Organization: Panasas, Inc. >Environment: FreeBSD perf-x4 6.2-RELEASE FreeBSD 6.2-RELEASE #0: Fri Jan 12 08:43:30 UTC 2007 root@portnoy.cse.buffalo.edu:/usr/obj/usr/src/sys/SMP amd64 >Description: The documentation for the Disk_Names() function in libdisk(3) states: Disk_Names() returns `char**' with all disk's names (wd0, wd1 ...). You must free each pointer, as well as the array by hand. However, looking at the code, in FreeBSD 6 this routine returns a vector that can be freed, but only one of the pointers in that vector can be freed, because they all point to space in a single malloced buffer, char *disklist. Since the pointers in the vector are sorted by the strings they point to, there is no way for the caller to know which is safe to free, and it is certainly wrong to free them all. It would be best for Disk_Names() to return a vector according to its docs. >How-To-Repeat: >Fix: I'll contribute a sample new version that returns a vector that is safe to free according to the docs. Patch attached with submission follows: char ** Disk_Names() { static char **disks; int error; size_t listsize; char *disklist; char *disklistp; size_t ndisks; size_t i; error = sysctlbyname("kern.disks", NULL, &listsize, NULL, 0); if (error) { warn("kern.disks sysctl not available"); return NULL; } if (listsize == 0) return (NULL); disklist = (char *)malloc(listsize + 1); if (disklist == NULL) { return NULL; } memset(disklist, 0, listsize + 1); error = sysctlbyname("kern.disks", disklist, &listsize, NULL, 0); if (error || disklist[0] == 0) { free(disklist); return NULL; } for (disklistp = disklist, ndisks = 0; disklistp;) { char *disk = strsep(&disklistp, " "); if (disk) { /* We restore the space for a second pass through the names. */ if (disklistp) { disklistp[-1] = ' '; } ndisks++; } } disks = malloc(sizeof *disks * (1 + ndisks)); if (disks == NULL) { free(disklist); return NULL; } memset(disks,0,sizeof *disks * (1 + ndisks)); for (disklistp = disklist, i = 0; i < ndisks; i++) { disks[i] = strdup(strsep(&disklistp, " ")); if (disks[i] == NULL) { size_t j; for (j = 0; j < i; j++) { free(disks[j]); } free(disklist); free(disks); return NULL; } } free(disklist); qsort(disks, ndisks, sizeof(char*), qstrcmp); return disks; } >Release-Note: >Audit-Trail: >Unformatted: