diff mbox series

[v2,2/2] malloc: make malloc fail with requests larger than PTRDIFF_MAX

Message ID 20181228141316.25952-2-adhemerval.zanella@linaro.org
State New
Headers show
Series [v2,1/2] Replace check_mul_overflow_size_t with __builtin_mul_overflow | expand

Commit Message

Adhemerval Zanella Netto Dec. 28, 2018, 2:13 p.m. UTC
Changes from previous version:

  - Fix wording on NEWS entry.
  - Added static assert to check for PTRDIFF_MAX == SIZE_MAX / 2.
  - Remove padsize check, requested size after aligning and padding
    are checked against PTRDIFF_MAX.  This does not guarantee that
    sysmalloc will eventually issue an request larger than PTRDIFF_MAX.
    One option to enforce it is to either add the PTRDIFF_MAX check on
    mmap or create an internal mmap implementation to enforce it.

--

As discussed previously on libc-alpha [1], this patch follows up the idea
and add both the __attribute_alloc_size__ on malloc functions (malloc,
calloc, realloc, reallocarray, valloc, pvalloc, memaling, and
posix_memalign) and limit maximum requested allocation size to up
PTRDIFF_MAX (taking in consideration internal padding and alignment).

This aligns glibc with gcc expected size defined by default warning
-Walloc-size-larger-than value which warns for allocation larger than
PTRDIFF_MAX.  It also aligns with gcc expectation regarding libc and
expected size, such as described in PR#67999 [2] and previously discussed
ISO C11 issues [3] on libc-alpha.

From the RFC thread [4] and previous discussion, it seems that consensus
is only to limit such requested size for malloc functions, not system
allocation one (mmap, sbrk, etc.).

The implementation changes checked_request2size to check for both overflow
and maximum object size up to PTRDIFF_MAX. No additional checks are done
on sysmalloc, so it can still issues mmap with values larger than
PTRDIFF_T depending of requested size.

Checked on x86_64-linux-gnu and i686-linux-gnu.

	[BZ #23741]
	* malloc/hooks.c (malloc_check, realloc_check): Use
	__builtin_add_overflow on overflow check and adapt to
	checked_request2size change.
	* malloc/malloc.c (__libc_malloc, __libc_realloc, _mid_memalign,
	__libc_pvalloc, __libc_calloc, _int_memalign): Limit maximum
	allocation size to PTRDIFF_MAX.
	(REQUEST_OUT_OF_RANGE): Remove macro.
	(checked_request2size): Change to inline function, use
	__builtin_add_overflow for overflow check and limit maximum requested
	size to PTRDIFF_MAX.
	(__libc_malloc, __libc_realloc, _int_malloc, _int_memalign): Limit
	maximum allocation size to PTRDIFF_MAX.
	(_mid_memalign): Rely on _int_memalign call for overflow check.
	(__libc_pvalloc): Use __builtin_add_overflow on overflow check.
	(__libc_calloc): Use __builtin_mul_overflow for overflow check and
	limit maximum requested size to PTRDIFF_MAX.
	* malloc/malloc.h (malloc, calloc, realloc, reallocarray, memalign,
	valloc, pvalloc): Add __attribute_alloc_size__.
	* stdlib/stdlib.h (malloc, realloc, reallocarray, valloc,
	posix_memalign): Likewise.
	* malloc/tst-malloc-too-large.c (do_test): Add check for allocation
	larger than PTRDIFF_MAX.
	* malloc/tst-memalign.c (do_test): Disable -Walloc-size-larger-than=
	around tests of malloc with negative sizes.
	* malloc/tst-posix_memalign.c (do_test): Likewise.
	* malloc/tst-pvalloc.c (do_test): Likewise.
	* malloc/tst-valloc.c (do_test): Likewise.
	* malloc/tst-reallocarray.c (do_test): Replace call to reallocarray
	with resulting size allocation larger than PTRDIFF_MAX with
	reallocarray_nowarn.
	(reallocarray_nowarn): New function.
	* NEWS: Mention the malloc function semantic change.

[1] https://sourceware.org/ml/libc-alpha/2018-11/msg00223.html
[2] https://gcc.gnu.org/bugzilla//show_bug.cgi?id=67999
[3] https://sourceware.org/ml/libc-alpha/2011-12/msg00066.html
[4] https://sourceware.org/ml/libc-alpha/2018-11/msg00224.html
---
 ChangeLog                     |  34 ++++++++++
 NEWS                          |   6 ++
 malloc/hooks.c                |  18 +++--
 malloc/malloc.c               | 121 ++++++++++++++++------------------
 malloc/malloc.h               |  17 +++--
 malloc/tst-malloc-too-large.c |  10 +++
 malloc/tst-memalign.c         |  10 +++
 malloc/tst-posix_memalign.c   |  10 +++
 malloc/tst-pvalloc.c          |  10 +++
 malloc/tst-reallocarray.c     |  27 ++++++--
 malloc/tst-valloc.c           |  10 +++
 stdlib/stdlib.h               |  13 ++--
 12 files changed, 198 insertions(+), 88 deletions(-)

-- 
2.17.1

Comments

Paul Eggert Dec. 31, 2018, 1:16 a.m. UTC | #1
Adhemerval Zanella wrote:
> Changes from previous version:


>    - Remove padsize check, requested size after aligning and padding

>      are checked against PTRDIFF_MAX.  This does not guarantee that

>      sysmalloc will eventually issue an request larger than PTRDIFF_MAX.

>      One option to enforce it is to either add the PTRDIFF_MAX check on

>      mmap or create an internal mmap implementation to enforce it.


Sorry, I'm having trouble parsing that, starting with the word "guarantee"; it 
sounds like you're saying sysmalloc must issue a request larger than 
PTRDIFF_MAX, which surely is the opposite of what is intended.

More important, I thought that the idea was to allow malloc (s) if s <= 
PTRDIFF_MAX, even if s > PTRDIFF_MAX - DELTA for some nonzero but small DELTA. 
See <https://www.sourceware.org/ml/libc-alpha/2018-12/msg00908.html>, where I 
don't understand the point of your reply 
<https://www.sourceware.org/ml/libc-alpha/2018-12/msg00953.html>, as the concern 
about the compiler is the compiler's assumption about the object that malloc 
returns, not about how glibc behaves internally (I am assuming that glibc is 
careful to never subtract pointers whose integer representations differ by more 
than PTRDIFF_MAX).

> +/* Check if REQ overflows when padded and aligned and if the resulting value

> +   is less than PTRDIFF_T.  Returns TRUE and the requested size or MINSIZE in

> +   case the value is less than MINSIZE on SZ or false if any of the previous

> +   check fail.  */

> +static inline bool

> +checked_request2size (size_t req, size_t *sz) __nonnull (1)

> +{

> +  size_t ret;

> +  if (__glibc_unlikely (__builtin_add_overflow (req,

> +						SIZE_SZ + MALLOC_ALIGN_MASK,

> +						&ret)))

> +    return false;

> +

> +  ret = ret < MINSIZE ? MINSIZE : ret & ~MALLOC_ALIGN_MASK;

> +  if (__glibc_unlikely (ret > PTRDIFF_MAX))

> +    return false;

> +

> +  *sz = ret;

> +  return true;

> +}


This function typically does 3 comparisons. Since we're assuming PTRDIFF_MAX <= 
SIZE_MAX / 2, we can shrink it to at most 2 comparisons. Something like the 
following, say, where on x86-64 the typical case is one conditional branch and 
one conditional move.

static inline bool
checked_request2size (size_t req, size_t *sz)
{
   if (__glibc_likely (req <= PTRDIFF_MAX))
     {
       int min_nontiny_size = SIZE_SZ < MINSIZE ? MINSIZE - SIZE_SZ : 0;
       size_t areq = MAX (min_nontiny_size, req);
       *sz = (areq + SIZE_SZ + MALLOC_ALIGN_MASK) & ~MALLOC_ALIGN_MASK;
       return true;
     }
   return false;
}
Adhemerval Zanella Netto Jan. 15, 2019, 1:19 p.m. UTC | #2
On 30/12/2018 23:16, Paul Eggert wrote:
> Adhemerval Zanella wrote:

>> Changes from previous version:

> 

>>    - Remove padsize check, requested size after aligning and padding

>>      are checked against PTRDIFF_MAX.  This does not guarantee that

>>      sysmalloc will eventually issue an request larger than PTRDIFF_MAX.

>>      One option to enforce it is to either add the PTRDIFF_MAX check on

>>      mmap or create an internal mmap implementation to enforce it.

> 

> Sorry, I'm having trouble parsing that, starting with the word "guarantee"; it sounds like you're saying sysmalloc must issue a request larger than PTRDIFF_MAX, which surely is the opposite of what is intended.


As pointed out by Wilco, there is no difference between limiting the size 
to PTRDIFF_MAX or PTRDIFF_MAX-C for C < pagesize() since sysmalloc will
align up to PTRDIFF_MAX + 1 based on pagesize and this will be larger than 
PTRDIFF_MAX.

That's why to actually to enforce all system allocation are up to PTRDIFF_MAX
we need to either add extra checks on sysmalloc and on mmap (either on
default implementation or by an internal alias only).

> 

> More important, I thought that the idea was to allow malloc (s) if s <= PTRDIFF_MAX, even if s > PTRDIFF_MAX - DELTA for some nonzero but small DELTA. See <https://www.sourceware.org/ml/libc-alpha/2018-12/msg00908.html>, where I don't understand the point of your reply <https://www.sourceware.org/ml/libc-alpha/2018-12/msg00953.html>, as the concern about the compiler is the compiler's assumption about the object that malloc returns, not about how glibc behaves internally (I am assuming that glibc is careful to never subtract pointers whose integer representations differ by more than PTRDIFF_MAX).


That's still the idea, this comment is just a remark that I removed the
exactly check to avoid it and allows this kind of allocation you
described. My question was how should we proceed about this specific
allocation pattern, where the internal allocation will ended up issuing
a value larger than PTRDIFF_MAX. Right now there is no enforcing on
sysmalloc to handle it.

> 

>> +/* Check if REQ overflows when padded and aligned and if the resulting value

>> +   is less than PTRDIFF_T.  Returns TRUE and the requested size or MINSIZE in

>> +   case the value is less than MINSIZE on SZ or false if any of the previous

>> +   check fail.  */

>> +static inline bool

>> +checked_request2size (size_t req, size_t *sz) __nonnull (1)

>> +{

>> +  size_t ret;

>> +  if (__glibc_unlikely (__builtin_add_overflow (req,

>> +                        SIZE_SZ + MALLOC_ALIGN_MASK,

>> +                        &ret)))

>> +    return false;

>> +

>> +  ret = ret < MINSIZE ? MINSIZE : ret & ~MALLOC_ALIGN_MASK;

>> +  if (__glibc_unlikely (ret > PTRDIFF_MAX))

>> +    return false;

>> +

>> +  *sz = ret;

>> +  return true;

>> +}

> 

> This function typically does 3 comparisons. Since we're assuming PTRDIFF_MAX <= SIZE_MAX / 2, we can shrink it to at most 2 comparisons. Something like the following, say, where on x86-64 the typical case is one conditional branch and one conditional move.

> 

> static inline bool

> checked_request2size (size_t req, size_t *sz)

> {

>   if (__glibc_likely (req <= PTRDIFF_MAX))

>     {

>       int min_nontiny_size = SIZE_SZ < MINSIZE ? MINSIZE - SIZE_SZ : 0;

>       size_t areq = MAX (min_nontiny_size, req);

>       *sz = (areq + SIZE_SZ + MALLOC_ALIGN_MASK) & ~MALLOC_ALIGN_MASK;

>       return true;

>     }

>   return false;

> }


I think there is no need redo the request2size logic, I changed to

static inline bool
checked_request2size (size_t req, size_t *sz) __nonnull (1)
{
  if (__glibc_unlikely (req > PTRDIFF_MAX))
    return false;
  *sz = request2size (req);
  return true;
}

I also fixed an issue with realloc mcheck hook that triggered on tst-mcheck. 
Updated patch below.

---

[PATCH] malloc: make malloc fail with requests larger than PTRDIFF_MAX

As discussed previously on libc-alpha [1], this patch follows up the idea
and add both the __attribute_alloc_size__ on malloc functions (malloc,
calloc, realloc, reallocarray, valloc, pvalloc, memaling, and
posix_memalign) and limit maximum requested allocation size to up
PTRDIFF_MAX (taking in consideration internal padding and alignment).

This aligns glibc with gcc expected size defined by default warning
-Walloc-size-larger-than value which warns for allocation larger than
PTRDIFF_MAX.  It also aligns with gcc expectation regarding libc and
expected size, such as described in PR#67999 [2] and previously discussed
ISO C11 issues [3] on libc-alpha.

From the RFC thread [4] and previous discussion, it seems that consensus
is only to limit such requested size for malloc functions, not system
allocation one (mmap, sbrk, etc.).

The implementation changes checked_request2size to check for both overflow
and maximum object size up to PTRDIFF_MAX. No additional checks are done
on sysmalloc, so it can still issues mmap with values larger than
PTRDIFF_T depending of requested size.

Checked on x86_64-linux-gnu and i686-linux-gnu.

	[BZ #23741]
	* malloc/hooks.c (malloc_check, realloc_check): Use
	__builtin_add_overflow on overflow check and adapt to
	checked_request2size change.
	* malloc/malloc.c (__libc_malloc, __libc_realloc, _mid_memalign,
	__libc_pvalloc, __libc_calloc, _int_memalign): Limit maximum
	allocation size to PTRDIFF_MAX.
	(REQUEST_OUT_OF_RANGE): Remove macro.
	(checked_request2size): Change to inline function, use
	__builtin_add_overflow for overflow check and limit maximum requested
	size to PTRDIFF_MAX.
	(__libc_malloc, __libc_realloc, _int_malloc, _int_memalign): Limit
	maximum allocation size to PTRDIFF_MAX.
	(_mid_memalign): Rely on _int_memalign call for overflow check.
	(__libc_pvalloc): Use __builtin_add_overflow on overflow check.
	(__libc_calloc): Use __builtin_mul_overflow for overflow check and
	limit maximum requested size to PTRDIFF_MAX.
	* malloc/malloc.h (malloc, calloc, realloc, reallocarray, memalign,
	valloc, pvalloc): Add __attribute_alloc_size__.
	* stdlib/stdlib.h (malloc, realloc, reallocarray, valloc,
	posix_memalign): Likewise.
	* malloc/tst-malloc-too-large.c (do_test): Add check for allocation
	larger than PTRDIFF_MAX.
	* malloc/tst-memalign.c (do_test): Disable -Walloc-size-larger-than=
	around tests of malloc with negative sizes.
	* malloc/tst-posix_memalign.c (do_test): Likewise.
	* malloc/tst-pvalloc.c (do_test): Likewise.
	* malloc/tst-valloc.c (do_test): Likewise.
	* malloc/tst-reallocarray.c (do_test): Replace call to reallocarray
	with resulting size allocation larger than PTRDIFF_MAX with
	reallocarray_nowarn.
	(reallocarray_nowarn): New function.
	* NEWS: Mention the malloc function semantic change.

[1] https://sourceware.org/ml/libc-alpha/2018-11/msg00223.html
[2] https://gcc.gnu.org/bugzilla//show_bug.cgi?id=67999
[3] https://sourceware.org/ml/libc-alpha/2011-12/msg00066.html
[4] https://sourceware.org/ml/libc-alpha/2018-11/msg00224.html
---
 ChangeLog                     |  34 ++++++++++
 NEWS                          |   6 ++
 malloc/hooks.c                |  17 ++---
 malloc/malloc.c               | 113 +++++++++++++++-------------------
 malloc/malloc.h               |  17 ++---
 malloc/tst-malloc-too-large.c |  10 +++
 malloc/tst-memalign.c         |  10 +++
 malloc/tst-posix_memalign.c   |  10 +++
 malloc/tst-pvalloc.c          |  10 +++
 malloc/tst-reallocarray.c     |  27 ++++++--
 malloc/tst-valloc.c           |  10 +++
 stdlib/stdlib.h               |  13 ++--
 12 files changed, 189 insertions(+), 88 deletions(-)

diff --git a/NEWS b/NEWS
index cc20102fda..e64ba80c1e 100644
--- a/NEWS
+++ b/NEWS
@@ -85,6 +85,12 @@ Deprecated and removed features, and other changes affecting compatibility:
   as all functions that call vscanf, vfscanf, or vsscanf are annotated with
   __attribute__ ((format (scanf, ...))).
 
+* Memory allocation functions malloc, calloc, realloc, reallocarray, valloc,
+  pvalloc, memalign, and posix_memalign fail now with total object size
+  larger than PTRDIFF_MAX.  This is to avoid potential undefined behavior with
+  pointer subtraction within the allocated object, where results might
+  overflow the ptrdiff_t type.
+
 Changes to build and runtime requirements:
 
 * Python 3.4 or later is required to build the GNU C Library.
diff --git a/malloc/hooks.c b/malloc/hooks.c
index 46789736f3..b7a453f6d2 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -226,8 +226,9 @@ static void *
 malloc_check (size_t sz, const void *caller)
 {
   void *victim;
+  size_t nb;
 
-  if (sz + 1 == 0)
+  if (__builtin_add_overflow (sz, 1, &nb))
     {
       __set_errno (ENOMEM);
       return NULL;
@@ -235,7 +236,7 @@ malloc_check (size_t sz, const void *caller)
 
   __libc_lock_lock (main_arena.mutex);
   top_check ();
-  victim = _int_malloc (&main_arena, sz + 1);
+  victim = _int_malloc (&main_arena, nb);
   __libc_lock_unlock (main_arena.mutex);
   return mem2mem_check (victim, sz);
 }
@@ -268,8 +269,9 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
   INTERNAL_SIZE_T nb;
   void *newmem = 0;
   unsigned char *magic_p;
+  size_t rb;
 
-  if (bytes + 1 == 0)
+  if (__builtin_add_overflow (bytes, 1, &rb))
     {
       __set_errno (ENOMEM);
       return NULL;
@@ -289,7 +291,9 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
     malloc_printerr ("realloc(): invalid pointer");
   const INTERNAL_SIZE_T oldsize = chunksize (oldp);
 
-  checked_request2size (bytes + 1, nb);
+  if (!checked_request2size (rb, &nb))
+    goto invert;
+
   __libc_lock_lock (main_arena.mutex);
 
   if (chunk_is_mmapped (oldp))
@@ -308,7 +312,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
           {
             /* Must alloc, copy, free. */
 	    top_check ();
-	    newmem = _int_malloc (&main_arena, bytes + 1);
+	    newmem = _int_malloc (&main_arena, rb);
             if (newmem)
               {
                 memcpy (newmem, oldmem, oldsize - 2 * SIZE_SZ);
@@ -320,8 +324,6 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
   else
     {
       top_check ();
-      INTERNAL_SIZE_T nb;
-      checked_request2size (bytes + 1, nb);
       newmem = _int_realloc (&main_arena, oldp, oldsize, nb);
     }
 
@@ -334,6 +336,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
   /* mem2chunk_check changed the magic byte in the old chunk.
      If newmem is NULL, then the old chunk will still be used though,
      so we need to invert that change here.  */
+invert:
   if (newmem == NULL)
     *magic_p ^= 0xFF;
   DIAG_POP_NEEDS_COMMENT;
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 1908956ed1..5214444b70 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1187,17 +1187,6 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   ((uintptr_t)(MALLOC_ALIGNMENT == 2 * SIZE_SZ ? (p) : chunk2mem (p)) \
    & MALLOC_ALIGN_MASK)
 
-
-/*
-   Check if a request is so large that it would wrap around zero when
-   padded and aligned. To simplify some other code, the bound is made
-   low enough so that adding MINSIZE will also not wrap around zero.
- */
-
-#define REQUEST_OUT_OF_RANGE(req)                                 \
-  ((unsigned long) (req) >=						      \
-   (unsigned long) (INTERNAL_SIZE_T) (-2 * MINSIZE))
-
 /* pad request bytes into a usable size -- internal version */
 
 #define request2size(req)                                         \
@@ -1205,21 +1194,19 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    MINSIZE :                                                      \
    ((req) + SIZE_SZ + MALLOC_ALIGN_MASK) & ~MALLOC_ALIGN_MASK)
 
-/* Same, except also perform an argument and result check.  First, we check
-   that the padding done by request2size didn't result in an integer
-   overflow.  Then we check (using REQUEST_OUT_OF_RANGE) that the resulting
-   size isn't so large that a later alignment would lead to another integer
-   overflow.  */
-#define checked_request2size(req, sz) \
-({				    \
-  (sz) = request2size (req);	    \
-  if (((sz) < (req))		    \
-      || REQUEST_OUT_OF_RANGE (sz)) \
-    {				    \
-      __set_errno (ENOMEM);	    \
-      return 0;			    \
-    }				    \
-})
+/* Check if REQ overflows when padded and aligned and if the resulting value
+   is less than PTRDIFF_T.  Returns TRUE and the requested size or MINSIZE in
+   case the value is less than MINSIZE on SZ or false if any of the previous
+   check fail.  */
+static inline bool
+checked_request2size (size_t req, size_t *sz) __nonnull (1)
+{
+  if (__glibc_unlikely (req > PTRDIFF_MAX))
+    return false;
+  *sz = request2size (req);
+  return true;
+}
+
 
 /*
    --------------- Physical chunk operations ---------------
@@ -3109,6 +3096,9 @@ __libc_malloc (size_t bytes)
   mstate ar_ptr;
   void *victim;
 
+  _Static_assert (PTRDIFF_MAX <= SIZE_MAX / 2,
+                  "PTRDIFF_MAX is not more than half of SIZE_MAX");
+
   void *(*hook) (size_t, const void *)
     = atomic_forced_read (__malloc_hook);
   if (__builtin_expect (hook != NULL, 0))
@@ -3116,7 +3106,11 @@ __libc_malloc (size_t bytes)
 #if USE_TCACHE
   /* int_free also calls request2size, be careful to not pad twice.  */
   size_t tbytes;
-  checked_request2size (bytes, tbytes);
+  if (!checked_request2size (bytes, &tbytes))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
   size_t tc_idx = csize2tidx (tbytes);
 
   MAYBE_INIT_TCACHE ();
@@ -3253,7 +3247,11 @@ __libc_realloc (void *oldmem, size_t bytes)
       && !DUMPED_MAIN_ARENA_CHUNK (oldp))
       malloc_printerr ("realloc(): invalid pointer");
 
-  checked_request2size (bytes, nb);
+  if (!checked_request2size (bytes, &nb))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
 
   if (chunk_is_mmapped (oldp))
     {
@@ -3363,13 +3361,6 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
       return 0;
     }
 
-  /* Check for overflow.  */
-  if (bytes > SIZE_MAX - alignment - MINSIZE)
-    {
-      __set_errno (ENOMEM);
-      return 0;
-    }
-
 
   /* Make sure alignment is power of 2.  */
   if (!powerof2 (alignment))
@@ -3429,14 +3420,16 @@ __libc_pvalloc (size_t bytes)
 
   void *address = RETURN_ADDRESS (0);
   size_t pagesize = GLRO (dl_pagesize);
-  size_t rounded_bytes = ALIGN_UP (bytes, pagesize);
-
-  /* Check for overflow.  */
-  if (bytes > SIZE_MAX - 2 * pagesize - MINSIZE)
+  size_t rounded_bytes;
+  /* ALIGN_UP with overflow check.  */
+  if (__glibc_unlikely (__builtin_add_overflow (bytes,
+						pagesize - 1,
+						&rounded_bytes)))
     {
       __set_errno (ENOMEM);
       return 0;
     }
+  rounded_bytes = rounded_bytes & -(pagesize - 1);
 
   return _mid_memalign (pagesize, rounded_bytes, address);
 }
@@ -3446,30 +3439,24 @@ __libc_calloc (size_t n, size_t elem_size)
 {
   mstate av;
   mchunkptr oldtop, p;
-  INTERNAL_SIZE_T bytes, sz, csz, oldtopsize;
+  INTERNAL_SIZE_T sz, csz, oldtopsize;
   void *mem;
   unsigned long clearsize;
   unsigned long nclears;
   INTERNAL_SIZE_T *d;
+  ptrdiff_t bytes;
 
-  /* size_t is unsigned so the behavior on overflow is defined.  */
-  bytes = n * elem_size;
-#define HALF_INTERNAL_SIZE_T \
-  (((INTERNAL_SIZE_T) 1) << (8 * sizeof (INTERNAL_SIZE_T) / 2))
-  if (__builtin_expect ((n | elem_size) >= HALF_INTERNAL_SIZE_T, 0))
+  if (__glibc_unlikely (__builtin_mul_overflow (n, elem_size, &bytes)))
     {
-      if (elem_size != 0 && bytes / elem_size != n)
-        {
-          __set_errno (ENOMEM);
-          return 0;
-        }
+       __set_errno (ENOMEM);
+       return NULL;
     }
+  sz = bytes;
 
   void *(*hook) (size_t, const void *) =
     atomic_forced_read (__malloc_hook);
   if (__builtin_expect (hook != NULL, 0))
     {
-      sz = bytes;
       mem = (*hook)(sz, RETURN_ADDRESS (0));
       if (mem == 0)
         return 0;
@@ -3477,8 +3464,6 @@ __libc_calloc (size_t n, size_t elem_size)
       return memset (mem, 0, sz);
     }
 
-  sz = bytes;
-
   MAYBE_INIT_TCACHE ();
 
   if (SINGLE_THREAD_P)
@@ -3625,12 +3610,16 @@ _int_malloc (mstate av, size_t bytes)
      Convert request size to internal form by adding SIZE_SZ bytes
      overhead plus possibly more to obtain necessary alignment and/or
      to obtain a size of at least MINSIZE, the smallest allocatable
-     size. Also, checked_request2size traps (returning 0) request sizes
+     size. Also, checked_request2size returns false for request sizes
      that are so large that they wrap around zero when padded and
      aligned.
    */
 
-  checked_request2size (bytes, nb);
+  if (!checked_request2size (bytes, &nb))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
 
   /* There are no usable arenas.  Fall back to sysmalloc to get a chunk from
      mmap.  */
@@ -4702,21 +4691,17 @@ _int_memalign (mstate av, size_t alignment, size_t bytes)
 
 
 
-  checked_request2size (bytes, nb);
+  if (!checked_request2size (bytes, &nb))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
 
   /*
      Strategy: find a spot within that chunk that meets the alignment
      request, and then possibly free the leading and trailing space.
    */
 
-
-  /* Check for overflow.  */
-  if (nb > SIZE_MAX - alignment - MINSIZE)
-    {
-      __set_errno (ENOMEM);
-      return 0;
-    }
-
   /* Call malloc with worst case padding to hit alignment. */
 
   m = (char *) (_int_malloc (av, nb + alignment + MINSIZE));
diff --git a/malloc/malloc.h b/malloc/malloc.h
index 4428edc06f..523f1b1af5 100644
--- a/malloc/malloc.h
+++ b/malloc/malloc.h
@@ -35,11 +35,12 @@
 __BEGIN_DECLS
 
 /* Allocate SIZE bytes of memory.  */
-extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;
+extern void *malloc (size_t __size) __THROW __attribute_malloc__
+     __attribute_alloc_size__ ((1)) __wur;
 
 /* Allocate NMEMB elements of SIZE bytes each, all initialized to 0.  */
 extern void *calloc (size_t __nmemb, size_t __size)
-__THROW __attribute_malloc__ __wur;
+__THROW __attribute_malloc__ __attribute_alloc_size__ ((1, 2)) __wur;
 
 /* Re-allocate the previously allocated block in __ptr, making the new
    block SIZE bytes long.  */
@@ -47,7 +48,7 @@ __THROW __attribute_malloc__ __wur;
    the same pointer that was passed to it, aliasing needs to be allowed
    between objects pointed by the old and new pointers.  */
 extern void *realloc (void *__ptr, size_t __size)
-__THROW __attribute_warn_unused_result__;
+__THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2));
 
 /* Re-allocate the previously allocated block in PTR, making the new
    block large enough for NMEMB elements of SIZE bytes each.  */
@@ -55,21 +56,23 @@ __THROW __attribute_warn_unused_result__;
    the same pointer that was passed to it, aliasing needs to be allowed
    between objects pointed by the old and new pointers.  */
 extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)
-__THROW __attribute_warn_unused_result__;
+__THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2, 3));
 
 /* Free a block allocated by `malloc', `realloc' or `calloc'.  */
 extern void free (void *__ptr) __THROW;
 
 /* Allocate SIZE bytes allocated to ALIGNMENT bytes.  */
 extern void *memalign (size_t __alignment, size_t __size)
-__THROW __attribute_malloc__ __wur;
+__THROW __attribute_malloc__ __attribute_alloc_size__ ((2)) __wur;
 
 /* Allocate SIZE bytes on a page boundary.  */
-extern void *valloc (size_t __size) __THROW __attribute_malloc__ __wur;
+extern void *valloc (size_t __size) __THROW __attribute_malloc__
+     __attribute_alloc_size__ ((1)) __wur;
 
 /* Equivalent to valloc(minimum-page-that-holds(n)), that is, round up
    __size to nearest pagesize. */
-extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ __wur;
+extern void *pvalloc (size_t __size) __THROW __attribute_malloc__
+     __attribute_alloc_size__ ((1)) __wur;
 
 /* Underlying allocation function; successive calls should return
    contiguous pieces of memory.  */
diff --git a/malloc/tst-malloc-too-large.c b/malloc/tst-malloc-too-large.c
index 15e25f558e..b652cb1ef4 100644
--- a/malloc/tst-malloc-too-large.c
+++ b/malloc/tst-malloc-too-large.c
@@ -226,6 +226,16 @@ do_test (void)
       test_large_aligned_allocations (SIZE_MAX - i);
     }
 
+  /* Allocation larger than PTRDIFF_MAX does play well with C standard,
+     since pointer subtraction within the object might overflow ptrdiff_t
+     resulting in undefined behavior.  To prevent it malloc function fail
+     for such allocations.  */
+  for (size_t i = 0; i <= FOURTEEN_ON_BITS; i++)
+    {
+      test_large_allocations (PTRDIFF_MAX + i);
+      test_large_aligned_allocations (PTRDIFF_MAX + i);
+    }
+
 #if __WORDSIZE >= 64
   /* On 64-bit targets, we need to test a much wider range of too-large
      sizes, so we test at intervals of (1 << 50) that allocation sizes
diff --git a/malloc/tst-memalign.c b/malloc/tst-memalign.c
index a6a9140a3d..e7997518cb 100644
--- a/malloc/tst-memalign.c
+++ b/malloc/tst-memalign.c
@@ -21,6 +21,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <libc-diag.h>
 
 static int errors = 0;
 
@@ -41,9 +42,18 @@ do_test (void)
 
   errno = 0;
 
+  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about too-large allocations; here we want to test
+     that they fail.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");
+#endif
   /* An attempt to allocate a huge value should return NULL and set
      errno to ENOMEM.  */
   p = memalign (sizeof (void *), -1);
+#if __GNUC_PREREQ (7, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
 
   save = errno;
 
diff --git a/malloc/tst-posix_memalign.c b/malloc/tst-posix_memalign.c
index bcf1592679..fc46955c51 100644
--- a/malloc/tst-posix_memalign.c
+++ b/malloc/tst-posix_memalign.c
@@ -21,6 +21,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <libc-diag.h>
 
 static int errors = 0;
 
@@ -41,9 +42,18 @@ do_test (void)
 
   p = NULL;
 
+  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about too-large allocations; here we want to test
+     that they fail.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");
+#endif
   /* An attempt to allocate a huge value should return ENOMEM and
      p should remain NULL.  */
   ret = posix_memalign (&p, sizeof (void *), -1);
+#if __GNUC_PREREQ (7, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
 
   if (ret != ENOMEM)
     merror ("posix_memalign (&p, sizeof (void *), -1) succeeded.");
diff --git a/malloc/tst-pvalloc.c b/malloc/tst-pvalloc.c
index 0b4baba67a..c0356c0c71 100644
--- a/malloc/tst-pvalloc.c
+++ b/malloc/tst-pvalloc.c
@@ -21,6 +21,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <libc-diag.h>
 
 static int errors = 0;
 
@@ -41,9 +42,18 @@ do_test (void)
 
   errno = 0;
 
+  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about too-large allocations; here we want to test
+     that they fail.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");
+#endif
   /* An attempt to allocate a huge value should return NULL and set
      errno to ENOMEM.  */
   p = pvalloc (-1);
+#if __GNUC_PREREQ (7, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
 
   save = errno;
 
diff --git a/malloc/tst-reallocarray.c b/malloc/tst-reallocarray.c
index 9a71ec7f45..fd50d4e36b 100644
--- a/malloc/tst-reallocarray.c
+++ b/malloc/tst-reallocarray.c
@@ -20,6 +20,23 @@
 #include <malloc.h>
 #include <string.h>
 #include <support/check.h>
+#include <libc-diag.h>
+
+static void *
+reallocarray_nowarn (void *ptr, size_t nmemb, size_t size)
+{
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about too-large allocations; here we want to test
+     that they fail.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");
+#endif
+  void *ret = reallocarray (ptr, nmemb, size);
+#if __GNUC_PREREQ (7, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
+  return ret;
+}
+
 
 static int
 do_test (void)
@@ -34,24 +51,24 @@ do_test (void)
 
   /* Test overflow detection.  */
   errno = 0;
-  ptr = reallocarray (NULL, max, 2);
+  ptr = reallocarray_nowarn (NULL, max, 2);
   TEST_VERIFY (!ptr);
   TEST_VERIFY (errno == ENOMEM);
 
   errno = 0;
-  ptr = reallocarray (NULL, 2, max);
+  ptr = reallocarray_nowarn (NULL, 2, max);
   TEST_VERIFY (!ptr);
   TEST_VERIFY (errno == ENOMEM);
 
   a = 65537;
   b = max/65537 + 1;
   errno = 0;
-  ptr = reallocarray (NULL, a, b);
+  ptr = reallocarray_nowarn (NULL, a, b);
   TEST_VERIFY (!ptr);
   TEST_VERIFY (errno == ENOMEM);
 
   errno = 0;
-  ptr = reallocarray (NULL, b, a);
+  ptr = reallocarray_nowarn (NULL, b, a);
   TEST_VERIFY (!ptr);
   TEST_VERIFY (errno == ENOMEM);
 
@@ -97,7 +114,7 @@ do_test (void)
 
   /* Overflow should leave buffer untouched.  */
   errno = 0;
-  ptr2 = reallocarray (ptr, 2, ~(size_t)0);
+  ptr2 = reallocarray_nowarn (ptr, 2, ~(size_t)0);
   TEST_VERIFY (!ptr2);
   TEST_VERIFY (errno == ENOMEM);
 
diff --git a/malloc/tst-valloc.c b/malloc/tst-valloc.c
index a2e27c1e91..1a1a2d2500 100644
--- a/malloc/tst-valloc.c
+++ b/malloc/tst-valloc.c
@@ -21,6 +21,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <libc-diag.h>
 
 static int errors = 0;
 
@@ -41,9 +42,18 @@ do_test (void)
 
   errno = 0;
 
+  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about too-large allocations; here we want to test
+     that they fail.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");
+#endif
   /* An attempt to allocate a huge value should return NULL and set
      errno to ENOMEM.  */
   p = valloc (-1);
