On Sun, Oct 16, 2016 at 01:14:35PM +0200, Otto Moerbeek wrote:
> Hi,
>
> this diff is somewhat big since I decided to rewrite wrterror() to be
> able to get better error messages.
>
> Please review and test this with malloc option C (and other flags). An
> example run:
>
> a.out(85360) in free(): chunk canary corrupted 0x13c68f696000 0x18a92@0x18a88
>
> This means I overwrote a byte at offset 0x18a92 in a chunk of size
> 0x18a88 that is located at 0x13c68f696000.
>
> Only max 32 bytes after the requested size are filled with 0xdb and
> checked on free.
This is very nice and reads very well. I tested it with C and CJ and S
on amd64 and it seems to work fine.
A few small comments about the new wrterror():
In
+ snprintf(pidbuf, sizeof(pidbuf), "%s(%d) in %s(): ", __progname,
+ getpid(), d->func ? d->func : "unknown");
I think you should truncate an insanely long __progname to 50 characters
to avoid overwriting the more important info that follows:
snprintf(pidbuf, sizeof(pidbuf), "%.50s(%d) in %s(): ", __progname,
getpid(), d->func ? d->func : "unknown");
because the longest possible string after __progname is
"(12345) in posix_memalign(): "
which has 29 characters, so there is room for 50 non-NULs in pidbuf[80],
and these should be plenty enough to identify the offending program.
I'm slightly concerned about running strlen on pidbuf[] and buf[] in
case {,v}snprintf fail. Wouldn't it be cleaner to do something like
this:
if ((ret = snprintf(pidbuf, ...)) >= 0) {
iov[0].iov_base = pidbuf;
iov[0].iov_len = ret;
} else {
/* hardcoded message */
}
and a small style nit:
> @@ -1181,6 +1177,13 @@ omalloc(struct dir_info *pool, size_t sz
> memset(p, SOME_JUNK,
> psz - mopts.malloc_guard);
> }
> + else if (mopts.chunk_canaries) {
} else if (mopts.chunk_canaries) {
> + size_t csz = psz - mopts.malloc_guard - sz;
> +
> + if (csz > CHUNK_CHECK_LENGTH)
> + csz = CHUNK_CHECK_LENGTH;
> + memset(p + sz, SOME_JUNK, csz);
> + }
> }
>
> } else {
