Re: enhanced use-after-free detection for malloc v2
On 02/11/15 06:40 AM, Theo Buehler wrote:
> Sorry for this rather long mail:
>
> I have three small comments on the patch itself
> (starting 80 lines below).
>
> For those who want to try both new features, I attached a patch against
> -current that merges the three parts of Daniel's diff (plus the trivial
> two of the nits below) at the very end of this mail.
>
> In addition to Daniel's test program, here's a small program that
> in 1000 test runs on amd64 always triggered an abort at the second
> free().
>
>
> $ cat heap.c
> #include
> #include
> #include
> #include
>
> int
> main(void)
> {
> char *p;
>
> p = malloc(1);
> if (p == NULL)
> err(1, "malloc");
>
> p = strcpy(p, "1234567");
> printf("%s\n", p);
> free(p);
>
> p = malloc(1);
> p = strcpy(p, "12345678");
> printf("%s\n", p);
> free(p);
>
> return 0;
> }
> $ cc -o heap heap.c
> /tmp//ccHH1E1h.o: In function `main': heap.c:(.text+0x6b): warning: warning:
> strcpy() is almost always
> misused, please use strlcpy()
> $ ./heap
> 1234567
> 12345678
> heap(5992) in free(): error: chunk canary corrupted 0x1f4619f0e9c0
> Abort trap (core dumped)
> $
>
>
> The performance impact of MALLOC_OPTIONS='CSV' is very noticeable,
> but 'CV' is negligible and 'CJV' makes the system feel a bit less
> snappy on my machine, but the actual effect seems almost negligible.
>
> Here's a rough 'benchmark':
>
> Building the base system on my i7 Thinkpad T420 with 4G RAM,
> with the various malloc options each with a freshly booted system.
>
> defaults: 31m04
> CV: 33m16
> CJV: 33m46
> CSV: 53m32
>
> Unfortunately, I have yet to hit an actual bug with this, one that isn't
> triggered by simple test programs. Casual browsing with firefox, even
> watching movies with mplayer is possible with MALLOC_OPTIONS='CSV'.
It would help quite a bit to bump up the size of the quarantine. The
default 16 large one doesn't keep around the allocations for very long.
A more expensive version of the feature could also check for either zero
or junk data before allocating anything. Haven't decided how to expose
that via the option though.
Most bugs triggering in fairly normal situations with Firefox, etc. have
probably already been found by Valgrind though. The strength of features
like this in malloc is that they can enabled all the time with little
performance loss, so eventually they'll find new bugs (which has been
the case for us on Android, but perhaps that's mostly because many apps
were never tested at all with ASAN/Valgrind).
It may also be viable as a security feature in some situations, not just
a way of finding bugs. Depends on how far away the UAF is from the free
call since one other free is all that's needed to lose reliable
detection. It might work better with a FIFO ring buffer rather than the
current fully randomized array (perhaps a mix? dunno).
> On Sun, Nov 01, 2015 at 02:56:11PM -0500, Daniel Micay wrote:
>> @@ -560,6 +563,12 @@ omalloc_init(struct dir_info **dp)
>> case 'J':
>> mopts.malloc_junk = 2;
>> break;
>> +case 'v':
>> +mopts.malloc_validate = 0;
>> +break;
>> +case 'V':
>> +mopts.malloc_validate = 1;
>> +break;
>> case 'n':
>> case 'N':
>> break;
>
> I'd keep the alphabetical order in the switch.
>
>> @@ -608,6 +617,9 @@ omalloc_init(struct dir_info **dp)
>> }
>> }
>>
>> +if (!mopts.malloc_junk)
>> +mopts.malloc_validate = 0;
>> +
>
> Wouldn't it make sense to just let 'V' imply 'J' (and 'j' imply 'v') as
> is already done with 'F' and 'U', respectively?
The V option doesn't benefit from J since it's only able to check for
use-after-frees in small allocations. I don't think there's a good
reason to tie them together.
A side note is that it would be nice if junk freeing was split up into
the security feature (full junk fill on free) and the debugging feature
(full junk fill on init). I ended up doing that in CopperheadOS because
it makes J significantly cheaper without giving up the nice security
properties gained by sanitizing everything on free. Might be too late to
change the meaning in OpenBSD though.
>> @@ -1190,6 +1208,35 @@ malloc(size_t size)
>> /*DEF_STRONG(malloc);*/
>>
>> static void
>> +validate_junk(void *p) {
>> +struct region_info *r;
>> +struct dir_info *pool = getpool();
>> +size_t byte, sz;
>> +if (p == NULL)
>> +return;
>> +r = find(pool, p);
>> +if (r == NULL)
>> +wrterror("bogus pointer in validate_junk", p);
>> +REALSIZE(sz, r);
>> +for (byte = 0; byte < sz; byte++) {
>> +if (((char *)p)[byte] !=
Re: enhanced use-after-free detection for malloc v2
Sorry for this rather long mail:
I have three small comments on the patch itself
(starting 80 lines below).
For those who want to try both new features, I attached a patch against
-current that merges the three parts of Daniel's diff (plus the trivial
two of the nits below) at the very end of this mail.
In addition to Daniel's test program, here's a small program that
in 1000 test runs on amd64 always triggered an abort at the second
free().
$ cat heap.c
#include
#include
#include
#include
int
main(void)
{
char *p;
p = malloc(1);
if (p == NULL)
err(1, "malloc");
p = strcpy(p, "1234567");
printf("%s\n", p);
free(p);
p = malloc(1);
p = strcpy(p, "12345678");
printf("%s\n", p);
free(p);
return 0;
}
$ cc -o heap heap.c
/tmp//ccHH1E1h.o: In function `main': heap.c:(.text+0x6b): warning: warning:
strcpy() is almost always
misused, please use strlcpy()
$ ./heap
1234567
12345678
heap(5992) in free(): error: chunk canary corrupted 0x1f4619f0e9c0
Abort trap (core dumped)
$
The performance impact of MALLOC_OPTIONS='CSV' is very noticeable,
but 'CV' is negligible and 'CJV' makes the system feel a bit less
snappy on my machine, but the actual effect seems almost negligible.
Here's a rough 'benchmark':
Building the base system on my i7 Thinkpad T420 with 4G RAM,
with the various malloc options each with a freshly booted system.
defaults: 31m04
CV: 33m16
CJV:33m46
CSV:53m32
Unfortunately, I have yet to hit an actual bug with this, one that isn't
triggered by simple test programs. Casual browsing with firefox, even
watching movies with mplayer is possible with MALLOC_OPTIONS='CSV'.
On Sun, Nov 01, 2015 at 02:56:11PM -0500, Daniel Micay wrote:
> @@ -560,6 +563,12 @@ omalloc_init(struct dir_info **dp)
> case 'J':
> mopts.malloc_junk = 2;
> break;
> + case 'v':
> + mopts.malloc_validate = 0;
> + break;
> + case 'V':
> + mopts.malloc_validate = 1;
> + break;
> case 'n':
> case 'N':
> break;
I'd keep the alphabetical order in the switch.
> @@ -608,6 +617,9 @@ omalloc_init(struct dir_info **dp)
> }
> }
>
> + if (!mopts.malloc_junk)
> + mopts.malloc_validate = 0;
> +
Wouldn't it make sense to just let 'V' imply 'J' (and 'j' imply 'v') as
is already done with 'F' and 'U', respectively?
> @@ -1190,6 +1208,35 @@ malloc(size_t size)
> /*DEF_STRONG(malloc);*/
>
> static void
> +validate_junk(void *p) {
> + struct region_info *r;
> + struct dir_info *pool = getpool();
> + size_t byte, sz;
> + if (p == NULL)
> + return;
> + r = find(pool, p);
> + if (r == NULL)
> + wrterror("bogus pointer in validate_junk", p);
> + REALSIZE(sz, r);
> + for (byte = 0; byte < sz; byte++) {
> + if (((char *)p)[byte] != SOME_FREEJUNK) {
This cast should be an (unsigned char *):
we have SOME_FREEJUNK == 0xdf, so '((char *)p)[byte] != SOME_FREEJUNK'
will always be true (this is pointed out when compiling with both clang
and gcc on amd64).
Index: stdlib/malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.176
diff -u -p -r1.176 malloc.c
--- stdlib/malloc.c 13 Sep 2015 20:29:23 - 1.176
+++ stdlib/malloc.c 2 Nov 2015 09:52:32 -
@@ -182,15 +182,18 @@ struct malloc_readonly {
int malloc_freeunmap; /* mprotect free pages PROT_NONE? */
int malloc_hint;/* call madvice on free pages? */
int malloc_junk;/* junk fill? */
+ int malloc_validate;/* validate junk */
int malloc_move;/* move allocations to end of page? */
int malloc_realloc; /* always realloc? */
int malloc_xmalloc; /* xmalloc behaviour? */
+ size_t malloc_canaries;/* use canaries after chunks? */
size_t malloc_guard; /* use guard pages after allocations? */
u_int malloc_cache; /* free pages we cache */
#ifdef MALLOC_STATS
int malloc_stats; /* dump statistics at end */
#endif
u_int32_t malloc_canary;/* Matched against ones in malloc_pool
*/
+ uintptr_t malloc_chunk_canary;
};
/* This object is mapped PROT_READ after initialisation to prevent tampering */
@@ -218,6 +221,8 @@ static void malloc_exit(void);
#define CALLER NULL
#endif
+static void validate_delayed_chunks(void);
+
/* low bits of r->p determine size: 0 means >= page size and p->size holding
*
Re: enhanced use-after-free detection for malloc v2
On Fri, Oct 30, 2015 at 11:51:17PM -0400, Daniel Micay wrote:
> On 26/10/15 04:19 PM, Daniel Micay wrote:
> > This is an improved revision of my earlier patch.
> >
> > It now validates the junk data in the delayed_chunks array in an atexit
> > handler
> > too, rather than just when allocations are swapped out.
> >
> > It will now catch this simple UAF 100% of the time:
> >
> > #include
> > #include
> >
> > int main(void) {
> > size_t i;
> > char *p;
> > for (i = 0; i < 32; i++) {
> > p = malloc(16);
> > if (!p) return 1;
> > }
> >
> > p = malloc(16);
> > if (!p) return 1;
> > free(p);
> > *p = 5;
> >
> > for (i = 0; i < 4; i++) {
> > p = malloc(16);
> > if (!p) return 1;
> > free(p);
> > }
> > return 0;
> > }
> >
> > In general, it depends on the allocation still being in the delayed chunks
> > array when the use-after-free happens. This means a larger delayed chunks
> > array would improve the detection rate.
>
> That revision was missing a NULL check as it got lost when refactoring
> and the test cases weren't short enough to trigger it. Properly working
> implementation:
Hi Daniel,
I did not have time to look into this up until now and I'm pretty sure
I will not have much time in the near future. But after a casual look
I consider this a very nice addition, so please do continue to persue
this.
To all others: please test and give Daniel feedback,
-Otto
>
> diff --git a/stdlib/malloc.c b/stdlib/malloc.c
> index 424dd77..c408594 100644
> --- a/stdlib/malloc.c
> +++ b/stdlib/malloc.c
> @@ -182,6 +182,7 @@ struct malloc_readonly {
> int malloc_freeunmap; /* mprotect free pages PROT_NONE? */
> int malloc_hint;/* call madvice on free pages? */
> int malloc_junk;/* junk fill? */
> + int malloc_validate;/* validate junk */
> int malloc_move;/* move allocations to end of page? */
> int malloc_realloc; /* always realloc? */
> int malloc_xmalloc; /* xmalloc behaviour? */
> @@ -218,6 +219,8 @@ static void malloc_exit(void);
> #define CALLER NULL
> #endif
>
> +static void validate_delayed_chunks(void);
> +
> /* low bits of r->p determine size: 0 means >= page size and p->size
> holding
> * real size, otherwise r->size is a shift count, or 1 for malloc(0)
> */
> @@ -560,6 +563,12 @@ omalloc_init(struct dir_info **dp)
> case 'J':
> mopts.malloc_junk = 2;
> break;
> + case 'v':
> + mopts.malloc_validate = 0;
> + break;
> + case 'V':
> + mopts.malloc_validate = 1;
> + break;
> case 'n':
> case 'N':
> break;
> @@ -608,6 +617,9 @@ omalloc_init(struct dir_info **dp)
> }
> }
>
> + if (!mopts.malloc_junk)
> + mopts.malloc_validate = 0;
> +
> #ifdef MALLOC_STATS
> if (mopts.malloc_stats && (atexit(malloc_exit) == -1)) {
> static const char q[] = "malloc() warning: atexit(2) failed."
> @@ -616,6 +628,12 @@ omalloc_init(struct dir_info **dp)
> }
> #endif /* MALLOC_STATS */
>
> + if (mopts.malloc_validate && (atexit(validate_delayed_chunks) == -1)) {
> + static const char q[] = "malloc() warning: atexit(2) failed."
> + " Will not be able to check for use after free\n";
> + write(STDERR_FILENO, q, sizeof(q) - 1);
> + }
> +
> while ((mopts.malloc_canary = arc4random()) == 0)
> ;
>
> @@ -1190,6 +1208,35 @@ malloc(size_t size)
> /*DEF_STRONG(malloc);*/
>
> static void
> +validate_junk(void *p) {
> + struct region_info *r;
> + struct dir_info *pool = getpool();
> + size_t byte, sz;
> + if (p == NULL)
> + return;
> + r = find(pool, p);
> + if (r == NULL)
> + wrterror("bogus pointer in validate_junk", p);
> + REALSIZE(sz, r);
> + for (byte = 0; byte < sz; byte++) {
> + if (((char *)p)[byte] != SOME_FREEJUNK) {
> + wrterror("use after free", p);
> + return;
> + }
> + }
> +}
> +
> +static void
> +validate_delayed_chunks(void) {
> + struct dir_info *pool = getpool();
> + int i;
> + if (pool == NULL)
> + return;
> + for (i = 0; i < MALLOC_DELAYED_CHUNK_MASK + 1; i++)
> + validate_junk(pool->delayed_chunks[i]);
> +}
> +
> +static void
> ofree(void *p)
> {
> struct dir_info *pool = getpool();
> @@ -1253,6 +1300,8 @@ ofree(void *p)
> wrterror("double free", p);
> return;
> }
> + if (mopts.malloc_validate)
> +
Re: enhanced use-after-free detection for malloc v2
(without mangling it this time...)
diff --git a/stdlib/malloc.c b/stdlib/malloc.c
index 424dd77..c408594 100644
--- a/stdlib/malloc.c
+++ b/stdlib/malloc.c
@@ -182,6 +182,7 @@ struct malloc_readonly {
int malloc_freeunmap; /* mprotect free pages PROT_NONE? */
int malloc_hint;/* call madvice on free pages? */
int malloc_junk;/* junk fill? */
+ int malloc_validate;/* validate junk */
int malloc_move;/* move allocations to end of page? */
int malloc_realloc; /* always realloc? */
int malloc_xmalloc; /* xmalloc behaviour? */
@@ -218,6 +219,8 @@ static void malloc_exit(void);
#define CALLER NULL
#endif
+static void validate_delayed_chunks(void);
+
/* low bits of r->p determine size: 0 means >= page size and p->size holding
* real size, otherwise r->size is a shift count, or 1 for malloc(0)
*/
@@ -560,6 +563,12 @@ omalloc_init(struct dir_info **dp)
case 'J':
mopts.malloc_junk = 2;
break;
+ case 'v':
+ mopts.malloc_validate = 0;
+ break;
+ case 'V':
+ mopts.malloc_validate = 1;
+ break;
case 'n':
case 'N':
break;
@@ -608,6 +617,9 @@ omalloc_init(struct dir_info **dp)
}
}
+ if (!mopts.malloc_junk)
+ mopts.malloc_validate = 0;
+
#ifdef MALLOC_STATS
if (mopts.malloc_stats && (atexit(malloc_exit) == -1)) {
static const char q[] = "malloc() warning: atexit(2) failed."
@@ -616,6 +628,12 @@ omalloc_init(struct dir_info **dp)
}
#endif /* MALLOC_STATS */
+ if (mopts.malloc_validate && (atexit(validate_delayed_chunks) == -1)) {
+ static const char q[] = "malloc() warning: atexit(2) failed."
+ " Will not be able to check for use after free\n";
+ write(STDERR_FILENO, q, sizeof(q) - 1);
+ }
+
while ((mopts.malloc_canary = arc4random()) == 0)
;
@@ -1190,6 +1208,35 @@ malloc(size_t size)
/*DEF_STRONG(malloc);*/
static void
+validate_junk(void *p) {
+ struct region_info *r;
+ struct dir_info *pool = getpool();
+ size_t byte, sz;
+ if (p == NULL)
+ return;
+ r = find(pool, p);
+ if (r == NULL)
+ wrterror("bogus pointer in validate_junk", p);
+ REALSIZE(sz, r);
+ for (byte = 0; byte < sz; byte++) {
+ if (((char *)p)[byte] != SOME_FREEJUNK) {
+ wrterror("use after free", p);
+ return;
+ }
+ }
+}
+
+static void
+validate_delayed_chunks(void) {
+ struct dir_info *pool = getpool();
+ int i;
+ if (pool == NULL)
+ return;
+ for (i = 0; i < MALLOC_DELAYED_CHUNK_MASK + 1; i++)
+ validate_junk(pool->delayed_chunks[i]);
+}
+
+static void
ofree(void *p)
{
struct dir_info *pool = getpool();
@@ -1253,6 +1300,8 @@ ofree(void *p)
wrterror("double free", p);
return;
}
+ if (mopts.malloc_validate)
+ validate_junk(p);
pool->delayed_chunks[i] = tmp;
}
if (p != NULL) {
Re: enhanced use-after-free detection for malloc v2
On 26/10/15 04:19 PM, Daniel Micay wrote:
> This is an improved revision of my earlier patch.
>
> It now validates the junk data in the delayed_chunks array in an atexit
> handler
> too, rather than just when allocations are swapped out.
>
> It will now catch this simple UAF 100% of the time:
>
> #include
> #include
>
> int main(void) {
> size_t i;
> char *p;
> for (i = 0; i < 32; i++) {
> p = malloc(16);
> if (!p) return 1;
> }
>
> p = malloc(16);
> if (!p) return 1;
> free(p);
> *p = 5;
>
> for (i = 0; i < 4; i++) {
> p = malloc(16);
> if (!p) return 1;
> free(p);
> }
> return 0;
> }
>
> In general, it depends on the allocation still being in the delayed chunks
> array when the use-after-free happens. This means a larger delayed chunks
> array would improve the detection rate.
That revision was missing a NULL check as it got lost when refactoring
and the test cases weren't short enough to trigger it. Properly working
implementation:
diff --git a/stdlib/malloc.c b/stdlib/malloc.c
index 424dd77..c408594 100644
--- a/stdlib/malloc.c
+++ b/stdlib/malloc.c
@@ -182,6 +182,7 @@ struct malloc_readonly {
int malloc_freeunmap; /* mprotect free pages PROT_NONE? */
int malloc_hint;/* call madvice on free pages? */
int malloc_junk;/* junk fill? */
+ int malloc_validate;/* validate junk */
int malloc_move;/* move allocations to end of page? */
int malloc_realloc; /* always realloc? */
int malloc_xmalloc; /* xmalloc behaviour? */
@@ -218,6 +219,8 @@ static void malloc_exit(void);
#define CALLER NULL
#endif
+static void validate_delayed_chunks(void);
+
/* low bits of r->p determine size: 0 means >= page size and p->size
holding
* real size, otherwise r->size is a shift count, or 1 for malloc(0)
*/
@@ -560,6 +563,12 @@ omalloc_init(struct dir_info **dp)
case 'J':
mopts.malloc_junk = 2;
break;
+ case 'v':
+ mopts.malloc_validate = 0;
+ break;
+ case 'V':
+ mopts.malloc_validate = 1;
+ break;
case 'n':
case 'N':
break;
@@ -608,6 +617,9 @@ omalloc_init(struct dir_info **dp)
}
}
+ if (!mopts.malloc_junk)
+ mopts.malloc_validate = 0;
+
#ifdef MALLOC_STATS
if (mopts.malloc_stats && (atexit(malloc_exit) == -1)) {
static const char q[] = "malloc() warning: atexit(2) failed."
@@ -616,6 +628,12 @@ omalloc_init(struct dir_info **dp)
}
#endif /* MALLOC_STATS */
+ if (mopts.malloc_validate && (atexit(validate_delayed_chunks) == -1)) {
+ static const char q[] = "malloc() warning: atexit(2) failed."
+ " Will not be able to check for use after free\n";
+ write(STDERR_FILENO, q, sizeof(q) - 1);
+ }
+
while ((mopts.malloc_canary = arc4random()) == 0)
;
@@ -1190,6 +1208,35 @@ malloc(size_t size)
/*DEF_STRONG(malloc);*/
static void
+validate_junk(void *p) {
+ struct region_info *r;
+ struct dir_info *pool = getpool();
+ size_t byte, sz;
+ if (p == NULL)
+ return;
+ r = find(pool, p);
+ if (r == NULL)
+ wrterror("bogus pointer in validate_junk", p);
+ REALSIZE(sz, r);
+ for (byte = 0; byte < sz; byte++) {
+ if (((char *)p)[byte] != SOME_FREEJUNK) {
+ wrterror("use after free", p);
+ return;
+ }
+ }
+}
+
+static void
+validate_delayed_chunks(void) {
+ struct dir_info *pool = getpool();
+ int i;
+ if (pool == NULL)
+ return;
+ for (i = 0; i < MALLOC_DELAYED_CHUNK_MASK + 1; i++)
+ validate_junk(pool->delayed_chunks[i]);
+}
+
+static void
ofree(void *p)
{
struct dir_info *pool = getpool();
@@ -1253,6 +1300,8 @@ ofree(void *p)
wrterror("double free", p);
return;
}
+ if (mopts.malloc_validate)
+ validate_junk(p);
pool->delayed_chunks[i] = tmp;
}
if (p != NULL) {
enhanced use-after-free detection for malloc v2
This is an improved revision of my earlier patch.
It now validates the junk data in the delayed_chunks array in an atexit handler
too, rather than just when allocations are swapped out.
It will now catch this simple UAF 100% of the time:
#include
#include
int main(void) {
size_t i;
char *p;
for (i = 0; i < 32; i++) {
p = malloc(16);
if (!p) return 1;
}
p = malloc(16);
if (!p) return 1;
free(p);
*p = 5;
for (i = 0; i < 4; i++) {
p = malloc(16);
if (!p) return 1;
free(p);
}
return 0;
}
In general, it depends on the allocation still being in the delayed chunks
array when the use-after-free happens. This means a larger delayed chunks
array would improve the detection rate.
diff --git a/stdlib/malloc.c b/stdlib/malloc.c
index 424dd77..4a635b6 100644
--- a/stdlib/malloc.c
+++ b/stdlib/malloc.c
@@ -182,6 +182,7 @@ struct malloc_readonly {
int malloc_freeunmap; /* mprotect free pages PROT_NONE? */
int malloc_hint;/* call madvice on free pages? */
int malloc_junk;/* junk fill? */
+ int malloc_validate;/* validate junk */
int malloc_move;/* move allocations to end of page? */
int malloc_realloc; /* always realloc? */
int malloc_xmalloc; /* xmalloc behaviour? */
@@ -218,6 +219,8 @@ static void malloc_exit(void);
#define CALLER NULL
#endif
+static void validate_delayed_chunks(void);
+
/* low bits of r->p determine size: 0 means >= page size and p->size holding
* real size, otherwise r->size is a shift count, or 1 for malloc(0)
*/
@@ -560,6 +563,12 @@ omalloc_init(struct dir_info **dp)
case 'J':
mopts.malloc_junk = 2;
break;
+ case 'v':
+ mopts.malloc_validate = 0;
+ break;
+ case 'V':
+ mopts.malloc_validate = 1;
+ break;
case 'n':
case 'N':
break;
@@ -608,6 +617,9 @@ omalloc_init(struct dir_info **dp)
}
}
+ if (!mopts.malloc_junk)
+ mopts.malloc_validate = 0;
+
#ifdef MALLOC_STATS
if (mopts.malloc_stats && (atexit(malloc_exit) == -1)) {
static const char q[] = "malloc() warning: atexit(2) failed."
@@ -616,6 +628,12 @@ omalloc_init(struct dir_info **dp)
}
#endif /* MALLOC_STATS */
+ if (mopts.malloc_validate && (atexit(validate_delayed_chunks) == -1)) {
+ static const char q[] = "malloc() warning: atexit(2) failed."
+ " Will not be able to check for use after free\n";
+ write(STDERR_FILENO, q, sizeof(q) - 1);
+ }
+
while ((mopts.malloc_canary = arc4random()) == 0)
;
@@ -1190,6 +1208,33 @@ malloc(size_t size)
/*DEF_STRONG(malloc);*/
static void
+validate_junk(void *p) {
+ struct region_info *r;
+ struct dir_info *pool = getpool();
+ size_t byte, sz;
+ r = find(pool, p);
+ if (r == NULL)
+ wrterror("bogus pointer in validate_junk", p);
+ REALSIZE(sz, r);
+ for (byte = 0; byte < sz; byte++) {
+ if (((char *)p)[byte] != SOME_FREEJUNK) {
+ wrterror("use after free", p);
+ return;
+ }
+ }
+}
+
+static void
+validate_delayed_chunks(void) {
+ struct dir_info *pool = getpool();
+ int i;
+ if (pool == NULL)
+ return;
+ for (i = 0; i < MALLOC_DELAYED_CHUNK_MASK + 1; i++)
+ validate_junk(pool->delayed_chunks[i]);
+}
+
+static void
ofree(void *p)
{
struct dir_info *pool = getpool();
@@ -1253,6 +1298,8 @@ ofree(void *p)
wrterror("double free", p);
return;
}
+ if (mopts.malloc_validate)
+ validate_junk(p);
pool->delayed_chunks[i] = tmp;
}
if (p != NULL) {
Re: enhanced use-after-free detection for malloc
Er, here it is without the screwed up whitespace (whoops):
diff --git a/stdlib/malloc.c b/stdlib/malloc.c
index 424dd77..7c33a7a 100644
--- a/stdlib/malloc.c
+++ b/stdlib/malloc.c
@@ -182,6 +182,7 @@ struct malloc_readonly {
int malloc_freeunmap; /* mprotect free pages PROT_NONE? */
int malloc_hint;/* call madvice on free pages? */
int malloc_junk;/* junk fill? */
+ int malloc_validate;/* validate junk */
int malloc_move;/* move allocations to end of page? */
int malloc_realloc; /* always realloc? */
int malloc_xmalloc; /* xmalloc behaviour? */
@@ -560,6 +561,12 @@ omalloc_init(struct dir_info **dp)
case 'J':
mopts.malloc_junk = 2;
break;
+ case 'v':
+ mopts.malloc_validate = 0;
+ break;
+ case 'V':
+ mopts.malloc_validate = 1;
+ break;
case 'n':
case 'N':
break;
@@ -1253,6 +1260,17 @@ ofree(void *p)
wrterror("double free", p);
return;
}
+ if (mopts.malloc_junk && mopts.malloc_validate && p !=
NULL) {
+ size_t byte;
+ r = find(pool, p);
+ REALSIZE(sz, r);
+ for (byte = 0; byte < sz; byte++) {
+ if (((char *)p)[byte] != SOME_FREEJUNK)
{
+ wrterror("use after free", p);
+ return;
+ }
+ }
+ }
pool->delayed_chunks[i] = tmp;
}
if (p != NULL) {
--
2.6.2
enhanced use-after-free detection for malloc
This patch adds a form of use-after-free detection based on validating that
the junk data is still in place when swapping out an allocation from the
delayed chunk cache. It will probably nearly double the cost of the junk free
feature that's enabled by default since it needs to do a whole extra pass over
the data, thus splitting it out into a separate option instead of always doing
it. It can catch UAF issues when Freeguard / Free unmap will not since those
can only kick in when a whole page is cleared out.
This could be extended to also check that the data is *either* zeroed or junk
when handing out a chunk as a new allocation. That would add an additional
pass over the data, so perhaps this should be given 3 levels like the junk
free feature (like this patch by default, disabled with v, also validating the
data when doing allocation with V).
diff --git a/stdlib/malloc.c b/stdlib/malloc.c
index 424dd77..7c33a7a 100644
--- a/stdlib/malloc.c
+++ b/stdlib/malloc.c
@@ -182,6 +182,7 @@ struct malloc_readonly {
int malloc_freeunmap; /* mprotect free pages PROT_NONE? */
int malloc_hint;/* call madvice on free pages? */
int malloc_junk;/* junk fill? */
+ int malloc_validate;/* validate junk */
int malloc_move;/* move allocations to end of page? */
int malloc_realloc; /* always realloc? */
int malloc_xmalloc; /* xmalloc behaviour? */
@@ -560,6 +561,12 @@ omalloc_init(struct dir_info **dp)
case 'J':
mopts.malloc_junk = 2;
break;
+ case 'v':
+ mopts.malloc_validate = 0;
+ break;
+ case 'V':
+ mopts.malloc_validate = 1;
+ break;
case 'n':
case 'N':
break;
@@ -1253,6 +1260,17 @@ ofree(void *p)
wrterror("double free", p);
return;
}
+ if (mopts.malloc_junk && mopts.malloc_validate && p !=
NULL) {
+ size_t byte;
+ r = find(pool, p);
+ REALSIZE(sz, r);
+ for (byte = 0; byte < sz; byte++) {
+ if (((char *)p)[byte] != SOME_FREEJUNK)
{
+ wrterror("use after free", p);
+ return;
+ }
+ }
+ }
pool->delayed_chunks[i] = tmp;
}
if (p != NULL) {
