Date: Sun, 23 May 2010 08:27:26 GMT From: Garrett Cooper <gcooper@FreeBSD.org> To: freebsd-gnats-submit@FreeBSD.org Subject: bin/146855: [patch] [sysinstall] address possible QA issues with dispatch.c Message-ID: <201005230827.o4N8RQfV034640@www.freebsd.org> Resent-Message-ID: <201005230830.o4N8U1Qe010951@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201005230827.o4N8RQfV034640>