From owner-freebsd-bugs@FreeBSD.ORG Sun May 23 08:30:02 2010 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DE1EB1065673 for ; Sun, 23 May 2010 08:30:01 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (unknown [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id A142C8FC19 for ; Sun, 23 May 2010 08:30:01 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o4N8U1dM010954 for ; Sun, 23 May 2010 08:30:01 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o4N8U1Qe010951; Sun, 23 May 2010 08:30:01 GMT (envelope-from gnats) Resent-Date: Sun, 23 May 2010 08:30:01 GMT Resent-Message-Id: <201005230830.o4N8U1Qe010951@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, Garrett Cooper Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 839EC1065759 for ; Sun, 23 May 2010 08:27:26 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id 597558FC08 for ; Sun, 23 May 2010 08:27:26 +0000 (UTC) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.3/8.14.3) with ESMTP id o4N8RQp8034641 for ; Sun, 23 May 2010 08:27:26 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.3/8.14.3/Submit) id o4N8RQfV034640; Sun, 23 May 2010 08:27:26 GMT (envelope-from nobody) Message-Id: <201005230827.o4N8RQfV034640@www.freebsd.org> Date: Sun, 23 May 2010 08:27:26 GMT From: Garrett Cooper To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: bin/146855: [patch] [sysinstall] address possible QA issues with dispatch.c 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: Sun, 23 May 2010 08:30:02 -0000 >Number: 146855 >Category: bin >Synopsis: [patch] [sysinstall] address possible QA issues with dispatch.c >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sun May 23 08:30:01 UTC 2010 >Closed-Date: >Last-Modified: >Originator: Garrett Cooper >Release: 9-CURRENT >Organization: Cisco Systems, Inc. >Environment: FreeBSD bayonetta.local 9.0-CURRENT FreeBSD 9.0-CURRENT #0 r206173M: Mon Apr 26 22:45:06 PDT 2010 root@bayonetta.local:/usr/obj/usr/src/sys/BAYONETTA.ata amd64 >Description: 1. dispatch_add_command: a. Modify the logic so there's only one exit point instead of two. b. Only insert valid (non-NULL) values into the queue. 2. dispatch_free_command: a. Doesn't ensure that item is NULL before it attempts to remove the item from the queue and dereference the pointer to item. b. Previously allocated memory isn't NULLed out, so if one of the calls misuses the memory it will result in a memory access violation. >How-To-Repeat: All of these conditions will occur under low memory situations, and thus shouldn't happen 99.9% of the time, but will occur given proper circumstances. >Fix: See attached patch. Patch attached with submission follows: Index: dispatch.c =================================================================== --- dispatch.c (revision 206173) +++ dispatch.c (working copy) @@ -136,9 +136,13 @@ static void dispatch_free_command(command_buffer *item) { - REMQUE(item); - free(item->string); - free(item); + if (item != NULL) { + REMQUE(item); + free(item->string); + item->string = NULL; + } + free(item); + item = NULL; } static void @@ -155,17 +159,28 @@ static command_buffer * dispatch_add_command(qelement *head, char *string) { - command_buffer *new; + command_buffer *new = NULL; - new = malloc(sizeof(command_buffer)); + new = malloc(sizeof(command_buffer)); - if (!new) - return NULL; + if (new != NULL) { - new->string = strdup(string); - INSQUEUE(new, head->q_back); + new->string = strdup(string); - return new; + /* + * We failed to copy `string'; clean up the allocated + * resources. + */ + if (new->string == NULL) { + free(new); + new = NULL; + } else { + INSQUEUE(new, head->q_back); + } + + } + + return new; } /* >Release-Note: >Audit-Trail: >Unformatted: