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 -0000 1.212
> +++ malloc.c 24 Jan 2017 06:08:13 -0000
> @@ -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;
> - if (mopts.chunk_canaries)
> + if (MALLOC_MOVE_COND(gnewsz)) {
> + void *pp = MALLOC_MOVE(r->p, gnewsz);
> + memmove(pp, p, newsz);
> + p = pp;
> + } else if (mopts.chunk_canaries)
> fill_canary(p, newsz, PAGEROUND(newsz));
> STATS_SETF(r, f);
> ret = p;
> goto done;
> } else {
> /* number of pages remains the same */
> - if (newsz > oldsz && mopts.malloc_junk == 2)
> - memset((char *)p + newsz, SOME_JUNK,
> - rnewsz - mopts.malloc_guard - newsz);
> + void *pp = r->p;
> +
> r->size = gnewsz;
> - if (mopts.chunk_canaries)
> - fill_canary(p, newsz, PAGEROUND(newsz));
> + if (MALLOC_MOVE_COND(gnewsz))
> + pp = MALLOC_MOVE(r->p, gnewsz);
> + if (p != pp) {
> + memmove(pp, p, oldsz < newsz ? oldsz : newsz);
> + p = pp;
> + }
> + if (p == r->p) {
> + if (newsz > oldsz && mopts.malloc_junk == 2)
> + memset((char *)p + newsz, SOME_JUNK,
> + rnewsz - mopts.malloc_guard -
> + newsz);
> + if (mopts.chunk_canaries)
> + fill_canary(p, newsz, PAGEROUND(newsz));
> + }
> STATS_SETF(r, f);
> ret = p;
> goto done;
