diff mbox series

MTE: Do not pad size in realloc_check

Message ID 20201222155958.362134-1-siddhesh@sourceware.org
State New
Headers show
Series MTE: Do not pad size in realloc_check | expand

Commit Message

Siddhesh Poyarekar Dec. 22, 2020, 3:59 p.m. UTC
The MTE patch to add malloc support incorrectly padded the size passed
to _int_realloc by SIZE_SZ when it ought to have sent just the
chunksize.  Revert that bit of the change so that realloc works
correctly with MALLOC_CHECK_ set.

This also brings the realloc_check implementation back in sync with
libc_realloc.
---
 malloc/hooks.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

H.J. Lu Dec. 22, 2020, 5:02 p.m. UTC | #1
On Tue, Dec 22, 2020 at 8:00 AM Siddhesh Poyarekar via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> The MTE patch to add malloc support incorrectly padded the size passed
> to _int_realloc by SIZE_SZ when it ought to have sent just the
> chunksize.  Revert that bit of the change so that realloc works
> correctly with MALLOC_CHECK_ set.
>
> This also brings the realloc_check implementation back in sync with
> libc_realloc.
> ---
>  malloc/hooks.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/malloc/hooks.c b/malloc/hooks.c
> index 8a1c16dfa4..6474ba8b38 100644
> --- a/malloc/hooks.c
> +++ b/malloc/hooks.c
> @@ -315,7 +315,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
>    __libc_lock_unlock (main_arena.mutex);
>    if (!oldp)
>      malloc_printerr ("realloc(): invalid pointer");
> -  const INTERNAL_SIZE_T oldchsize = CHUNK_AVAILABLE_SIZE (oldp);
> +  const INTERNAL_SIZE_T oldsize = chunksize (oldp);
>
>    if (!checked_request2size (rb, &chnb))
>      goto invert;
> @@ -331,7 +331,8 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
>        else
>  #endif
>        {
> -        if (oldchsize >= chnb)
> +       /* Note the extra SIZE_SZ overhead. */
> +        if (oldsize - SIZE_SZ >= chnb)
>            newmem = oldmem; /* do nothing */
>          else
>            {
> @@ -340,7 +341,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
>             newmem = _int_malloc (&main_arena, rb);
>              if (newmem)
>                {
> -                memcpy (newmem, oldmem, oldchsize - CHUNK_HDR_SZ);
> +                memcpy (newmem, oldmem, oldsize - CHUNK_HDR_SZ);
>                  munmap_chunk (oldp);
>                }
>            }
> @@ -349,7 +350,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
>    else
>      {
>        top_check ();
> -      newmem = _int_realloc (&main_arena, oldp, oldchsize, chnb);
> +      newmem = _int_realloc (&main_arena, oldp, oldsize, chnb);
>      }
>
>    DIAG_PUSH_NEEDS_COMMENT;

Please add some tests with MALLOC_CHECK_=.
Siddhesh Poyarekar Dec. 23, 2020, 7:46 a.m. UTC | #2
On 12/22/20 10:32 PM, H.J. Lu wrote:
> 
> Please add some tests with MALLOC_CHECK_=.
> 

I've done it separately now, as a testing subsystem enhancement to run 
as many tests as possible with MALLOC_CHECK_=3.  Over time we can add 
more tests to test-mcheck across glibc.

This fix cleans up all failures in that patch.  Is it OK now?

https://sourceware.org/pipermail/libc-alpha/2020-December/121026.html

Thanks,
Siddhesh
H.J. Lu Dec. 23, 2020, 1:51 p.m. UTC | #3
On Tue, Dec 22, 2020 at 11:46 PM Siddhesh Poyarekar
<siddhesh@sourceware.org> wrote:
>
> On 12/22/20 10:32 PM, H.J. Lu wrote:
> >
> > Please add some tests with MALLOC_CHECK_=.
> >
>
> I've done it separately now, as a testing subsystem enhancement to run
> as many tests as possible with MALLOC_CHECK_=3.  Over time we can add
> more tests to test-mcheck across glibc.
>
> This fix cleans up all failures in that patch.  Is it OK now?

LGTM.

Thanks.

> https://sourceware.org/pipermail/libc-alpha/2020-December/121026.html
>
diff mbox series

Patch

diff --git a/malloc/hooks.c b/malloc/hooks.c
index 8a1c16dfa4..6474ba8b38 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -315,7 +315,7 @@  realloc_check (void *oldmem, size_t bytes, const void *caller)
   __libc_lock_unlock (main_arena.mutex);
   if (!oldp)
     malloc_printerr ("realloc(): invalid pointer");
-  const INTERNAL_SIZE_T oldchsize = CHUNK_AVAILABLE_SIZE (oldp);
+  const INTERNAL_SIZE_T oldsize = chunksize (oldp);
 
   if (!checked_request2size (rb, &chnb))
     goto invert;
@@ -331,7 +331,8 @@  realloc_check (void *oldmem, size_t bytes, const void *caller)
       else
 #endif
       {
-        if (oldchsize >= chnb)
+	/* Note the extra SIZE_SZ overhead. */
+        if (oldsize - SIZE_SZ >= chnb)
           newmem = oldmem; /* do nothing */
         else
           {
@@ -340,7 +341,7 @@  realloc_check (void *oldmem, size_t bytes, const void *caller)
 	    newmem = _int_malloc (&main_arena, rb);
             if (newmem)
               {
-                memcpy (newmem, oldmem, oldchsize - CHUNK_HDR_SZ);
+                memcpy (newmem, oldmem, oldsize - CHUNK_HDR_SZ);
                 munmap_chunk (oldp);
               }
           }
@@ -349,7 +350,7 @@  realloc_check (void *oldmem, size_t bytes, const void *caller)
   else
     {
       top_check ();
-      newmem = _int_realloc (&main_arena, oldp, oldchsize, chnb);
+      newmem = _int_realloc (&main_arena, oldp, oldsize, chnb);
     }
 
   DIAG_PUSH_NEEDS_COMMENT;