Date: Thu, 19 Feb 2004 09:24:09 +0100 From: "Poul-Henning Kamp" <phk@phk.freebsd.dk> To: kientzle@acm.org Cc: current@freebsd.org Subject: Re: standard error handling for malloc() broken for user root and group wheel Message-ID: <24950.1077179049@critter.freebsd.dk> In-Reply-To: Your message of "Thu, 19 Feb 2004 00:13:00 PST." <4034700C.9090107@kientzle.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <4034700C.9090107@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@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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?24950.1077179049>