[Bug malloc/22057] malloc_usable_size is broken with mcheck

["siddhesh at sourceware dot org" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=22057

Siddhesh Poyarekar <siddhesh at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
                 CC|                            |siddhesh at sourceware dot org
           Assignee|unassigned at sourceware dot org   |siddhesh at sourceware dot org
            Summary|mcheck does not set the     |malloc_usable_size is
                   |using_malloc_checking flag, |broken with mcheck
                   |so malloc_usable_size       |
                   |returns an invalid size     |
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-07-12

--- Comment #1 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
(In reply to mo from comment #0)
> The using_malloc_checking flag is only set in the __malloc_check_init
> function,
> which is called when enabling the additional security check via the
> environment variable MALLOC_CHECK_.

mcheck and MALLOC_CHECK_ are distinct features; using_malloc_checking flag is
only for MALLOC_CHECK_ and makes no difference to mcheck...

> When someone calls malloc_usable_size now, ultimately musable will check if
> using_malloc_checking is set and call the special malloc_check_get_size
> function.
> Because the bit isn't set malloc_usable_size will return the value at the
> normal size location which is the hdr->magic2 field now, which is set to
> this:
> 
> hdr->magic2 = (uintptr_t) hdr ^ MAGICWORD;
> 
> While this will be rarely used apart from debugging, this could still have
> some security implications as the value is most likely bigger than the
> actual size and
> if used for some bounds checking could lead to an overflow.

... however you're right in that this is a bug in mcheck.  In needs to override
malloc_usable_size like malloc_check does and provide its own result for size. 
I'll fix this after the malloc hooks have been removed and mcheck moved out
into a separate debug library.

Sample program:

#include <stdio.h>
#include <stdlib.h>
#include <malloc.h>

int
main (int argc, char **argv)
{
  size_t sz = 32;
  if (argc > 1)
    sz = strtoul (argv[2], NULL, 0);

  printf ("sz: %zu, usable: %zu\n", sz, malloc_usable_size (malloc (sz)));
}

Expected result:

sz: 32, usable: 40   /* usable should be a valid value >= sz */

Actual result:

sz: 32, usable: 4241992728    /* which is actually magic2 as OP pointed out */

-- 
You are receiving this mail because:
You are on the CC list for the bug.