+#if __GNUC_PREREQ (7, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
 
   save = errno;
 
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index 15debe1e31..a9fd989d39 100644
--- a/stdlib/stdlib.h
+++ b/stdlib/stdlib.h
@@ -536,10 +536,11 @@ extern int lcong48_r (unsigned short int __param[7],
 #endif	/* Use misc or X/Open.  */
 
 /* Allocate SIZE bytes of memory.  */
-extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;
+extern void *malloc (size_t __size) __THROW __attribute_malloc__
+     __attribute_alloc_size__ ((1)) __wur;
 /* Allocate NMEMB elements of SIZE bytes each, all initialized to 0.  */
 extern void *calloc (size_t __nmemb, size_t __size)
-     __THROW __attribute_malloc__ __wur;
+     __THROW __attribute_malloc__ __attribute_alloc_size__ ((1, 2)) __wur;
 
 /* Re-allocate the previously allocated block
    in PTR, making the new block SIZE bytes long.  */
@@ -547,7 +548,7 @@ extern void *calloc (size_t __nmemb, size_t __size)
    the same pointer that was passed to it, aliasing needs to be allowed
    between objects pointed by the old and new pointers.  */
 extern void *realloc (void *__ptr, size_t __size)
-     __THROW __attribute_warn_unused_result__;
+     __THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2));
 
 #ifdef __USE_MISC
 /* Re-allocate the previously allocated block in PTR, making the new
@@ -556,7 +557,8 @@ extern void *realloc (void *__ptr, size_t __size)
    the same pointer that was passed to it, aliasing needs to be allowed
    between objects pointed by the old and new pointers.  */
 extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)
-     __THROW __attribute_warn_unused_result__;
+     __THROW __attribute_warn_unused_result__
+     __attribute_alloc_size__ ((2, 3));
 #endif
 
 /* Free a block allocated by `malloc', `realloc' or `calloc'.  */
@@ -569,7 +571,8 @@ extern void free (void *__ptr) __THROW;
 #if (defined __USE_XOPEN_EXTENDED && !defined __USE_XOPEN2K) \
     || defined __USE_MISC
 /* Allocate SIZE bytes on a page boundary.  The storage cannot be freed.  */
-extern void *valloc (size_t __size) __THROW __attribute_malloc__ __wur;
+extern void *valloc (size_t __size) __THROW __attribute_malloc__
+     __attribute_alloc_size__ ((1)) __wur;
 #endif
 
 #ifdef __USE_XOPEN2K
Adhemerval Zanella Netto Feb. 12, 2019, 11:21 a.m. UTC | #3
Hi D.J and Paul, still ok to push upstream?

On 15/01/2019 11:19, Adhemerval Zanella wrote:
> 

> 

> On 30/12/2018 23:16, Paul Eggert wrote:

>> Adhemerval Zanella wrote:

>>> Changes from previous version:

>>

>>>    - Remove padsize check, requested size after aligning and padding

>>>      are checked against PTRDIFF_MAX.  This does not guarantee that

>>>      sysmalloc will eventually issue an request larger than PTRDIFF_MAX.

>>>      One option to enforce it is to either add the PTRDIFF_MAX check on

>>>      mmap or create an internal mmap implementation to enforce it.

>>

>> Sorry, I'm having trouble parsing that, starting with the word "guarantee"; it sounds like you're saying sysmalloc must issue a request larger than PTRDIFF_MAX, which surely is the opposite of what is intended.

> 

> As pointed out by Wilco, there is no difference between limiting the size 

> to PTRDIFF_MAX or PTRDIFF_MAX-C for C < pagesize() since sysmalloc will

> align up to PTRDIFF_MAX + 1 based on pagesize and this will be larger than 

> PTRDIFF_MAX.

> 

> That's why to actually to enforce all system allocation are up to PTRDIFF_MAX

> we need to either add extra checks on sysmalloc and on mmap (either on

> default implementation or by an internal alias only).

> 

>>

>> More important, I thought that the idea was to allow malloc (s) if s <= PTRDIFF_MAX, even if s > PTRDIFF_MAX - DELTA for some nonzero but small DELTA. See <https://www.sourceware.org/ml/libc-alpha/2018-12/msg00908.html>, where I don't understand the point of your reply <https://www.sourceware.org/ml/libc-alpha/2018-12/msg00953.html>, as the concern about the compiler is the compiler's assumption about the object that malloc returns, not about how glibc behaves internally (I am assuming that glibc is careful to never subtract pointers whose integer representations differ by more than PTRDIFF_MAX).

> 

> That's still the idea, this comment is just a remark that I removed the

> exactly check to avoid it and allows this kind of allocation you

> described. My question was how should we proceed about this specific

> allocation pattern, where the internal allocation will ended up issuing

> a value larger than PTRDIFF_MAX. Right now there is no enforcing on

> sysmalloc to handle it.

> 

>>

>>> +/* Check if REQ overflows when padded and aligned and if the resulting value

>>> +   is less than PTRDIFF_T.  Returns TRUE and the requested size or MINSIZE in

>>> +   case the value is less than MINSIZE on SZ or false if any of the previous

>>> +   check fail.  */

>>> +static inline bool

>>> +checked_request2size (size_t req, size_t *sz) __nonnull (1)

>>> +{

>>> +  size_t ret;

>>> +  if (__glibc_unlikely (__builtin_add_overflow (req,

>>> +                        SIZE_SZ + MALLOC_ALIGN_MASK,

>>> +                        &ret)))

>>> +    return false;

>>> +

>>> +  ret = ret < MINSIZE ? MINSIZE : ret & ~MALLOC_ALIGN_MASK;

>>> +  if (__glibc_unlikely (ret > PTRDIFF_MAX))

>>> +    return false;

>>> +

>>> +  *sz = ret;

>>> +  return true;

>>> +}

>>

>> This function typically does 3 comparisons. Since we're assuming PTRDIFF_MAX <= SIZE_MAX / 2, we can shrink it to at most 2 comparisons. Something like the following, say, where on x86-64 the typical case is one conditional branch and one conditional move.

>>

>> static inline bool

>> checked_request2size (size_t req, size_t *sz)

>> {

>>   if (__glibc_likely (req <= PTRDIFF_MAX))

>>     {

>>       int min_nontiny_size = SIZE_SZ < MINSIZE ? MINSIZE - SIZE_SZ : 0;

>>       size_t areq = MAX (min_nontiny_size, req);

>>       *sz = (areq + SIZE_SZ + MALLOC_ALIGN_MASK) & ~MALLOC_ALIGN_MASK;

>>       return true;

>>     }

>>   return false;

>> }

> 

> I think there is no need redo the request2size logic, I changed to

> 

> static inline bool

> checked_request2size (size_t req, size_t *sz) __nonnull (1)

> {

>   if (__glibc_unlikely (req > PTRDIFF_MAX))

>     return false;

>   *sz = request2size (req);

>   return true;

> }

> 

> I also fixed an issue with realloc mcheck hook that triggered on tst-mcheck. 

> Updated patch below.

> 

> ---

> 

> [PATCH] malloc: make malloc fail with requests larger than PTRDIFF_MAX

> 

> As discussed previously on libc-alpha [1], this patch follows up the idea

> and add both the __attribute_alloc_size__ on malloc functions (malloc,

> calloc, realloc, reallocarray, valloc, pvalloc, memaling, and

> posix_memalign) and limit maximum requested allocation size to up

> PTRDIFF_MAX (taking in consideration internal padding and alignment).

> 

> This aligns glibc with gcc expected size defined by default warning

> -Walloc-size-larger-than value which warns for allocation larger than

> PTRDIFF_MAX.  It also aligns with gcc expectation regarding libc and

> expected size, such as described in PR#67999 [2] and previously discussed

> ISO C11 issues [3] on libc-alpha.

> 

> From the RFC thread [4] and previous discussion, it seems that consensus

> is only to limit such requested size for malloc functions, not system

> allocation one (mmap, sbrk, etc.).

> 

> The implementation changes checked_request2size to check for both overflow

> and maximum object size up to PTRDIFF_MAX. No additional checks are done

> on sysmalloc, so it can still issues mmap with values larger than

> PTRDIFF_T depending of requested size.

> 

> Checked on x86_64-linux-gnu and i686-linux-gnu.

> 

> 	[BZ #23741]

> 	* malloc/hooks.c (malloc_check, realloc_check): Use

> 	__builtin_add_overflow on overflow check and adapt to

> 	checked_request2size change.

> 	* malloc/malloc.c (__libc_malloc, __libc_realloc, _mid_memalign,

> 	__libc_pvalloc, __libc_calloc, _int_memalign): Limit maximum

> 	allocation size to PTRDIFF_MAX.

> 	(REQUEST_OUT_OF_RANGE): Remove macro.

> 	(checked_request2size): Change to inline function, use

> 	__builtin_add_overflow for overflow check and limit maximum requested

> 	size to PTRDIFF_MAX.

> 	(__libc_malloc, __libc_realloc, _int_malloc, _int_memalign): Limit

> 	maximum allocation size to PTRDIFF_MAX.

> 	(_mid_memalign): Rely on _int_memalign call for overflow check.

> 	(__libc_pvalloc): Use __builtin_add_overflow on overflow check.

> 	(__libc_calloc): Use __builtin_mul_overflow for overflow check and

> 	limit maximum requested size to PTRDIFF_MAX.

> 	* malloc/malloc.h (malloc, calloc, realloc, reallocarray, memalign,

> 	valloc, pvalloc): Add __attribute_alloc_size__.

> 	* stdlib/stdlib.h (malloc, realloc, reallocarray, valloc,

> 	posix_memalign): Likewise.

> 	* malloc/tst-malloc-too-large.c (do_test): Add check for allocation

> 	larger than PTRDIFF_MAX.

> 	* malloc/tst-memalign.c (do_test): Disable -Walloc-size-larger-than=

> 	around tests of malloc with negative sizes.

> 	* malloc/tst-posix_memalign.c (do_test): Likewise.

> 	* malloc/tst-pvalloc.c (do_test): Likewise.

> 	* malloc/tst-valloc.c (do_test): Likewise.

> 	* malloc/tst-reallocarray.c (do_test): Replace call to reallocarray

> 	with resulting size allocation larger than PTRDIFF_MAX with

> 	reallocarray_nowarn.

> 	(reallocarray_nowarn): New function.

> 	* NEWS: Mention the malloc function semantic change.

> 

> [1] https://sourceware.org/ml/libc-alpha/2018-11/msg00223.html

> [2] https://gcc.gnu.org/bugzilla//show_bug.cgi?id=67999

> [3] https://sourceware.org/ml/libc-alpha/2011-12/msg00066.html

> [4] https://sourceware.org/ml/libc-alpha/2018-11/msg00224.html

> ---

>  ChangeLog                     |  34 ++++++++++

>  NEWS                          |   6 ++

>  malloc/hooks.c                |  17 ++---

>  malloc/malloc.c               | 113 +++++++++++++++-------------------

>  malloc/malloc.h               |  17 ++---

>  malloc/tst-malloc-too-large.c |  10 +++

>  malloc/tst-memalign.c         |  10 +++

>  malloc/tst-posix_memalign.c   |  10 +++

>  malloc/tst-pvalloc.c          |  10 +++

>  malloc/tst-reallocarray.c     |  27 ++++++--

>  malloc/tst-valloc.c           |  10 +++

>  stdlib/stdlib.h               |  13 ++--

>  12 files changed, 189 insertions(+), 88 deletions(-)

> 

> diff --git a/NEWS b/NEWS

> index cc20102fda..e64ba80c1e 100644

> --- a/NEWS

> +++ b/NEWS

> @@ -85,6 +85,12 @@ Deprecated and removed features, and other changes affecting compatibility:

>    as all functions that call vscanf, vfscanf, or vsscanf are annotated with

>    __attribute__ ((format (scanf, ...))).

>  

> +* Memory allocation functions malloc, calloc, realloc, reallocarray, valloc,

> +  pvalloc, memalign, and posix_memalign fail now with total object size

> +  larger than PTRDIFF_MAX.  This is to avoid potential undefined behavior with

> +  pointer subtraction within the allocated object, where results might

> +  overflow the ptrdiff_t type.

> +

>  Changes to build and runtime requirements:

>  

>  * Python 3.4 or later is required to build the GNU C Library.

> diff --git a/malloc/hooks.c b/malloc/hooks.c

> index 46789736f3..b7a453f6d2 100644

> --- a/malloc/hooks.c

> +++ b/malloc/hooks.c

> @@ -226,8 +226,9 @@ static void *

>  malloc_check (size_t sz, const void *caller)

>  {

>    void *victim;

> +  size_t nb;

>  

> -  if (sz + 1 == 0)

> +  if (__builtin_add_overflow (sz, 1, &nb))

>      {

>        __set_errno (ENOMEM);

>        return NULL;

> @@ -235,7 +236,7 @@ malloc_check (size_t sz, const void *caller)

>  

>    __libc_lock_lock (main_arena.mutex);

>    top_check ();

> -  victim = _int_malloc (&main_arena, sz + 1);

> +  victim = _int_malloc (&main_arena, nb);

>    __libc_lock_unlock (main_arena.mutex);

>    return mem2mem_check (victim, sz);

>  }

> @@ -268,8 +269,9 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)

>    INTERNAL_SIZE_T nb;

>    void *newmem = 0;

>    unsigned char *magic_p;

> +  size_t rb;

>  

> -  if (bytes + 1 == 0)

> +  if (__builtin_add_overflow (bytes, 1, &rb))

>      {

>        __set_errno (ENOMEM);

>        return NULL;

> @@ -289,7 +291,9 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)

>      malloc_printerr ("realloc(): invalid pointer");

>    const INTERNAL_SIZE_T oldsize = chunksize (oldp);

>  

> -  checked_request2size (bytes + 1, nb);

> +  if (!checked_request2size (rb, &nb))

> +    goto invert;

> +

>    __libc_lock_lock (main_arena.mutex);

>  

>    if (chunk_is_mmapped (oldp))

> @@ -308,7 +312,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)

