Re: realloc(3) and MALLOC_MOVE
On Tue, Jan 24, 2017 at 10:07:23AM +0100, Otto Moerbeek wrote:
> Hi,
>
> malloc(3) has the nice feature to move (subject to alignment
> constraints) allocations that are between the max chunk size (half a
> page) and a page size towards the end of the allocated page, to catch
> more buffer overflows. Due to the allocation being higher up within a
> page, buffer overflows will end up beyond the page in more cases.
>
> Until now, realloc(3) did not handle this in a smart way. When it
> encountered an existing moved allocation it always did a
> malloc-copy-free dance.
>
> This diff fixes that, at the same time doing more cases without
> requesting new pages from the kernel.
>
> Please review and test,
>
> BTW, we cannot use this feature for allocations larger than or equal
> to a page, since these should return page-aligned pointers.
>
> -Otto
>
Anybody testing this? I like to make progress on this...
-Otto
> Index: malloc.c
> ===
> RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.212
> diff -u -p -r1.212 malloc.c
> --- malloc.c 21 Jan 2017 07:47:42 - 1.212
> +++ malloc.c 24 Jan 2017 06:08:13 -
> @@ -1328,13 +1328,9 @@ ofree(struct dir_info *argpool, void *p)
> sz - mopts.malloc_guard,
> PAGEROUND(sz - mopts.malloc_guard));
> } else {
> -#if notyetbecause_of_realloc
> /* shifted towards the end */
> - if (p != ((char *)r->p) + ((MALLOC_PAGESIZE -
> - MALLOC_MINSIZE - sz - mopts.malloc_guard) &
> - ~(MALLOC_MINSIZE-1))) {
> - }
> -#endif
> + if (p != MALLOC_MOVE(r->p, sz))
> + wrterror(pool, "bogus moved pointer %p", p);
> p = r->p;
> }
> if (mopts.malloc_guard) {
> @@ -1474,7 +1470,7 @@ orealloc(struct dir_info *argpool, void
> if (gnewsz > MALLOC_MAXCHUNK)
> gnewsz += mopts.malloc_guard;
>
> - if (newsz > MALLOC_MAXCHUNK && oldsz > MALLOC_MAXCHUNK && p == r->p &&
> + if (newsz > MALLOC_MAXCHUNK && oldsz > MALLOC_MAXCHUNK &&
> !mopts.malloc_realloc) {
> /* First case: from n pages sized allocation to m pages sized
> allocation, no malloc_move in effect */
> @@ -1484,7 +1480,7 @@ orealloc(struct dir_info *argpool, void
> if (rnewsz > roldsz) {
> /* try to extend existing region */
> if (!mopts.malloc_guard) {
> - void *hint = (char *)p + roldsz;
> + void *hint = (char *)r->p + roldsz;
> size_t needed = rnewsz - roldsz;
>
> STATS_INC(pool->cheap_realloc_tries);
> @@ -1502,9 +1498,15 @@ gotit:
> STATS_ADD(pool->malloc_used, needed);
> if (mopts.malloc_junk == 2)
> memset(q, SOME_JUNK, needed);
> - r->size = newsz;
> + r->size = gnewsz;
> + if (r->p != p) {
> + /* old pointer is moved */
> + memmove(r->p, p, oldsz);
> + p = r->p;
> + }
> if (mopts.chunk_canaries)
> - fill_canary(p, newsz,
> PAGEROUND(newsz));
> + fill_canary(p, newsz,
> + PAGEROUND(newsz));
> STATS_SETF(r, f);
> STATS_INC(pool->cheap_reallocs);
> ret = p;
> @@ -1517,30 +1519,45 @@ gotit:
> } else if (rnewsz < roldsz) {
> /* shrink number of pages */
> if (mopts.malloc_guard) {
> - if (mprotect((char *)p + roldsz -
> + if (mprotect((char *)r->p + roldsz -
> mopts.malloc_guard, mopts.malloc_guard,
> PROT_READ | PROT_WRITE))
> wrterror(pool, "mprotect");
> - if (mprotect((char *)p + rnewsz -
> + if (mprotect((char *)r->p + rnewsz -
> mopts.malloc_guard, mopts.malloc_guard,
> PROT_NONE))
> wrterror(pool, "mprotect");
> }
> -
realloc(3) and MALLOC_MOVE
Hi,
malloc(3) has the nice feature to move (subject to alignment
constraints) allocations that are between the max chunk size (half a
page) and a page size towards the end of the allocated page, to catch
more buffer overflows. Due to the allocation being higher up within a
page, buffer overflows will end up beyond the page in more cases.
Until now, realloc(3) did not handle this in a smart way. When it
encountered an existing moved allocation it always did a
malloc-copy-free dance.
This diff fixes that, at the same time doing more cases without
requesting new pages from the kernel.
Please review and test,
BTW, we cannot use this feature for allocations larger than or equal
to a page, since these should return page-aligned pointers.
-Otto
Index: malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.212
diff -u -p -r1.212 malloc.c
--- malloc.c21 Jan 2017 07:47:42 - 1.212
+++ malloc.c24 Jan 2017 06:08:13 -
@@ -1328,13 +1328,9 @@ ofree(struct dir_info *argpool, void *p)
sz - mopts.malloc_guard,
PAGEROUND(sz - mopts.malloc_guard));
} else {
-#if notyetbecause_of_realloc
/* shifted towards the end */
- if (p != ((char *)r->p) + ((MALLOC_PAGESIZE -
- MALLOC_MINSIZE - sz - mopts.malloc_guard) &
- ~(MALLOC_MINSIZE-1))) {
- }
-#endif
+ if (p != MALLOC_MOVE(r->p, sz))
+ wrterror(pool, "bogus moved pointer %p", p);
p = r->p;
}
if (mopts.malloc_guard) {
@@ -1474,7 +1470,7 @@ orealloc(struct dir_info *argpool, void
if (gnewsz > MALLOC_MAXCHUNK)
gnewsz += mopts.malloc_guard;
- if (newsz > MALLOC_MAXCHUNK && oldsz > MALLOC_MAXCHUNK && p == r->p &&
+ if (newsz > MALLOC_MAXCHUNK && oldsz > MALLOC_MAXCHUNK &&
!mopts.malloc_realloc) {
/* First case: from n pages sized allocation to m pages sized
allocation, no malloc_move in effect */
@@ -1484,7 +1480,7 @@ orealloc(struct dir_info *argpool, void
if (rnewsz > roldsz) {
/* try to extend existing region */
if (!mopts.malloc_guard) {
- void *hint = (char *)p + roldsz;
+ void *hint = (char *)r->p + roldsz;
size_t needed = rnewsz - roldsz;
STATS_INC(pool->cheap_realloc_tries);
@@ -1502,9 +1498,15 @@ gotit:
STATS_ADD(pool->malloc_used, needed);
if (mopts.malloc_junk == 2)
memset(q, SOME_JUNK, needed);
- r->size = newsz;
+ r->size = gnewsz;
+ if (r->p != p) {
+ /* old pointer is moved */
+ memmove(r->p, p, oldsz);
+ p = r->p;
+ }
if (mopts.chunk_canaries)
- fill_canary(p, newsz,
PAGEROUND(newsz));
+ fill_canary(p, newsz,
+ PAGEROUND(newsz));
STATS_SETF(r, f);
STATS_INC(pool->cheap_reallocs);
ret = p;
@@ -1517,30 +1519,45 @@ gotit:
} else if (rnewsz < roldsz) {
/* shrink number of pages */
if (mopts.malloc_guard) {
- if (mprotect((char *)p + roldsz -
+ if (mprotect((char *)r->p + roldsz -
mopts.malloc_guard, mopts.malloc_guard,
PROT_READ | PROT_WRITE))
wrterror(pool, "mprotect");
- if (mprotect((char *)p + rnewsz -
+ if (mprotect((char *)r->p + rnewsz -
mopts.malloc_guard, mopts.malloc_guard,
PROT_NONE))
wrterror(pool, "mprotect");
}
- unmap(pool, (char *)p + rnewsz, roldsz - rnewsz);
+ unmap(pool, (char *)r->p + rnewsz, roldsz - rnewsz);
r->size = gnewsz;
-
