On Sun, May 14, 2023 at 10:42:34AM +0200, Otto Moerbeek wrote:
> Hi,
>
> On free, chunks (the pieces of a pages used for smaller allocations)
> are junked and then validated after they leave the delayed free list.
> So after free, a chunk always contains junk bytes. This means that if
> we start with the right contents for a new page of chunks, we can
> *validate* instead of *write* junk bytes when (re)-using a chunk.
>
> Wiht this, we can detect write-after-free when a chunk is recycled,
> not justy when a chunk is in the delayed free list. We do a little
> bit more work on initial allocation of a page of chunks and when
> re-using (as I validate now even on junk level 1), so some performance
> validation is needed. In my tests I did not see negative effects, even
> some slight improvemt (likely because validating junk bytes is cheaper
> than writing). But this needs tests to see if that is true in more
> cases than my tests.
>
> Also: some extra consistency checks for recallocaray(3) and fixes in
> error messages to make them more consistent, with man page bits. Plus
> regress additions.
>
> Please test and review!
>
> Thanks,
>
> -Otto
>
Now with diff attached!
Index: lib/libc/stdlib/malloc.3
===================================================================
RCS file: /home/cvs/src/lib/libc/stdlib/malloc.3,v
retrieving revision 1.132
diff -u -p -r1.132 malloc.3
--- lib/libc/stdlib/malloc.3 17 Apr 2023 05:45:06 -0000 1.132
+++ lib/libc/stdlib/malloc.3 14 May 2023 08:27:16 -0000
@@ -314,7 +314,7 @@ Increase the junk level by one if it is
Decrease the junk level by one if it is larger than 0.
Junking writes some junk bytes into the area allocated.
Junk is bytes of 0xdb when allocating;
-freed chunks are filled with 0xdf.
+freed allocations are filled with 0xdf.
By default the junk level is 1: after free,
small chunks are completely junked;
for pages the first part is junked.
@@ -628,22 +628,24 @@ An attempt to
.Fn free
or
reallocate an unallocated pointer was made.
-.It Dq chunk is already free
-There was an attempt to free a chunk that had already been freed.
+.It Dq double free
+There was an attempt to free an allocation that had already been freed.
.It Dq write after free
-A chunk has been modified after it was freed.
+An allocation has been modified after it was freed.
.It Dq modified chunk-pointer
The pointer passed to
.Fn free
or a reallocation function has been modified.
-.It Dq chunk canary corrupted address offset@length
+.It Dq canary corrupted address offset@length
A byte after the requested size has been overwritten,
indicating a heap overflow.
The offset at which corruption was detected is printed before the @,
and the requested length of the allocation after the @.
-.It Dq recorded old size oldsize != size
+.It Dq recorded size oldsize inconsistent with size
.Fn recallocarray
-has detected that the given old size does not equal the recorded size in its
+or
+.Fn freezero
+has detected that the given old size does not match the recorded size in its
meta data.
Enabling option
.Cm C
Index: lib/libc/stdlib/malloc.c
===================================================================
RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.283
diff -u -p -r1.283 malloc.c
--- lib/libc/stdlib/malloc.c 10 May 2023 07:58:06 -0000 1.283
+++ lib/libc/stdlib/malloc.c 14 May 2023 08:27:16 -0000
@@ -977,6 +977,10 @@ omalloc_make_chunks(struct dir_info *d,
NULL))
goto err;
LIST_INSERT_HEAD(&d->chunk_dir[bucket][listnum], bp, entries);
+
+ if (bucket > 0 && d->malloc_junk != 0)
+ memset(pp, SOME_FREEJUNK, MALLOC_PAGESIZE);
+
return bp;
err:
@@ -1113,9 +1117,8 @@ found:
p = (char *)bp->page + k;
if (bp->bucket > 0) {
- if (d->malloc_junk == 2)
- memset(p, SOME_JUNK, B2SIZE(bp->bucket));
- else if (mopts.chunk_canaries)
+ validate_junk(d, p, B2SIZE(bp->bucket));
+ if (mopts.chunk_canaries)
fill_canary(p, size, B2SIZE(bp->bucket));
}
return p;
@@ -1134,7 +1137,7 @@ validate_canary(struct dir_info *d, u_ch
while (p < q) {
if (*p != (u_char)mopts.chunk_canaries && *p != SOME_JUNK) {
- wrterror(d, "chunk canary corrupted %p %#tx@%#zx%s",
+ wrterror(d, "canary corrupted %p %#tx@%#zx%s",
ptr, p - ptr, sz,
*p == SOME_FREEJUNK ? " (double free?)" : "");
}
@@ -1157,7 +1160,7 @@ find_chunknum(struct dir_info *d, struct
wrterror(d, "modified chunk-pointer %p", ptr);
if (info->bits[chunknum / MALLOC_BITS] &
(1U << (chunknum % MALLOC_BITS)))
- wrterror(d, "chunk is already free %p", ptr);
+ wrterror(d, "double free %p", ptr);
if (check && info->bucket > 0) {
validate_canary(d, ptr, info->bits[info->offset + chunknum],
B2SIZE(info->bucket));
@@ -1924,13 +1927,22 @@ orecallocarray(struct dir_info **argpool
uint32_t chunknum = find_chunknum(pool, info, p, 0);
if (info->bits[info->offset + chunknum] != oldsize)
- wrterror(pool, "recorded old size %hu != %zu",
+ wrterror(pool, "recorded size %hu != %zu",
info->bits[info->offset + chunknum],
oldsize);
+ } else {
+ if (sz < oldsize)
+ wrterror(pool, "chunk size %zu < %zu",
+ sz, oldsize);
}
- } else if (oldsize < (sz - mopts.malloc_guard) / 2)
- wrterror(pool, "recorded old size %zu != %zu",
- sz - mopts.malloc_guard, oldsize);
+ } else {
+ if (sz - mopts.malloc_guard < oldsize)
+ wrterror(pool, "recorded size %zu < %zu",
+ sz - mopts.malloc_guard, oldsize);
+ if (oldsize < (sz - mopts.malloc_guard) / 2)
+ wrterror(pool, "recorded size %zu inconsistent with
%zu",
+ sz - mopts.malloc_guard, oldsize);
+ }
newptr = omalloc(pool, newsize, 0, f);
if (newptr == NULL)
Index: regress/lib/libc/malloc/malloc_errs/malloc_errs.c
===================================================================
RCS file: /home/cvs/src/regress/lib/libc/malloc/malloc_errs/malloc_errs.c,v
retrieving revision 1.2
diff -u -p -r1.2 malloc_errs.c
--- regress/lib/libc/malloc/malloc_errs/malloc_errs.c 9 May 2023 19:07:37
-0000 1.2
+++ regress/lib/libc/malloc/malloc_errs/malloc_errs.c 14 May 2023 08:27:16
-0000
@@ -138,9 +138,7 @@ t8(void)
void
t9(void)
{
- char *p;
-
- p = malloc(100);
+ char *p = malloc(100);
p[100] = 0;
free(p);
}
@@ -191,7 +189,6 @@ t15(void)
void
t16(void)
{
- abort(); /* not yet */
char *p = recallocarray(NULL, 0, 16, 1);
char *q = recallocarray(p, 2, 3, 16);
}
@@ -208,11 +205,27 @@ t17(void)
void
t18(void)
{
- abort(); /* not yet */
char *p = recallocarray(NULL, 0, 1, getpagesize());
char *q = recallocarray(p, 2, 3, getpagesize());
}
+/* recallocarray with wrong size, pages */
+void
+t19(void)
+{
+ char *p = recallocarray(NULL, 0, 1, 10 * getpagesize());
+ char *q = recallocarray(p, 1, 2, 4 * getpagesize());
+}
+
+/* canary check pages */
+void
+t20(void)
+{
+ char *p = malloc(2*getpagesize() - 100);
+ p[2*getpagesize() - 100] = 0;
+ free(p);
+}
+
struct test {
void (*test)(void);
const char *flags;
@@ -238,6 +251,8 @@ struct test tests[] = {
{ t16, "" },
{ t17, "C" },
{ t18, "" },
+ { t19, "" },
+ { t20, "C" },
};
int main(int argc, char *argv[])
