Date: Sat, 19 Dec 2015 07:21:23 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: John Baldwin <jhb@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r292443 - head/usr.sbin/boot0cfg Message-ID: <20151219063208.M2407@besplex.bde.org> In-Reply-To: <201512181752.tBIHq9QN069909@repo.freebsd.org> References: <201512181752.tBIHq9QN069909@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 18 Dec 2015, John Baldwin wrote:
> Log:
> Fix the precious change to check the pointer returned by malloc().
>
> Submitted by: luke <luke.tw@gmail.com>
> Pointy hat to: jhb
Of course, malloc can't fail, especially for small utilities like boot0cfg,
especially in -current.
> Modified: head/usr.sbin/boot0cfg/boot0cfg.c
> ==============================================================================
> --- head/usr.sbin/boot0cfg/boot0cfg.c Fri Dec 18 17:39:54 2015 (r292442)
> +++ head/usr.sbin/boot0cfg/boot0cfg.c Fri Dec 18 17:52:08 2015 (r292443)
> @@ -337,7 +337,7 @@ read_mbr(const char *disk, u_int8_t **mb
> return (mbr_size);
> }
> *mbr = malloc(sizeof(buf));
> - if (mbr == NULL)
> + if (*mbr == NULL)
> errx(1, "%s: unable to allocate MBR buffer", disk);
Style bug. Someone suggested using the 'if ((*mbr = malloc(...)) ==
NULL)' style. That might be less clear, but it is what is used elswhere
in the program, including for the old malloc check. Also, the error
messages are too different. The other one calls the buffer a "read"
buffer although it spells its variable name mbr.
> memcpy(*mbr, buf, sizeof(buf));
> close(fd);
Program for checking that malloc can't fail:
X #define SIZE 16
X
X #include <err.h>
X #include <stdlib.h>
X #include <string.h>
X
X int
X main(void)
X {
X char *p;
X size_t tot;
X
X tot = 0;
X while ((p = malloc(SIZE)) != NULL) {
X bzero(p, SIZE);
X tot += SIZE;
X if (tot > 4 * 1024 * 1024)
X errx(1, "giving up after allocating %zu bytes", tot);
X }
X errx(1, "malloc failed after allocating %zu bytes", tot);
X }
It is hard to get this to fail in -current. ulimit -Sm 4096 no longer
works for limiting data size (ulimit -m never worked). Virtual memory must
be limited in -current. But ulimit -Sv 4096 prevents most programs from
starting, due to bugs in dynamic linkage:
pts/9:bde@freefall:~/zz> ulimit -Sv 4096
pts/9:bde@freefall:~/zz> ls
/lib/libutil.so.9: mmap of entire address space failed: Cannot allocate memory
pts/9:bde@freefall:~/zz> ./z
/lib/libc.so.7: mmap of entire address space failed: Cannot allocate memory
"./z" is this program. Linking the program statically makes it run
in the measly 4096k of virtual and mallocable memory. It then allocates
4194320 bytes using the small allocation size. Of course, cc can't
run in 4096k. However, it is not necessary to map the _entire_ address
space like the error messages say. Dynamic "./z" runs in a measly
10000k. ls fails as above up to at least 12000k, but dumps core with
14000k. ls starts working with a measly 16000k. cc needs more still.
I didn't find the exact requirement, and used 160000k to compile this
little program. Not so measly.
The program is careful to touch all the memory that it allocates. Without
that, I think things are even more broken. Somehow the process virtual
size doesn't grown above 6.2M. Apparently even the virtual memory is
not completely allocated until it is used. So malloc() returns success,
but failures may happen much later when the memory is used. Similarly
with SIZE = 16MB but only the first byte of each allocation touched.
OTOH, with size = 16MB all touched and no limits, the process grows
very large and takes too long to respond to job control symbols, as
if it is swapping. The -d and -m limits don't work for preventing
growth of its real memory.
ulimit -Sv should be used in testing since ulimit -v sets the hard
limit and prevents restoring a usable soft limit. Setting -v to a low
value (lower than 16000k for ls) is not useful except for testing
malloc failure since it gives an unusable shell.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20151219063208.M2407>