>            {

>              /* Must alloc, copy, free. */

>  	    top_check ();

> -	    newmem = _int_malloc (&main_arena, bytes + 1);

> +	    newmem = _int_malloc (&main_arena, rb);

>              if (newmem)

>                {

>                  memcpy (newmem, oldmem, oldsize - 2 * SIZE_SZ);

> @@ -320,8 +324,6 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)

>    else

>      {

>        top_check ();

> -      INTERNAL_SIZE_T nb;

> -      checked_request2size (bytes + 1, nb);

>        newmem = _int_realloc (&main_arena, oldp, oldsize, nb);

>      }

>  

> @@ -334,6 +336,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)

>    /* mem2chunk_check changed the magic byte in the old chunk.

>       If newmem is NULL, then the old chunk will still be used though,

>       so we need to invert that change here.  */

> +invert:

>    if (newmem == NULL)

>      *magic_p ^= 0xFF;

>    DIAG_POP_NEEDS_COMMENT;

> diff --git a/malloc/malloc.c b/malloc/malloc.c

> index 1908956ed1..5214444b70 100644

> --- a/malloc/malloc.c

> +++ b/malloc/malloc.c

> @@ -1187,17 +1187,6 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

>    ((uintptr_t)(MALLOC_ALIGNMENT == 2 * SIZE_SZ ? (p) : chunk2mem (p)) \

>     & MALLOC_ALIGN_MASK)

>  

> -

> -/*

> -   Check if a request is so large that it would wrap around zero when

> -   padded and aligned. To simplify some other code, the bound is made

> -   low enough so that adding MINSIZE will also not wrap around zero.

> - */

> -

> -#define REQUEST_OUT_OF_RANGE(req)                                 \

> -  ((unsigned long) (req) >=						      \

> -   (unsigned long) (INTERNAL_SIZE_T) (-2 * MINSIZE))

> -

>  /* pad request bytes into a usable size -- internal version */

>  

>  #define request2size(req)                                         \

> @@ -1205,21 +1194,19 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

>     MINSIZE :                                                      \

>     ((req) + SIZE_SZ + MALLOC_ALIGN_MASK) & ~MALLOC_ALIGN_MASK)

>  

> -/* Same, except also perform an argument and result check.  First, we check

> -   that the padding done by request2size didn't result in an integer

> -   overflow.  Then we check (using REQUEST_OUT_OF_RANGE) that the resulting

> -   size isn't so large that a later alignment would lead to another integer

> -   overflow.  */

> -#define checked_request2size(req, sz) \

> -({				    \

> -  (sz) = request2size (req);	    \

> -  if (((sz) < (req))		    \

> -      || REQUEST_OUT_OF_RANGE (sz)) \

> -    {				    \

> -      __set_errno (ENOMEM);	    \

> -      return 0;			    \

> -    }				    \

> -})

> +/* Check if REQ overflows when padded and aligned and if the resulting value

> +   is less than PTRDIFF_T.  Returns TRUE and the requested size or MINSIZE in

> +   case the value is less than MINSIZE on SZ or false if any of the previous

> +   check fail.  */

> +static inline bool

> +checked_request2size (size_t req, size_t *sz) __nonnull (1)

> +{

> +  if (__glibc_unlikely (req > PTRDIFF_MAX))

> +    return false;

> +  *sz = request2size (req);

> +  return true;

> +}

> +

>  

>  /*

>     --------------- Physical chunk operations ---------------

> @@ -3109,6 +3096,9 @@ __libc_malloc (size_t bytes)

>    mstate ar_ptr;

>    void *victim;

>  

> +  _Static_assert (PTRDIFF_MAX <= SIZE_MAX / 2,

> +                  "PTRDIFF_MAX is not more than half of SIZE_MAX");

> +

>    void *(*hook) (size_t, const void *)

>      = atomic_forced_read (__malloc_hook);

>    if (__builtin_expect (hook != NULL, 0))

> @@ -3116,7 +3106,11 @@ __libc_malloc (size_t bytes)

>  #if USE_TCACHE

>    /* int_free also calls request2size, be careful to not pad twice.  */

>    size_t tbytes;

> -  checked_request2size (bytes, tbytes);

> +  if (!checked_request2size (bytes, &tbytes))

> +    {

> +      __set_errno (ENOMEM);

> +      return NULL;

> +    }

>    size_t tc_idx = csize2tidx (tbytes);

>  

>    MAYBE_INIT_TCACHE ();

> @@ -3253,7 +3247,11 @@ __libc_realloc (void *oldmem, size_t bytes)

>        && !DUMPED_MAIN_ARENA_CHUNK (oldp))

>        malloc_printerr ("realloc(): invalid pointer");

>  

> -  checked_request2size (bytes, nb);

> +  if (!checked_request2size (bytes, &nb))

> +    {

> +      __set_errno (ENOMEM);

> +      return NULL;

> +    }

>  

>    if (chunk_is_mmapped (oldp))

>      {

> @@ -3363,13 +3361,6 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)

>        return 0;

>      }

>  

> -  /* Check for overflow.  */

> -  if (bytes > SIZE_MAX - alignment - MINSIZE)

> -    {

> -      __set_errno (ENOMEM);

> -      return 0;

> -    }

> -

>  

>    /* Make sure alignment is power of 2.  */

>    if (!powerof2 (alignment))

> @@ -3429,14 +3420,16 @@ __libc_pvalloc (size_t bytes)

>  

>    void *address = RETURN_ADDRESS (0);

>    size_t pagesize = GLRO (dl_pagesize);

> -  size_t rounded_bytes = ALIGN_UP (bytes, pagesize);

> -

> -  /* Check for overflow.  */

> -  if (bytes > SIZE_MAX - 2 * pagesize - MINSIZE)

> +  size_t rounded_bytes;

> +  /* ALIGN_UP with overflow check.  */

> +  if (__glibc_unlikely (__builtin_add_overflow (bytes,

> +						pagesize - 1,

> +						&rounded_bytes)))

>      {

>        __set_errno (ENOMEM);

>        return 0;

>      }

> +  rounded_bytes = rounded_bytes & -(pagesize - 1);

>  

>    return _mid_memalign (pagesize, rounded_bytes, address);

>  }

> @@ -3446,30 +3439,24 @@ __libc_calloc (size_t n, size_t elem_size)

>  {

>    mstate av;

>    mchunkptr oldtop, p;

> -  INTERNAL_SIZE_T bytes, sz, csz, oldtopsize;

> +  INTERNAL_SIZE_T sz, csz, oldtopsize;

>    void *mem;

>    unsigned long clearsize;

>    unsigned long nclears;

>    INTERNAL_SIZE_T *d;

> +  ptrdiff_t bytes;

>  

> -  /* size_t is unsigned so the behavior on overflow is defined.  */

> -  bytes = n * elem_size;

> -#define HALF_INTERNAL_SIZE_T \

> -  (((INTERNAL_SIZE_T) 1) << (8 * sizeof (INTERNAL_SIZE_T) / 2))

> -  if (__builtin_expect ((n | elem_size) >= HALF_INTERNAL_SIZE_T, 0))

> +  if (__glibc_unlikely (__builtin_mul_overflow (n, elem_size, &bytes)))

>      {

> -      if (elem_size != 0 && bytes / elem_size != n)

> -        {

> -          __set_errno (ENOMEM);

> -          return 0;

> -        }

> +       __set_errno (ENOMEM);

> +       return NULL;

>      }

> +  sz = bytes;

>  

>    void *(*hook) (size_t, const void *) =

>      atomic_forced_read (__malloc_hook);

>    if (__builtin_expect (hook != NULL, 0))

>      {

> -      sz = bytes;

>        mem = (*hook)(sz, RETURN_ADDRESS (0));

>        if (mem == 0)

>          return 0;

> @@ -3477,8 +3464,6 @@ __libc_calloc (size_t n, size_t elem_size)

>        return memset (mem, 0, sz);

>      }

>  

> -  sz = bytes;

> -

>    MAYBE_INIT_TCACHE ();

>  

>    if (SINGLE_THREAD_P)

> @@ -3625,12 +3610,16 @@ _int_malloc (mstate av, size_t bytes)

>       Convert request size to internal form by adding SIZE_SZ bytes

>       overhead plus possibly more to obtain necessary alignment and/or

>       to obtain a size of at least MINSIZE, the smallest allocatable

> -     size. Also, checked_request2size traps (returning 0) request sizes

> +     size. Also, checked_request2size returns false for request sizes

>       that are so large that they wrap around zero when padded and

>       aligned.

>     */

>  

> -  checked_request2size (bytes, nb);

> +  if (!checked_request2size (bytes, &nb))

> +    {

> +      __set_errno (ENOMEM);

> +      return NULL;

> +    }

>  

>    /* There are no usable arenas.  Fall back to sysmalloc to get a chunk from

>       mmap.  */

> @@ -4702,21 +4691,17 @@ _int_memalign (mstate av, size_t alignment, size_t bytes)

>  

>  

>  

> -  checked_request2size (bytes, nb);

> +  if (!checked_request2size (bytes, &nb))

> +    {

> +      __set_errno (ENOMEM);

> +      return NULL;

> +    }

>  

>    /*

>       Strategy: find a spot within that chunk that meets the alignment

>       request, and then possibly free the leading and trailing space.

>     */

>  

> -

> -  /* Check for overflow.  */

> -  if (nb > SIZE_MAX - alignment - MINSIZE)

> -    {

> -      __set_errno (ENOMEM);

> -      return 0;

> -    }

> -

>    /* Call malloc with worst case padding to hit alignment. */

>  

>    m = (char *) (_int_malloc (av, nb + alignment + MINSIZE));

> diff --git a/malloc/malloc.h b/malloc/malloc.h

> index 4428edc06f..523f1b1af5 100644

> --- a/malloc/malloc.h

> +++ b/malloc/malloc.h

> @@ -35,11 +35,12 @@

>  __BEGIN_DECLS

>  

>  /* Allocate SIZE bytes of memory.  */

> -extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;

> +extern void *malloc (size_t __size) __THROW __attribute_malloc__

> +     __attribute_alloc_size__ ((1)) __wur;

>  

>  /* Allocate NMEMB elements of SIZE bytes each, all initialized to 0.  */

>  extern void *calloc (size_t __nmemb, size_t __size)

> -__THROW __attribute_malloc__ __wur;

> +__THROW __attribute_malloc__ __attribute_alloc_size__ ((1, 2)) __wur;

>  

>  /* Re-allocate the previously allocated block in __ptr, making the new

>     block SIZE bytes long.  */

> @@ -47,7 +48,7 @@ __THROW __attribute_malloc__ __wur;

>     the same pointer that was passed to it, aliasing needs to be allowed

>     between objects pointed by the old and new pointers.  */

>  extern void *realloc (void *__ptr, size_t __size)

> -__THROW __attribute_warn_unused_result__;

> +__THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2));

