diff mbox

malloc: Correct size computation in realloc for dumped fake mmapped chunks

Message ID 20160608185123.BB0D84012CF92@oldenburg.str.redhat.com
State New
Headers show

Commit Message

Florian Weimer June 8, 2016, 6:51 p.m. UTC
For regular mmapped chunks there are two size fields (hence a reduction
by 2 * SIZE_SZ bytes), but for fake chunks, we only have one size field,
so we need to subtract SIZE_SZ bytes.

This was initially reported as Emacs bug 23726.

2016-06-08  Florian Weimer  <fweimer@redhat.com>

	Emacs bug 23726.
	* malloc/malloc.c (dumped_main_arena_start): Update comment.
	(__libc_realloc): Correct size computation for dumped fake mmapped
	chunks.

Comments

Paul Eggert June 8, 2016, 6:55 p.m. UTC | #1
Thanks for the quick work! This patch looks good to me.
Carlos O'Donell June 8, 2016, 8:38 p.m. UTC | #2
On 06/08/2016 02:51 PM, Florian Weimer wrote:
> For regular mmapped chunks there are two size fields (hence a reduction
> by 2 * SIZE_SZ bytes), but for fake chunks, we only have one size field,
> so we need to subtract SIZE_SZ bytes.
> 
> This was initially reported as Emacs bug 23726.
> 
> 2016-06-08  Florian Weimer  <fweimer@redhat.com>
> 
> 	Emacs bug 23726.
> 	* malloc/malloc.c (dumped_main_arena_start): Update comment.
> 	(__libc_realloc): Correct size computation for dumped fake mmapped
> 	chunks.

Reviewed the existing mmapped chunk code, and this fix looks good to me.
Mike Frysinger June 9, 2016, 7:22 p.m. UTC | #3
On 08 Jun 2016 20:51, Florian Weimer wrote:
> For regular mmapped chunks there are two size fields (hence a reduction
> by 2 * SIZE_SZ bytes), but for fake chunks, we only have one size field,
> so we need to subtract SIZE_SZ bytes.
> 
> This was initially reported as Emacs bug 23726.

is it normally user visible ?  if so, i think we'd want a bug on our side
too for it.
-mike
Florian Weimer June 9, 2016, 7:45 p.m. UTC | #4
On 06/09/2016 09:22 PM, Mike Frysinger wrote:
> On 08 Jun 2016 20:51, Florian Weimer wrote:
>> For regular mmapped chunks there are two size fields (hence a reduction
>> by 2 * SIZE_SZ bytes), but for fake chunks, we only have one size field,
>> so we need to subtract SIZE_SZ bytes.
>>
>> This was initially reported as Emacs bug 23726.
>
> is it normally user visible ?  if so, i think we'd want a bug on our side
> too for it.

Well, it was, unless you were a vi user.

But the bug was never part of a released version, and the patch isn't 
exactly backporting material.  So it doesn't really matter from a 
release documentation and tracking perspective.

I can file a bug and put it into the appropriate state.  I normally file 
such bugs only if there's a risk I get side-tracked and fail to fix the bug.

Thanks,
Florian
Mike Frysinger June 9, 2016, 8:19 p.m. UTC | #5
On 09 Jun 2016 21:45, Florian Weimer wrote:
> On 06/09/2016 09:22 PM, Mike Frysinger wrote:
> > On 08 Jun 2016 20:51, Florian Weimer wrote:
> >> For regular mmapped chunks there are two size fields (hence a reduction
> >> by 2 * SIZE_SZ bytes), but for fake chunks, we only have one size field,
> >> so we need to subtract SIZE_SZ bytes.
> >>
> >> This was initially reported as Emacs bug 23726.
> >
> > is it normally user visible ?  if so, i think we'd want a bug on our side
> > too for it.
> 
> Well, it was, unless you were a vi user.
> 
> But the bug was never part of a released version, and the patch isn't 
> exactly backporting material.  So it doesn't really matter from a 
> release documentation and tracking perspective.

np then
-mike
diff mbox

Patch

diff --git a/malloc/malloc.c b/malloc/malloc.c
index ead9a21..6f77d37 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1748,7 +1748,9 @@  static struct malloc_state main_arena =
 
 /* These variables are used for undumping support.  Chunked are marked
    as using mmap, but we leave them alone if they fall into this
-   range.  */
+   range.  NB: The chunk size for these chunks only includes the
+   initial size field (of SIZE_SZ bytes), there is no trailing size
+   field (unlike with regular mmapped chunks).  */
 static mchunkptr dumped_main_arena_start; /* Inclusive.  */
 static mchunkptr dumped_main_arena_end;   /* Exclusive.  */
 
@@ -3029,9 +3031,11 @@  __libc_realloc (void *oldmem, size_t bytes)
 	  if (newmem == 0)
 	    return NULL;
 	  /* Copy as many bytes as are available from the old chunk
-	     and fit into the new size.  */
-	  if (bytes > oldsize - 2 * SIZE_SZ)
-	    bytes = oldsize - 2 * SIZE_SZ;
+	     and fit into the new size.  NB: The overhead for faked
+	     mmapped chunks is only SIZE_SZ, not 2 * SIZE_SZ as for
+	     regular mmapped chunks.  */
+	  if (bytes > oldsize - SIZE_SZ)
+	    bytes = oldsize - SIZE_SZ;
 	  memcpy (newmem, oldmem, bytes);
 	  return newmem;
 	}