standard error handling for malloc() broken for user root and group wheel

[Poul-Henning Kamp]

In message <4034700C.9090107 at kientzle.com>, Tim Kientzle writes:
>On Wed, 18 Feb 2004, Poul-Henning Kamp wrote:
>>
>>The situations which can result in the 'a' vs 'A' flag making a
>>difference in malloc(3) behavior are all violations of the malloc(3)
>>API as defined by ISO C and as such the standard defines the behaviour
>>as "undefined".
>
>ANSI/ISO 9899-1990, Section 7.10.3.3 clearly states:
>
>    "The malloc function returns either a null pointer
>     or a pointer to the allocated space."
>
>There are no "undefined" possibilities here.  Aborting the program
>on a failure to allocate memory is pretty clearly a violation
>of the standard, which requires the malloc function to
>always return.

There is neither requirements nor guarantees how any function in
the ansi/iso regime reacts if you grossly violate the API or stomp
on random memory.

Think if the 'A' option as userland SIGSEGV.  (peter@ actually suggested
that I make it kill(getpid(), SIGSEGV) before calling abort in order
to let a process install a handler for it.

I'm currently testing this patch:

Index: malloc.c
===================================================================
RCS file: /home/ncvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.84
diff -u -r1.84 malloc.c
--- malloc.c	28 Nov 2003 18:03:22 -0000	1.84
+++ malloc.c	19 Feb 2004 06:38:27 -0000
@@ -112,6 +112,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <signal.h>
 #include <unistd.h>
 #include "un-namespace.h"
 
@@ -308,6 +309,7 @@
 
     suicide = 1;
     _malloc_message(_getprogname(), malloc_func, " error: ", p);
+    kill(getpid(), SIGSEGV);
     abort();
 }
 
@@ -315,7 +317,11 @@
 wrtwarning(char *p)
 {
 
-    if (malloc_abort)
+    /*
+     * Sensitive processes, somewhat arbitrarily defined here as setuid,
+     * setgid, root and wheel cannot afford to have malloc mistakes.
+     */
+    if (malloc_abort || issetugid() || getuid() == 0 || getgid() == 0)
 	wrterror(p);
     _malloc_message(_getprogname(), malloc_func, " warning: ", p);
 }
@@ -458,21 +464,13 @@
 		case 'z': malloc_zero    = 0; break;
 		case 'Z': malloc_zero    = 1; break;
 		default:
-		    j = malloc_abort;
-		    malloc_abort = 0;
-		    wrtwarning("unknown char in MALLOC_OPTIONS\n");
-		    malloc_abort = j;
+		    _malloc_message(_getprogname(), malloc_func,
+			 " warning: ", "unknown char in MALLOC_OPTIONS\n");
 		    break;
 	    }
 	}
     }
 
-    /*
-     * Sensitive processes, somewhat arbitrarily defined here as setuid,
-     * setgid, root and wheel cannot afford to have malloc mistakes.
-     */
-    if (issetugid() || getuid() == 0 || getgid() == 0)
-	    malloc_abort = 1;
 
     UTRACE(0, 0, 0);
 
@@ -750,9 +748,6 @@
 	result = malloc_bytes(size);
     else
 	result = malloc_pages(size);
-
-    if (malloc_abort && result == NULL)
-	wrterror("allocation failed\n");
 
     if (malloc_zero && result != NULL)
 	memset(result, 0, size);

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.