>  

>  /* Re-allocate the previously allocated block in PTR, making the new

>     block large enough for NMEMB elements of SIZE bytes each.  */

> @@ -55,21 +56,23 @@ __THROW __attribute_warn_unused_result__;

>     the same pointer that was passed to it, aliasing needs to be allowed

>     between objects pointed by the old and new pointers.  */

>  extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)

> -__THROW __attribute_warn_unused_result__;

> +__THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2, 3));

>  

>  /* Free a block allocated by `malloc', `realloc' or `calloc'.  */

>  extern void free (void *__ptr) __THROW;

>  

>  /* Allocate SIZE bytes allocated to ALIGNMENT bytes.  */

>  extern void *memalign (size_t __alignment, size_t __size)

> -__THROW __attribute_malloc__ __wur;

> +__THROW __attribute_malloc__ __attribute_alloc_size__ ((2)) __wur;

>  

>  /* Allocate SIZE bytes on a page boundary.  */

> -extern void *valloc (size_t __size) __THROW __attribute_malloc__ __wur;

> +extern void *valloc (size_t __size) __THROW __attribute_malloc__

> +     __attribute_alloc_size__ ((1)) __wur;

>  

>  /* Equivalent to valloc(minimum-page-that-holds(n)), that is, round up

>     __size to nearest pagesize. */

> -extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ __wur;

> +extern void *pvalloc (size_t __size) __THROW __attribute_malloc__

> +     __attribute_alloc_size__ ((1)) __wur;

>  

>  /* Underlying allocation function; successive calls should return

>     contiguous pieces of memory.  */

> diff --git a/malloc/tst-malloc-too-large.c b/malloc/tst-malloc-too-large.c

> index 15e25f558e..b652cb1ef4 100644

> --- a/malloc/tst-malloc-too-large.c

> +++ b/malloc/tst-malloc-too-large.c

> @@ -226,6 +226,16 @@ do_test (void)

>        test_large_aligned_allocations (SIZE_MAX - i);

>      }

>  

> +  /* Allocation larger than PTRDIFF_MAX does play well with C standard,

> +     since pointer subtraction within the object might overflow ptrdiff_t

> +     resulting in undefined behavior.  To prevent it malloc function fail

> +     for such allocations.  */

> +  for (size_t i = 0; i <= FOURTEEN_ON_BITS; i++)

> +    {

> +      test_large_allocations (PTRDIFF_MAX + i);

> +      test_large_aligned_allocations (PTRDIFF_MAX + i);

> +    }

> +

>  #if __WORDSIZE >= 64

>    /* On 64-bit targets, we need to test a much wider range of too-large

>       sizes, so we test at intervals of (1 << 50) that allocation sizes

> diff --git a/malloc/tst-memalign.c b/malloc/tst-memalign.c

> index a6a9140a3d..e7997518cb 100644

> --- a/malloc/tst-memalign.c

> +++ b/malloc/tst-memalign.c

> @@ -21,6 +21,7 @@

>  #include <stdio.h>

>  #include <string.h>

>  #include <unistd.h>

> +#include <libc-diag.h>

>  

>  static int errors = 0;

>  

> @@ -41,9 +42,18 @@ do_test (void)

>  

>    errno = 0;

>  

> +  DIAG_PUSH_NEEDS_COMMENT;

> +#if __GNUC_PREREQ (7, 0)

> +  /* GCC 7 warns about too-large allocations; here we want to test

> +     that they fail.  */

> +  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");

> +#endif

>    /* An attempt to allocate a huge value should return NULL and set

>       errno to ENOMEM.  */

>    p = memalign (sizeof (void *), -1);

> +#if __GNUC_PREREQ (7, 0)

> +  DIAG_POP_NEEDS_COMMENT;

> +#endif

>  

>    save = errno;

>  

> diff --git a/malloc/tst-posix_memalign.c b/malloc/tst-posix_memalign.c

> index bcf1592679..fc46955c51 100644

> --- a/malloc/tst-posix_memalign.c

> +++ b/malloc/tst-posix_memalign.c

> @@ -21,6 +21,7 @@

>  #include <stdio.h>

>  #include <string.h>

>  #include <unistd.h>

> +#include <libc-diag.h>

>  

>  static int errors = 0;

>  

> @@ -41,9 +42,18 @@ do_test (void)

>  

>    p = NULL;

>  

> +  DIAG_PUSH_NEEDS_COMMENT;

> +#if __GNUC_PREREQ (7, 0)

> +  /* GCC 7 warns about too-large allocations; here we want to test

> +     that they fail.  */

> +  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");

> +#endif

>    /* An attempt to allocate a huge value should return ENOMEM and

>       p should remain NULL.  */

>    ret = posix_memalign (&p, sizeof (void *), -1);

> +#if __GNUC_PREREQ (7, 0)

> +  DIAG_POP_NEEDS_COMMENT;

> +#endif

>  

>    if (ret != ENOMEM)

>      merror ("posix_memalign (&p, sizeof (void *), -1) succeeded.");

> diff --git a/malloc/tst-pvalloc.c b/malloc/tst-pvalloc.c

> index 0b4baba67a..c0356c0c71 100644

> --- a/malloc/tst-pvalloc.c

> +++ b/malloc/tst-pvalloc.c

> @@ -21,6 +21,7 @@

>  #include <stdio.h>

>  #include <string.h>

>  #include <unistd.h>

> +#include <libc-diag.h>

>  

>  static int errors = 0;

>  

> @@ -41,9 +42,18 @@ do_test (void)

>  

>    errno = 0;

>  

> +  DIAG_PUSH_NEEDS_COMMENT;

> +#if __GNUC_PREREQ (7, 0)

> +  /* GCC 7 warns about too-large allocations; here we want to test

> +     that they fail.  */

> +  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");

> +#endif

>    /* An attempt to allocate a huge value should return NULL and set

>       errno to ENOMEM.  */

>    p = pvalloc (-1);

> +#if __GNUC_PREREQ (7, 0)

> +  DIAG_POP_NEEDS_COMMENT;

> +#endif

>  

>    save = errno;

>  

> diff --git a/malloc/tst-reallocarray.c b/malloc/tst-reallocarray.c

> index 9a71ec7f45..fd50d4e36b 100644

> --- a/malloc/tst-reallocarray.c

> +++ b/malloc/tst-reallocarray.c

> @@ -20,6 +20,23 @@

>  #include <malloc.h>

>  #include <string.h>

>  #include <support/check.h>

> +#include <libc-diag.h>

> +

> +static void *

> +reallocarray_nowarn (void *ptr, size_t nmemb, size_t size)

> +{

> +#if __GNUC_PREREQ (7, 0)

> +  /* GCC 7 warns about too-large allocations; here we want to test

> +     that they fail.  */

> +  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");

> +#endif

> +  void *ret = reallocarray (ptr, nmemb, size);

> +#if __GNUC_PREREQ (7, 0)

> +  DIAG_POP_NEEDS_COMMENT;

> +#endif

> +  return ret;

> +}

> +

>  

>  static int

>  do_test (void)

> @@ -34,24 +51,24 @@ do_test (void)

>  

>    /* Test overflow detection.  */

>    errno = 0;

> -  ptr = reallocarray (NULL, max, 2);

> +  ptr = reallocarray_nowarn (NULL, max, 2);

>    TEST_VERIFY (!ptr);

>    TEST_VERIFY (errno == ENOMEM);

>  

>    errno = 0;

> -  ptr = reallocarray (NULL, 2, max);

> +  ptr = reallocarray_nowarn (NULL, 2, max);

>    TEST_VERIFY (!ptr);

>    TEST_VERIFY (errno == ENOMEM);

>  

>    a = 65537;

>    b = max/65537 + 1;

>    errno = 0;

> -  ptr = reallocarray (NULL, a, b);

> +  ptr = reallocarray_nowarn (NULL, a, b);

>    TEST_VERIFY (!ptr);

>    TEST_VERIFY (errno == ENOMEM);

>  

>    errno = 0;

> -  ptr = reallocarray (NULL, b, a);

> +  ptr = reallocarray_nowarn (NULL, b, a);

>    TEST_VERIFY (!ptr);

>    TEST_VERIFY (errno == ENOMEM);

>  

> @@ -97,7 +114,7 @@ do_test (void)

>  

>    /* Overflow should leave buffer untouched.  */

>    errno = 0;

> -  ptr2 = reallocarray (ptr, 2, ~(size_t)0);

> +  ptr2 = reallocarray_nowarn (ptr, 2, ~(size_t)0);

>    TEST_VERIFY (!ptr2);

>    TEST_VERIFY (errno == ENOMEM);

>  

> diff --git a/malloc/tst-valloc.c b/malloc/tst-valloc.c

> index a2e27c1e91..1a1a2d2500 100644

> --- a/malloc/tst-valloc.c

> +++ b/malloc/tst-valloc.c

> @@ -21,6 +21,7 @@

>  #include <stdio.h>

>  #include <string.h>

>  #include <unistd.h>

> +#include <libc-diag.h>

>  

>  static int errors = 0;

>  

> @@ -41,9 +42,18 @@ do_test (void)

>  

>    errno = 0;

>  

> +  DIAG_PUSH_NEEDS_COMMENT;

> +#if __GNUC_PREREQ (7, 0)

> +  /* GCC 7 warns about too-large allocations; here we want to test

> +     that they fail.  */

> +  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");

> +#endif

>    /* An attempt to allocate a huge value should return NULL and set

>       errno to ENOMEM.  */

>    p = valloc (-1);

> +#if __GNUC_PREREQ (7, 0)

> +  DIAG_POP_NEEDS_COMMENT;

> +#endif

>  

>    save = errno;

>  

> diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h

> index 15debe1e31..a9fd989d39 100644

> --- a/stdlib/stdlib.h

> +++ b/stdlib/stdlib.h

> @@ -536,10 +536,11 @@ extern int lcong48_r (unsigned short int __param[7],

>  #endif	/* Use misc or X/Open.  */

>  

>  /* Allocate SIZE bytes of memory.  */

> -extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;

> +extern void *malloc (size_t __size) __THROW __attribute_malloc__

> +     __attribute_alloc_size__ ((1)) __wur;

>  /* Allocate NMEMB elements of SIZE bytes each, all initialized to 0.  */

>  extern void *calloc (size_t __nmemb, size_t __size)

> -     __THROW __attribute_malloc__ __wur;

> +     __THROW __attribute_malloc__ __attribute_alloc_size__ ((1, 2)) __wur;

>  

>  /* Re-allocate the previously allocated block

>     in PTR, making the new block SIZE bytes long.  */

> @@ -547,7 +548,7 @@ extern void *calloc (size_t __nmemb, size_t __size)

>     the same pointer that was passed to it, aliasing needs to be allowed

>     between objects pointed by the old and new pointers.  */

>  extern void *realloc (void *__ptr, size_t __size)

> -     __THROW __attribute_warn_unused_result__;

> +     __THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2));

