[v2] Handle NULL input to malloc_usable_size [BZ #28506]
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
Hoist the NULL check for malloc_usable_size into its entry points in
malloc-debug and malloc and assume non-NULL in all callees. This fixes
BZ #28506
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
malloc/malloc-debug.c | 12 ++++++------
malloc/malloc.c | 24 ++++++++----------------
malloc/tst-malloc-usable.c | 21 ++++++++-------------
3 files changed, 22 insertions(+), 35 deletions(-)
Comments
On Fri, Oct 29, 2021 at 02:25:21PM +0530, Siddhesh Poyarekar wrote:
> Hoist the NULL check for malloc_usable_size into its entry points in
> malloc-debug and malloc and assume non-NULL in all callees. This fixes
> BZ #28506
>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> ---
> malloc/malloc-debug.c | 12 ++++++------
> malloc/malloc.c | 24 ++++++++----------------
> malloc/tst-malloc-usable.c | 21 ++++++++-------------
> 3 files changed, 22 insertions(+), 35 deletions(-)
>
> diff --git a/malloc/malloc-debug.c b/malloc/malloc-debug.c
> index 9922ef5f25..5e954d7dc2 100644
> --- a/malloc/malloc-debug.c
> +++ b/malloc/malloc-debug.c
> @@ -399,17 +399,17 @@ strong_alias (__debug_calloc, calloc)
> size_t
> malloc_usable_size (void *mem)
> {
> + if (mem == NULL)
> + return 0;
> +
> if (__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK))
> return mcheck_usable_size (mem);
> if (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK))
> return malloc_check_get_size (mem);
>
> - if (mem != NULL)
> - {
> - mchunkptr p = mem2chunk (mem);
> - if (DUMPED_MAIN_ARENA_CHUNK (p))
> - return chunksize (p) - SIZE_SZ;
> - }
> + mchunkptr p = mem2chunk (mem);
> + if (DUMPED_MAIN_ARENA_CHUNK (p))
> + return chunksize (p) - SIZE_SZ;
>
> return musable (mem);
> }
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 2ba1fee144..a9dfc82788 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -5007,20 +5007,13 @@ __malloc_trim (size_t s)
> static size_t
> musable (void *mem)
> {
> - mchunkptr p;
> - if (mem != 0)
> - {
> - size_t result = 0;
> -
> - p = mem2chunk (mem);
> + mchunkptr p = mem2chunk (mem);
>
> - if (chunk_is_mmapped (p))
> - result = chunksize (p) - CHUNK_HDR_SZ;
> - else if (inuse (p))
> - result = memsize (p);
> + if (chunk_is_mmapped (p))
> + return chunksize (p) - CHUNK_HDR_SZ;
> + else if (inuse (p))
> + return memsize (p);
>
> - return result;
> - }
> return 0;
> }
>
> @@ -5028,10 +5021,9 @@ musable (void *mem)
> size_t
> __malloc_usable_size (void *m)
> {
> - size_t result;
> -
> - result = musable (m);
> - return result;
> + if (m == NULL)
> + return 0;
> + return musable (m);
> }
> #endif
>
> diff --git a/malloc/tst-malloc-usable.c b/malloc/tst-malloc-usable.c
> index a1074b782a..e50cadcf10 100644
> --- a/malloc/tst-malloc-usable.c
> +++ b/malloc/tst-malloc-usable.c
> @@ -21,29 +21,24 @@
> #include <malloc.h>
> #include <string.h>
> #include <stdio.h>
> +#include <support/support.h>
> +#include <support/check.h>
>
> static int
> do_test (void)
> {
> size_t usable_size;
> void *p = malloc (7);
> - if (!p)
> - {
> - printf ("memory allocation failed\n");
> - return 1;
> - }
>
> + TEST_VERIFY_EXIT (p != NULL);
> usable_size = malloc_usable_size (p);
> - if (usable_size != 7)
> - {
> - printf ("malloc_usable_size: expected 7 but got %zu\n", usable_size);
> - return 1;
> - }
> -
> + TEST_COMPARE (usable_size, 7);
> memset (p, 0, usable_size);
> free (p);
> +
> + TEST_COMPARE (malloc_usable_size (NULL), 0);
> +
> return 0;
> }
>
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include "support/test-driver.c"
Also looks sensible, so:
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Rich.
* Siddhesh Poyarekar:
> Hoist the NULL check for malloc_usable_size into its entry points in
> malloc-debug and malloc and assume non-NULL in all callees. This fixes
> BZ #28506
>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
I forgot: If you use Signed-off-by: for real (without FSF copyright
assignment), you need to adjust the copyright headers.
The rest of the patch looks okay to me.
Thanks,
Florian
@@ -399,17 +399,17 @@ strong_alias (__debug_calloc, calloc)
size_t
malloc_usable_size (void *mem)
{
+ if (mem == NULL)
+ return 0;
+
if (__is_malloc_debug_enabled (MALLOC_MCHECK_HOOK))
return mcheck_usable_size (mem);
if (__is_malloc_debug_enabled (MALLOC_CHECK_HOOK))
return malloc_check_get_size (mem);
- if (mem != NULL)
- {
- mchunkptr p = mem2chunk (mem);
- if (DUMPED_MAIN_ARENA_CHUNK (p))
- return chunksize (p) - SIZE_SZ;
- }
+ mchunkptr p = mem2chunk (mem);
+ if (DUMPED_MAIN_ARENA_CHUNK (p))
+ return chunksize (p) - SIZE_SZ;
return musable (mem);
}
@@ -5007,20 +5007,13 @@ __malloc_trim (size_t s)
static size_t
musable (void *mem)
{
- mchunkptr p;
- if (mem != 0)
- {
- size_t result = 0;
-
- p = mem2chunk (mem);
+ mchunkptr p = mem2chunk (mem);
- if (chunk_is_mmapped (p))
- result = chunksize (p) - CHUNK_HDR_SZ;
- else if (inuse (p))
- result = memsize (p);
+ if (chunk_is_mmapped (p))
+ return chunksize (p) - CHUNK_HDR_SZ;
+ else if (inuse (p))
+ return memsize (p);
- return result;
- }
return 0;
}
@@ -5028,10 +5021,9 @@ musable (void *mem)
size_t
__malloc_usable_size (void *m)
{
- size_t result;
-
- result = musable (m);
- return result;
+ if (m == NULL)
+ return 0;
+ return musable (m);
}
#endif
@@ -21,29 +21,24 @@
#include <malloc.h>
#include <string.h>
#include <stdio.h>
+#include <support/support.h>
+#include <support/check.h>
static int
do_test (void)
{
size_t usable_size;
void *p = malloc (7);
- if (!p)
- {
- printf ("memory allocation failed\n");
- return 1;
- }
+ TEST_VERIFY_EXIT (p != NULL);
usable_size = malloc_usable_size (p);
- if (usable_size != 7)
- {
- printf ("malloc_usable_size: expected 7 but got %zu\n", usable_size);
- return 1;
- }
-
+ TEST_COMPARE (usable_size, 7);
memset (p, 0, usable_size);
free (p);
+
+ TEST_COMPARE (malloc_usable_size (NULL), 0);
+
return 0;
}
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include "support/test-driver.c"