* [PATCH] Handle NULL input to malloc_usable_size [BZ #28506]
@ 2021-10-29 3:18 Siddhesh Poyarekar
2021-10-29 8:30 ` Florian Weimer
0 siblings, 1 reply; 7+ messages in thread
From: Siddhesh Poyarekar @ 2021-10-29 3:18 UTC (permalink / raw)
To: libc-alpha
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 | 23 ++++++++++-------------
malloc/tst-malloc-usable.c | 21 ++++++++-------------
3 files changed, 24 insertions(+), 32 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..9a345572a1 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -5008,29 +5008,26 @@ static size_t
musable (void *mem)
{
mchunkptr p;
- if (mem != 0)
- {
- size_t result = 0;
+ size_t result = 0;
- p = mem2chunk (mem);
+ 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))
+ result = chunksize (p) - CHUNK_HDR_SZ;
+ else if (inuse (p))
+ result = memsize (p);
- return result;
- }
- return 0;
+ return result;
}
#if IS_IN (libc)
size_t
__malloc_usable_size (void *m)
{
- size_t result;
+ size_t result = 0;
- result = musable (m);
+ if (m != NULL)
+ result = musable (m);
return result;
}
#endif
diff --git a/malloc/tst-malloc-usable.c b/malloc/tst-malloc-usable.c
index a1074b782a..cd5c27cfcd 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_VERIFY_EXIT (usable_size == 7);
memset (p, 0, usable_size);
free (p);
+
+ TEST_VERIFY_EXIT (malloc_usable_size (NULL) == 0);
+
return 0;
}
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include "support/test-driver.c"
--
2.31.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Handle NULL input to malloc_usable_size [BZ #28506]
2021-10-29 3:18 [PATCH] Handle NULL input to malloc_usable_size [BZ #28506] Siddhesh Poyarekar
@ 2021-10-29 8:30 ` Florian Weimer
2021-10-29 8:55 ` [PATCH v2] " Siddhesh Poyarekar
0 siblings, 1 reply; 7+ messages in thread
From: Florian Weimer @ 2021-10-29 8:30 UTC (permalink / raw)
To: Siddhesh Poyarekar via Libc-alpha; +Cc: Siddhesh Poyarekar
* Siddhesh Poyarekar via Libc-alpha:
> size_t
> __malloc_usable_size (void *m)
> {
> + size_t result = 0;
>
> + if (m != NULL)
> + result = musable (m);
> return result;
> }
Is there a reason for not writing it this way?
size_t
__malloc_usable_size (void *m)
{
if (m == NULL)
return 0;
return musable (m);
}
The extra variable seems a bit … unnecessary?
> diff --git a/malloc/tst-malloc-usable.c b/malloc/tst-malloc-usable.c
> index a1074b782a..cd5c27cfcd 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);
>
> + TEST_VERIFY_EXIT (p != NULL);
> usable_size = malloc_usable_size (p);
> + TEST_VERIFY_EXIT (usable_size == 7);
You can use TEST_COMPARE here.
> memset (p, 0, usable_size);
> free (p);
> +
> + TEST_VERIFY_EXIT (malloc_usable_size (NULL) == 0);
And here.
Thanks,
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] Handle NULL input to malloc_usable_size [BZ #28506]
2021-10-29 8:30 ` Florian Weimer
@ 2021-10-29 8:55 ` Siddhesh Poyarekar
2021-10-29 9:00 ` Richard W.M. Jones
2021-10-29 9:06 ` Florian Weimer
0 siblings, 2 replies; 7+ messages in thread
From: Siddhesh Poyarekar @ 2021-10-29 8:55 UTC (permalink / raw)
To: libc-alpha; +Cc: rjones, fweimer
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"
--
2.31.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Handle NULL input to malloc_usable_size [BZ #28506]
2021-10-29 8:55 ` [PATCH v2] " Siddhesh Poyarekar
@ 2021-10-29 9:00 ` Richard W.M. Jones
2021-10-29 9:06 ` Florian Weimer
1 sibling, 0 replies; 7+ messages in thread
From: Richard W.M. Jones @ 2021-10-29 9:00 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: libc-alpha, fweimer
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.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Handle NULL input to malloc_usable_size [BZ #28506]
2021-10-29 8:55 ` [PATCH v2] " Siddhesh Poyarekar
2021-10-29 9:00 ` Richard W.M. Jones
@ 2021-10-29 9:06 ` Florian Weimer
2021-10-29 9:25 ` [COMMITTED] " Siddhesh Poyarekar
1 sibling, 1 reply; 7+ messages in thread
From: Florian Weimer @ 2021-10-29 9:06 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: libc-alpha, rjones
* 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [COMMITTED] Handle NULL input to malloc_usable_size [BZ #28506]
2021-10-29 9:06 ` Florian Weimer
@ 2021-10-29 9:25 ` Siddhesh Poyarekar
2021-10-29 9:27 ` Florian Weimer
0 siblings, 1 reply; 7+ messages in thread
From: Siddhesh Poyarekar @ 2021-10-29 9:25 UTC (permalink / raw)
To: libc-alpha; +Cc: Florian Weimer, Richard W . M . Jones
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>
Reviewed-by: Florian Weimer <fweimer@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
---
malloc/malloc-debug.c | 13 +++++++------
malloc/malloc.c | 25 +++++++++----------------
malloc/tst-malloc-usable.c | 22 +++++++++-------------
3 files changed, 25 insertions(+), 35 deletions(-)
diff --git a/malloc/malloc-debug.c b/malloc/malloc-debug.c
index 9922ef5f25..3d7e6d44fd 100644
--- a/malloc/malloc-debug.c
+++ b/malloc/malloc-debug.c
@@ -1,5 +1,6 @@
/* Malloc debug DSO.
Copyright (C) 2021 Free Software Foundation, Inc.
+ Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -399,17 +400,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..095d97a3be 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1,5 +1,6 @@
/* Malloc implementation for multiple threads without lock contention.
Copyright (C) 1996-2021 Free Software Foundation, Inc.
+ Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -5007,20 +5008,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 +5022,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..b0d702be10 100644
--- a/malloc/tst-malloc-usable.c
+++ b/malloc/tst-malloc-usable.c
@@ -2,6 +2,7 @@
MALLOC_CHECK_ exported to a positive value.
Copyright (C) 2012-2021 Free Software Foundation, Inc.
+ Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -21,29 +22,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"
--
2.31.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [COMMITTED] Handle NULL input to malloc_usable_size [BZ #28506]
2021-10-29 9:25 ` [COMMITTED] " Siddhesh Poyarekar
@ 2021-10-29 9:27 ` Florian Weimer
0 siblings, 0 replies; 7+ messages in thread
From: Florian Weimer @ 2021-10-29 9:27 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: libc-alpha, Richard W . M . Jones
* 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>
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
> Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Looks good, thanks.
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-29 9:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-29 3:18 [PATCH] Handle NULL input to malloc_usable_size [BZ #28506] Siddhesh Poyarekar
2021-10-29 8:30 ` Florian Weimer
2021-10-29 8:55 ` [PATCH v2] " Siddhesh Poyarekar
2021-10-29 9:00 ` Richard W.M. Jones
2021-10-29 9:06 ` Florian Weimer
2021-10-29 9:25 ` [COMMITTED] " Siddhesh Poyarekar
2021-10-29 9:27 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).