>  

>  #ifdef __USE_MISC

>  /* Re-allocate the previously allocated block in PTR, making the new

> @@ -556,7 +557,8 @@ extern void *realloc (void *__ptr, size_t __size)

>     the same pointer that was passed to it, aliasing needs to be allowed

>     between objects pointed by the old and new pointers.  */

>  extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)

> -     __THROW __attribute_warn_unused_result__;

> +     __THROW __attribute_warn_unused_result__

> +     __attribute_alloc_size__ ((2, 3));

>  #endif

>  

>  /* Free a block allocated by `malloc', `realloc' or `calloc'.  */

> @@ -569,7 +571,8 @@ extern void free (void *__ptr) __THROW;

>  #if (defined __USE_XOPEN_EXTENDED && !defined __USE_XOPEN2K) \

>      || defined __USE_MISC

>  /* Allocate SIZE bytes on a page boundary.  The storage cannot be freed.  */

> -extern void *valloc (size_t __size) __THROW __attribute_malloc__ __wur;

> +extern void *valloc (size_t __size) __THROW __attribute_malloc__

> +     __attribute_alloc_size__ ((1)) __wur;

>  #endif

>  

>  #ifdef __USE_XOPEN2K

>
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index cd29ec7c81..67dd692b73 100644
--- a/NEWS
+++ b/NEWS
@@ -68,6 +68,12 @@  Deprecated and removed features, and other changes affecting compatibility:
   used by the Linux kernel.  This affects the size and layout of those
   structures.
 
+* Memory allocation functions malloc, calloc, realloc, reallocarray, valloc,
+  pvalloc, memalign, and posix_memalign fail now with total object size
+  larger than PTRDIFF_MAX.  This is to avoid potential undefined behavior with
+  pointer subtraction within the allocated object, where results might
+  overflow the ptrdiff_t type.
+
 Changes to build and runtime requirements:
 
 * Python 3.4 or later is required to build the GNU C Library.
diff --git a/malloc/hooks.c b/malloc/hooks.c
index ae7305b036..c1deb9edf3 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -226,8 +226,9 @@  static void *
 malloc_check (size_t sz, const void *caller)
 {
   void *victim;
+  size_t nb;
 
-  if (sz + 1 == 0)
+  if (__builtin_add_overflow (sz, 1, &nb))
     {
       __set_errno (ENOMEM);
       return NULL;
@@ -235,7 +236,7 @@  malloc_check (size_t sz, const void *caller)
 
   __libc_lock_lock (main_arena.mutex);
   top_check ();
-  victim = _int_malloc (&main_arena, sz + 1);
+  victim = _int_malloc (&main_arena, nb);
   __libc_lock_unlock (main_arena.mutex);
   return mem2mem_check (victim, sz);
 }
@@ -268,8 +269,9 @@  realloc_check (void *oldmem, size_t bytes, const void *caller)
   INTERNAL_SIZE_T nb;
   void *newmem = 0;
   unsigned char *magic_p;
+  size_t rb;
 
-  if (bytes + 1 == 0)
+  if (__builtin_add_overflow (bytes, 1, &rb))
     {
       __set_errno (ENOMEM);
       return NULL;
@@ -289,7 +291,11 @@  realloc_check (void *oldmem, size_t bytes, const void *caller)
     malloc_printerr ("realloc(): invalid pointer");
   const INTERNAL_SIZE_T oldsize = chunksize (oldp);
 
-  checked_request2size (bytes + 1, nb);
+  if (!checked_request2size (rb, &nb))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
   __libc_lock_lock (main_arena.mutex);
 
   if (chunk_is_mmapped (oldp))
@@ -308,7 +314,7 @@  realloc_check (void *oldmem, size_t bytes, const void *caller)
           {
             /* Must alloc, copy, free. */
 	    top_check ();
-	    newmem = _int_malloc (&main_arena, bytes + 1);
+	    newmem = _int_malloc (&main_arena, rb);
             if (newmem)
               {
                 memcpy (newmem, oldmem, oldsize - 2 * SIZE_SZ);
@@ -320,8 +326,6 @@  realloc_check (void *oldmem, size_t bytes, const void *caller)
   else
     {
       top_check ();
-      INTERNAL_SIZE_T nb;
-      checked_request2size (bytes + 1, nb);
       newmem = _int_realloc (&main_arena, oldp, oldsize, nb);
     }
 
diff --git a/malloc/malloc.c b/malloc/malloc.c
index c33709e966..21a117bd6b 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1187,17 +1187,6 @@  nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   ((uintptr_t)(MALLOC_ALIGNMENT == 2 * SIZE_SZ ? (p) : chunk2mem (p)) \
    & MALLOC_ALIGN_MASK)
 
-
-/*
-   Check if a request is so large that it would wrap around zero when
-   padded and aligned. To simplify some other code, the bound is made
-   low enough so that adding MINSIZE will also not wrap around zero.
- */
-
-#define REQUEST_OUT_OF_RANGE(req)                                 \
-  ((unsigned long) (req) >=						      \
-   (unsigned long) (INTERNAL_SIZE_T) (-2 * MINSIZE))
-
 /* pad request bytes into a usable size -- internal version */
 
 #define request2size(req)                                         \
@@ -1205,21 +1194,27 @@  nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    MINSIZE :                                                      \
    ((req) + SIZE_SZ + MALLOC_ALIGN_MASK) & ~MALLOC_ALIGN_MASK)
 
-/* Same, except also perform an argument and result check.  First, we check
-   that the padding done by request2size didn't result in an integer
-   overflow.  Then we check (using REQUEST_OUT_OF_RANGE) that the resulting
-   size isn't so large that a later alignment would lead to another integer
-   overflow.  */
-#define checked_request2size(req, sz) \
-({				    \
-  (sz) = request2size (req);	    \
-  if (((sz) < (req))		    \
-      || REQUEST_OUT_OF_RANGE (sz)) \
-    {				    \
-      __set_errno (ENOMEM);	    \
-      return 0;			    \
-    }				    \
-})
+/* Check if REQ overflows when padded and aligned and if the resulting value
+   is less than PTRDIFF_T.  Returns TRUE and the requested size or MINSIZE in
+   case the value is less than MINSIZE on SZ or false if any of the previous
+   check fail.  */
+static inline bool
+checked_request2size (size_t req, size_t *sz) __nonnull (1)
+{
+  size_t ret;
+  if (__glibc_unlikely (__builtin_add_overflow (req,
+						SIZE_SZ + MALLOC_ALIGN_MASK,
+						&ret)))
+    return false;
+
+  ret = ret < MINSIZE ? MINSIZE : ret & ~MALLOC_ALIGN_MASK;
+  if (__glibc_unlikely (ret > PTRDIFF_MAX))
+    return false;
+
+  *sz = ret;
+  return true;
+}
+
 
 /*
    --------------- Physical chunk operations ---------------
@@ -3109,6 +3104,9 @@  __libc_malloc (size_t bytes)
   mstate ar_ptr;
   void *victim;
 
+  _Static_assert (PTRDIFF_MAX <= SIZE_MAX / 2,
+                  "PTRDIFF_MAX is not more than half of SIZE_MAX");
+
   void *(*hook) (size_t, const void *)
     = atomic_forced_read (__malloc_hook);
   if (__builtin_expect (hook != NULL, 0))
@@ -3116,7 +3114,11 @@  __libc_malloc (size_t bytes)
 #if USE_TCACHE
   /* int_free also calls request2size, be careful to not pad twice.  */
   size_t tbytes;
-  checked_request2size (bytes, tbytes);
+  if (!checked_request2size (bytes, &tbytes))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
   size_t tc_idx = csize2tidx (tbytes);
 
   MAYBE_INIT_TCACHE ();
@@ -3253,7 +3255,11 @@  __libc_realloc (void *oldmem, size_t bytes)
       && !DUMPED_MAIN_ARENA_CHUNK (oldp))
       malloc_printerr ("realloc(): invalid pointer");
 
-  checked_request2size (bytes, nb);
+  if (!checked_request2size (bytes, &nb))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
 
   if (chunk_is_mmapped (oldp))
     {
@@ -3363,13 +3369,6 @@  _mid_memalign (size_t alignment, size_t bytes, void *address)
       return 0;
     }
 
-  /* Check for overflow.  */
-  if (bytes > SIZE_MAX - alignment - MINSIZE)
-    {
-      __set_errno (ENOMEM);
-      return 0;
-    }
-
 
   /* Make sure alignment is power of 2.  */
   if (!powerof2 (alignment))
@@ -3429,14 +3428,16 @@  __libc_pvalloc (size_t bytes)
 
   void *address = RETURN_ADDRESS (0);
   size_t pagesize = GLRO (dl_pagesize);
-  size_t rounded_bytes = ALIGN_UP (bytes, pagesize);
-
-  /* Check for overflow.  */
-  if (bytes > SIZE_MAX - 2 * pagesize - MINSIZE)
+  size_t rounded_bytes;
+  /* ALIGN_UP with overflow check.  */
+  if (__glibc_unlikely (__builtin_add_overflow (bytes,
+						pagesize - 1,
+						&rounded_bytes)))
     {
       __set_errno (ENOMEM);
       return 0;
     }
+  rounded_bytes = rounded_bytes & -(pagesize - 1);
 
   return _mid_memalign (pagesize, rounded_bytes, address);
 }
@@ -3446,30 +3447,24 @@  __libc_calloc (size_t n, size_t elem_size)
 {
   mstate av;
   mchunkptr oldtop, p;
-  INTERNAL_SIZE_T bytes, sz, csz, oldtopsize;
+  INTERNAL_SIZE_T sz, csz, oldtopsize;
   void *mem;
   unsigned long clearsize;
   unsigned long nclears;
   INTERNAL_SIZE_T *d;
+  ptrdiff_t bytes;
 
-  /* size_t is unsigned so the behavior on overflow is defined.  */
-  bytes = n * elem_size;
-#define HALF_INTERNAL_SIZE_T \
-  (((INTERNAL_SIZE_T) 1) << (8 * sizeof (INTERNAL_SIZE_T) / 2))
-  if (__builtin_expect ((n | elem_size) >= HALF_INTERNAL_SIZE_T, 0))
+  if (__glibc_unlikely (__builtin_mul_overflow (n, elem_size, &bytes)))
     {
-      if (elem_size != 0 && bytes / elem_size != n)
-        {
-          __set_errno (ENOMEM);
-          return 0;
-        }
+       __set_errno (ENOMEM);
+       return NULL;
     }
+  sz = bytes;
 
   void *(*hook) (size_t, const void *) =
     atomic_forced_read (__malloc_hook);
   if (__builtin_expect (hook != NULL, 0))
     {
-      sz = bytes;
       mem = (*hook)(sz, RETURN_ADDRESS (0));
       if (mem == 0)
         return 0;
@@ -3477,8 +3472,6 @@  __libc_calloc (size_t n, size_t elem_size)
       return memset (mem, 0, sz);
     }
 
-  sz = bytes;
-
   MAYBE_INIT_TCACHE ();
 
   if (SINGLE_THREAD_P)
@@ -3625,12 +3618,16 @@  _int_malloc (mstate av, size_t bytes)
      Convert request size to internal form by adding SIZE_SZ bytes
      overhead plus possibly more to obtain necessary alignment and/or
      to obtain a size of at least MINSIZE, the smallest allocatable
-     size. Also, checked_request2size traps (returning 0) request sizes
+     size. Also, checked_request2size returns false for request sizes
      that are so large that they wrap around zero when padded and
      aligned.
    */
 
-  checked_request2size (bytes, nb);
+  if (!checked_request2size (bytes, &nb))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
 
   /* There are no usable arenas.  Fall back to sysmalloc to get a chunk from
      mmap.  */
@@ -4743,21 +4740,17 @@  _int_memalign (mstate av, size_t alignment, size_t bytes)
 
 
 
-  checked_request2size (bytes, nb);
+  if (!checked_request2size (bytes, &nb))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
 
   /*
      Strategy: find a spot within that chunk that meets the alignment
      request, and then possibly free the leading and trailing space.
    */
 
-
-  /* Check for overflow.  */
-  if (nb > SIZE_MAX - alignment - MINSIZE)
-    {
-      __set_errno (ENOMEM);
-      return 0;
-    }
-
   /* Call malloc with worst case padding to hit alignment. */
 
   m = (char *) (_int_malloc (av, nb + alignment + MINSIZE));
diff --git a/malloc/malloc.h b/malloc/malloc.h
index 3e7c447be1..d3c4841f11 100644
--- a/malloc/malloc.h
+++ b/malloc/malloc.h
@@ -35,11 +35,12 @@ 
 __BEGIN_DECLS
 
 /* Allocate SIZE bytes of memory.  */
-extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;
+extern void *malloc (size_t __size) __THROW __attribute_malloc__
+     __attribute_alloc_size__ ((1)) __wur;
 
 /* Allocate NMEMB elements of SIZE bytes each, all initialized to 0.  */
 extern void *calloc (size_t __nmemb, size_t __size)
-__THROW __attribute_malloc__ __wur;
+__THROW __attribute_malloc__ __attribute_alloc_size__ ((1, 2)) __wur;
 
 /* Re-allocate the previously allocated block in __ptr, making the new
    block SIZE bytes long.  */
@@ -47,7 +48,7 @@  __THROW __attribute_malloc__ __wur;
    the same pointer that was passed to it, aliasing needs to be allowed
    between objects pointed by the old and new pointers.  */
 extern void *realloc (void *__ptr, size_t __size)
-__THROW __attribute_warn_unused_result__;
+__THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2));
 
 /* Re-allocate the previously allocated block in PTR, making the new
    block large enough for NMEMB elements of SIZE bytes each.  */
@@ -55,21 +56,23 @@  __THROW __attribute_warn_unused_result__;
    the same pointer that was passed to it, aliasing needs to be allowed
    between objects pointed by the old and new pointers.  */
 extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)
-__THROW __attribute_warn_unused_result__;
+__THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2, 3));
 
 /* Free a block allocated by `malloc', `realloc' or `calloc'.  */
 extern void free (void *__ptr) __THROW;
 
 /* Allocate SIZE bytes allocated to ALIGNMENT bytes.  */
 extern void *memalign (size_t __alignment, size_t __size)
-__THROW __attribute_malloc__ __wur;
+__THROW __attribute_malloc__ __attribute_alloc_size__ ((2)) __wur;
 
 /* Allocate SIZE bytes on a page boundary.  */
-extern void *valloc (size_t __size) __THROW __attribute_malloc__ __wur;
+extern void *valloc (size_t __size) __THROW __attribute_malloc__
+     __attribute_alloc_size__ ((1)) __wur;
 
 /* Equivalent to valloc(minimum-page-that-holds(n)), that is, round up
    __size to nearest pagesize. */
-extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ __wur;
+extern void *pvalloc (size_t __size) __THROW __attribute_malloc__
+     __attribute_alloc_size__ ((1)) __wur;
 
 /* Underlying allocation function; successive calls should return
    contiguous pieces of memory.  */
diff --git a/malloc/tst-malloc-too-large.c b/malloc/tst-malloc-too-large.c
index 10fb136528..ca41827a2a 100644
--- a/malloc/tst-malloc-too-large.c
+++ b/malloc/tst-malloc-too-large.c
@@ -226,6 +226,16 @@  do_test (void)
       test_large_aligned_allocations (SIZE_MAX - i);
     }
 
+  /* Allocation larger than PTRDIFF_MAX does play well with C standard,
+     since pointer subtraction within the object might overflow ptrdiff_t
+     resulting in undefined behavior.  To prevent it malloc function fail
+     for such allocations.  */
+  for (size_t i = 0; i <= FOURTEEN_ON_BITS; i++)
+    {
+      test_large_allocations (PTRDIFF_MAX + i);
+      test_large_aligned_allocations (PTRDIFF_MAX + i);
+    }
+
 #if __WORDSIZE >= 64
   /* On 64-bit targets, we need to test a much wider range of too-large
      sizes, so we test at intervals of (1 << 50) that allocation sizes
diff --git a/malloc/tst-memalign.c b/malloc/tst-memalign.c
index 904bf9491d..3cba6dc7d3 100644
--- a/malloc/tst-memalign.c
+++ b/malloc/tst-memalign.c
@@ -21,6 +21,7 @@ 
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <libc-diag.h>
 
 static int errors = 0;
 
@@ -41,9 +42,18 @@  do_test (void)
 
   errno = 0;
 
+  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about too-large allocations; here we want to test
+     that they fail.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");
+#endif
   /* An attempt to allocate a huge value should return NULL and set
      errno to ENOMEM.  */
   p = memalign (sizeof (void *), -1);
+#if __GNUC_PREREQ (7, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
 
   save = errno;
 
diff --git a/malloc/tst-posix_memalign.c b/malloc/tst-posix_memalign.c
index 81b4c05ef4..5c4188a7ac 100644
--- a/malloc/tst-posix_memalign.c
+++ b/malloc/tst-posix_memalign.c
@@ -21,6 +21,7 @@ 
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <libc-diag.h>
 
 static int errors = 0;
 
@@ -41,9 +42,18 @@  do_test (void)
 
   p = NULL;
 
+  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about too-large allocations; here we want to test
+     that they fail.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");
+#endif
   /* An attempt to allocate a huge value should return ENOMEM and
      p should remain NULL.  */
   ret = posix_memalign (&p, sizeof (void *), -1);
+#if __GNUC_PREREQ (7, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
 
   if (ret != ENOMEM)
     merror ("posix_memalign (&p, sizeof (void *), -1) succeeded.");
diff --git a/malloc/tst-pvalloc.c b/malloc/tst-pvalloc.c
index aa391447ee..cd3b46d68b 100644
--- a/malloc/tst-pvalloc.c
+++ b/malloc/tst-pvalloc.c
@@ -21,6 +21,7 @@ 
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <libc-diag.h>
 
 static int errors = 0;
 
@@ -41,9 +42,18 @@  do_test (void)
 
   errno = 0;
 
+  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about too-large allocations; here we want to test
+     that they fail.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");
+#endif
   /* An attempt to allocate a huge value should return NULL and set
      errno to ENOMEM.  */
   p = pvalloc (-1);
+#if __GNUC_PREREQ (7, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
 
   save = errno;
 
diff --git a/malloc/tst-reallocarray.c b/malloc/tst-reallocarray.c
index b0d80ebc58..784f77e7b0 100644
--- a/malloc/tst-reallocarray.c
+++ b/malloc/tst-reallocarray.c
@@ -20,6 +20,23 @@ 
 #include <malloc.h>
 #include <string.h>
 #include <support/check.h>
+#include <libc-diag.h>
+
+static void *
+reallocarray_nowarn (void *ptr, size_t nmemb, size_t size)
+{
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about too-large allocations; here we want to test
+     that they fail.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");
+#endif
+  void *ret = reallocarray (ptr, nmemb, size);
+#if __GNUC_PREREQ (7, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
+  return ret;
+}
+
 
 static int
 do_test (void)
@@ -34,24 +51,24 @@  do_test (void)
 
   /* Test overflow detection.  */
   errno = 0;
-  ptr = reallocarray (NULL, max, 2);
+  ptr = reallocarray_nowarn (NULL, max, 2);
   TEST_VERIFY (!ptr);
   TEST_VERIFY (errno == ENOMEM);
 
   errno = 0;
-  ptr = reallocarray (NULL, 2, max);
+  ptr = reallocarray_nowarn (NULL, 2, max);
   TEST_VERIFY (!ptr);
   TEST_VERIFY (errno == ENOMEM);
 
   a = 65537;
   b = max/65537 + 1;
   errno = 0;
-  ptr = reallocarray (NULL, a, b);
+  ptr = reallocarray_nowarn (NULL, a, b);
   TEST_VERIFY (!ptr);
   TEST_VERIFY (errno == ENOMEM);
 
   errno = 0;
-  ptr = reallocarray (NULL, b, a);
+  ptr = reallocarray_nowarn (NULL, b, a);
   TEST_VERIFY (!ptr);
   TEST_VERIFY (errno == ENOMEM);
 
@@ -97,7 +114,7 @@  do_test (void)
 
   /* Overflow should leave buffer untouched.  */
   errno = 0;
-  ptr2 = reallocarray (ptr, 2, ~(size_t)0);
+  ptr2 = reallocarray_nowarn (ptr, 2, ~(size_t)0);
   TEST_VERIFY (!ptr2);
   TEST_VERIFY (errno == ENOMEM);
 
diff --git a/malloc/tst-valloc.c b/malloc/tst-valloc.c
index ba57d9559a..b37edb6c74 100644
--- a/malloc/tst-valloc.c
+++ b/malloc/tst-valloc.c
@@ -21,6 +21,7 @@ 
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <libc-diag.h>
 
 static int errors = 0;
 
@@ -41,9 +42,18 @@  do_test (void)
 
   errno = 0;
 
+  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (7, 0)
+  /* GCC 7 warns about too-large allocations; here we want to test
+     that they fail.  */
+  DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than=");
+#endif
   /* An attempt to allocate a huge value should return NULL and set
      errno to ENOMEM.  */
   p = valloc (-1);
+#if __GNUC_PREREQ (7, 0)
+  DIAG_POP_NEEDS_COMMENT;
+#endif
 
   save = errno;
 
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index 870e02d904..4e55d82f4c 100644
--- a/stdlib/stdlib.h
+++ b/stdlib/stdlib.h
@@ -536,10 +536,11 @@  extern int lcong48_r (unsigned short int __param[7],
 #endif	/* Use misc or X/Open.  */
 
 /* Allocate SIZE bytes of memory.  */
-extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;
+extern void *malloc (size_t __size) __THROW __attribute_malloc__
+     __attribute_alloc_size__ ((1)) __wur;
 /* Allocate NMEMB elements of SIZE bytes each, all initialized to 0.  */
 extern void *calloc (size_t __nmemb, size_t __size)
-     __THROW __attribute_malloc__ __wur;
+     __THROW __attribute_malloc__ __attribute_alloc_size__ ((1, 2)) __wur;
 
 /* Re-allocate the previously allocated block
    in PTR, making the new block SIZE bytes long.  */
@@ -547,7 +548,7 @@  extern void *calloc (size_t __nmemb, size_t __size)
    the same pointer that was passed to it, aliasing needs to be allowed
    between objects pointed by the old and new pointers.  */
 extern void *realloc (void *__ptr, size_t __size)
-     __THROW __attribute_warn_unused_result__;
+     __THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2));
 
 #ifdef __USE_MISC
 /* Re-allocate the previously allocated block in PTR, making the new
@@ -556,7 +557,8 @@  extern void *realloc (void *__ptr, size_t __size)
    the same pointer that was passed to it, aliasing needs to be allowed
    between objects pointed by the old and new pointers.  */
 extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)
-     __THROW __attribute_warn_unused_result__;
+     __THROW __attribute_warn_unused_result__
+     __attribute_alloc_size__ ((2, 3));
 #endif
 
 /* Free a block allocated by `malloc', `realloc' or `calloc'.  */
@@ -569,7 +571,8 @@  extern void free (void *__ptr) __THROW;
 #if (defined __USE_XOPEN_EXTENDED && !defined __USE_XOPEN2K) \
     || defined __USE_MISC
 /* Allocate SIZE bytes on a page boundary.  The storage cannot be freed.  */
-extern void *valloc (size_t __size) __THROW __attribute_malloc__ __wur;
+extern void *valloc (size_t __size) __THROW __attribute_malloc__
+     __attribute_alloc_size__ ((1)) __wur;
 #endif
 
 #ifdef __USE_XOPEN2K