diff mbox series

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

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

Commit Message

Adhemerval Zanella Dec. 21, 2018, 6:38 p.m. UTC 
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 (minus internal padding and alignment when required).

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
now is only to limit such requested size for malloc functions, not system
allocation one (mmap, sbrk, etc.).  This patch follows this idea.

The implementation removes checked_request2size and move the requested
size before hook calls (this avoid code duplication in the hook and make
allocations returned by hooks safe even if they do not check for requested
size).

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

	[BZ #23741]
	* malloc/hooks.c (malloc_check, realloc_check, memalign_check): Remove
	overflow check.
	* malloc/malloc.c (__libc_malloc, __libc_realloc, _mid_memalign,
	__libc_pvalloc, __libc_calloc, _int_memalign): Limit maximum
	allocation size to PTRDIFF_MAX.
	(_int_malloc): Remove overflow check.
	(padize): New define.
	(request2size): Define based on padsize.
	* 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.

[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                     |  25 +++++++
 malloc/hooks.c                |  22 +------
 malloc/malloc.c               | 118 +++++++++++++---------------------
 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               |  15 +++--
 11 files changed, 160 insertions(+), 114 deletions(-)

-- 
2.17.1

Comments

Joseph Myers Dec. 21, 2018, 6:41 p.m. UTC | #1 
I think this needs a NEWS entry (in the "Deprecated and removed features, 
and other changes affecting compatibility" section), as it may affect 
people currently doing > 2GB allocations in 32-bit programs.

-- 
Joseph S. Myers
joseph@codesourcery.com
Adhemerval Zanella Dec. 21, 2018, 7:30 p.m. UTC | #2 
On 21/12/2018 16:41, Joseph Myers wrote:
> I think this needs a NEWS entry (in the "Deprecated and removed features, 

> and other changes affecting compatibility" section), as it may affect 

> people currently doing > 2GB allocations in 32-bit programs.

> 


What about:

  * Memory allocation functions malloc, calloc, realloc, reallocarray, valloc,
    pvalloc, memaling, and posix_memalignfail now fail with requests larger
    than PTRDIFF_MAX.  This is to avoid potential undefined behavior with
    pointer subtraction within the allocated object, where results might
    in ptrdiff_t type overflow.


As a side note, gcc 9 now seems to add -Wattributes in -Wall which results
in a "error: ‘alloc_size’ attribute ignored on a function returning ‘int’" 
on posix_memalign with __attribute_alloc_size__ ((3)). 

Does gcc provide an attribute to indicate first argument is the expected 
result, so it can match the attribute with argument type instead of
function returned one? Otherwise I think we will need to suppress it 
from posix_memalign.
Joseph Myers Dec. 21, 2018, 7:33 p.m. UTC | #3 
On Fri, 21 Dec 2018, Adhemerval Zanella wrote:

> What about:

> 

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

>     pvalloc, memaling, and posix_memalignfail now fail with requests larger


Typos "memaling", "posix_memalignfail".

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

>     pointer subtraction within the allocated object, where results might

>     in ptrdiff_t type overflow.


"might overflow the ptrdiff_t type".

> Does gcc provide an attribute to indicate first argument is the expected 

> result, so it can match the attribute with argument type instead of

> function returned one? Otherwise I think we will need to suppress it 

> from posix_memalign.


No, the alloc_size attribute is inapplicable to posix_memalign.

-- 
Joseph S. Myers
joseph@codesourcery.com
Adhemerval Zanella Dec. 21, 2018, 7:49 p.m. UTC | #4 
On 21/12/2018 17:33, Joseph Myers wrote:
> On Fri, 21 Dec 2018, Adhemerval Zanella wrote:

> 

>> What about:

>>

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

>>     pvalloc, memaling, and posix_memalignfail now fail with requests larger

> 

> Typos "memaling", "posix_memalignfail".

> 

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

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

>>     in ptrdiff_t type overflow.

> 

> "might overflow the ptrdiff_t type".

> 

>> Does gcc provide an attribute to indicate first argument is the expected 

>> result, so it can match the attribute with argument type instead of

>> function returned one? Otherwise I think we will need to suppress it 

>> from posix_memalign.

> 

> No, the alloc_size attribute is inapplicable to posix_memalign.

> 


Updated patch below.  The changes are to add a NEWS entry and to remove
the alloc_size attribute from posix_memalign.

---

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 (minus internal padding and alignment when required).

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
now is only to limit such requested size for malloc functions, not system
allocation one (mmap, sbrk, etc.).  This patch follows this idea.

The implementation removes checked_request2size and move the requested
size before hook calls (this avoid code duplication in the hook and make
allocations returned by hooks safe even if they do not check for requested
size).

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

	[BZ #23741]
	* malloc/hooks.c (malloc_check, realloc_check, memalign_check): Remove
	overflow check.
	* malloc/malloc.c (__libc_malloc, __libc_realloc, _mid_memalign,
	__libc_pvalloc, __libc_calloc, _int_memalign): Limit maximum
	allocation size to PTRDIFF_MAX.
	(_int_malloc): Remove overflow check.
	(padize): New define.
	(request2size): Define based on padsize.
	* 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                     |  26 ++++++++
 NEWS                          |   6 ++
 malloc/hooks.c                |  22 +------
 malloc/malloc.c               | 118 +++++++++++++---------------------
 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, 166 insertions(+), 113 deletions(-)

diff --git a/NEWS b/NEWS
index 300e978de6..cad50fa3bc 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 fail with requests 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..b77c908e28 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -227,12 +227,6 @@ malloc_check (size_t sz, const void *caller)
 {
   void *victim;
 
-  if (sz + 1 == 0)
-    {
-      __set_errno (ENOMEM);
-      return NULL;
-    }
-
   __libc_lock_lock (main_arena.mutex);
   top_check ();
   victim = _int_malloc (&main_arena, sz + 1);
@@ -269,11 +263,6 @@ realloc_check (void *oldmem, size_t bytes, const void *caller)
   void *newmem = 0;
   unsigned char *magic_p;
 
-  if (bytes + 1 == 0)
-    {
-      __set_errno (ENOMEM);
-      return NULL;
-    }
   if (oldmem == 0)
     return malloc_check (bytes, NULL);
 
@@ -289,7 +278,7 @@ 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);
+  nb = request2size (bytes + 1);
   __libc_lock_lock (main_arena.mutex);
 
   if (chunk_is_mmapped (oldp))
@@ -320,8 +309,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);
     }
 
@@ -362,13 +349,6 @@ memalign_check (size_t alignment, size_t bytes, const void *caller)
       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))
     {
diff --git a/malloc/malloc.c b/malloc/malloc.c
index c33709e966..bb80809983 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -247,6 +247,9 @@
 /* For SINGLE_THREAD_P.  */
 #include <sysdep-cancel.h>
 
+/* For INT_MULTIPLY_WRAPV.  */
+#include <intprops.h>
+
 /*
   Debugging:
 
@@ -1187,39 +1190,15 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   ((uintptr_t)(MALLOC_ALIGNMENT == 2 * SIZE_SZ ? (p) : chunk2mem (p)) \
    & MALLOC_ALIGN_MASK)
 
+/* pad request bytes into a usable size -- internal version */
 
-/*
-   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))
+#define padsize (SIZE_SZ + MALLOC_ALIGN_MASK)
 
-/* pad request bytes into a usable size -- internal version */
+#define request2size(req)                     \
+  (((req) + padsize < MINSIZE)  ?             \
+   MINSIZE :                                  \
+   ((req) + padsize) & ~MALLOC_ALIGN_MASK)
 
-#define request2size(req)                                         \
-  (((req) + SIZE_SZ + MALLOC_ALIGN_MASK < MINSIZE)  ?             \
-   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;			    \
-    }				    \
-})
 
 /*
    --------------- Physical chunk operations ---------------
@@ -3109,14 +3088,20 @@ __libc_malloc (size_t bytes)
   mstate ar_ptr;
   void *victim;
 
+  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
+
   void *(*hook) (size_t, const void *)
     = atomic_forced_read (__malloc_hook);
   if (__builtin_expect (hook != NULL, 0))
     return (*hook)(bytes, RETURN_ADDRESS (0));
+
 #if USE_TCACHE
   /* int_free also calls request2size, be careful to not pad twice.  */
-  size_t tbytes;
-  checked_request2size (bytes, tbytes);
+  size_t tbytes = request2size (bytes);
   size_t tc_idx = csize2tidx (tbytes);
 
   MAYBE_INIT_TCACHE ();
@@ -3213,6 +3198,12 @@ __libc_realloc (void *oldmem, size_t bytes)
 
   void *newp;             /* chunk to return */
 
+  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
+    {
+      __set_errno (ENOMEM);
+      return 0;
+    }
+
   void *(*hook) (void *, size_t, const void *) =
     atomic_forced_read (__realloc_hook);
   if (__builtin_expect (hook != NULL, 0))
@@ -3253,7 +3244,7 @@ __libc_realloc (void *oldmem, size_t bytes)
       && !DUMPED_MAIN_ARENA_CHUNK (oldp))
       malloc_printerr ("realloc(): invalid pointer");
 
-  checked_request2size (bytes, nb);
+  nb = request2size (bytes);
 
   if (chunk_is_mmapped (oldp))
     {
@@ -3342,6 +3333,12 @@ _mid_memalign (size_t alignment, size_t bytes, void *address)
   mstate ar_ptr;
   void *p;
 
+  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
+    {
+      __set_errno (ENOMEM);
+      return 0;
+    }
+
   void *(*hook) (size_t, size_t, const void *) =
     atomic_forced_read (__memalign_hook);
   if (__builtin_expect (hook != NULL, 0))
@@ -3363,14 +3360,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))
     {
@@ -3432,7 +3421,7 @@ __libc_pvalloc (size_t bytes)
   size_t rounded_bytes = ALIGN_UP (bytes, pagesize);
 
   /* Check for overflow.  */
-  if (bytes > SIZE_MAX - 2 * pagesize - MINSIZE)
+  if (__glibc_unlikely (bytes > PTRDIFF_MAX - 2 * pagesize - MINSIZE))
     {
       __set_errno (ENOMEM);
       return 0;
@@ -3452,24 +3441,18 @@ __libc_calloc (size_t n, size_t elem_size)
   unsigned long nclears;
   INTERNAL_SIZE_T *d;
 
-  /* 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 (INT_MULTIPLY_WRAPV (n, elem_size, &bytes)
+      || __glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
     {
-      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 +3460,6 @@ __libc_calloc (size_t n, size_t elem_size)
       return memset (mem, 0, sz);
     }
 
-  sz = bytes;
-
   MAYBE_INIT_TCACHE ();
 
   if (SINGLE_THREAD_P)
@@ -3621,16 +3602,7 @@ _int_malloc (mstate av, size_t bytes)
   size_t tcache_unsorted_count;	    /* count of unsorted chunks processed */
 #endif
 
-  /*
-     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
-     that are so large that they wrap around zero when padded and
-     aligned.
-   */
-
-  checked_request2size (bytes, nb);
+  nb = request2size (bytes);
 
   /* There are no usable arenas.  Fall back to sysmalloc to get a chunk from
      mmap.  */
@@ -4741,22 +4713,18 @@ _int_memalign (mstate av, size_t alignment, size_t bytes)
   unsigned long remainder_size;   /* its size */
   INTERNAL_SIZE_T size;
 
-
-
-  checked_request2size (bytes, nb);
+  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
+    {
+      __set_errno (ENOMEM);
+      return 0;
+    }
 
   /*
      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;
-    }
+  nb = request2size (bytes);
 
   /* Call malloc with worst case padding to hit alignment. */
 
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
-- 
2.17.1
Florian Weimer Dec. 21, 2018, 9:26 p.m. UTC | #5 
* Adhemerval Zanella:

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

> +  pvalloc, memalign, and posix_memalign fail now fail with requests 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.


“fail now fail”.  “with requests larger than PTRDIFF_MAX” is
under-specified for calloc and reallocarray.  The intent probably is
that the total object size counts.

I have not reviewed the actual patch.
Paul Eggert Dec. 21, 2018, 10:32 p.m. UTC | #6 
The patch assumes that PTRDIFF_MAX is well under SIZE_MAX. A while ago Joseph 
wrote that m32c sometimes has ptrdiff_t wider than size_t 
<https://gcc.gnu.org/ml/gcc-patches/2011-07/msg00483.html> and if someone ever 
does a glibc port to such a platform this issue will come up in unlikely cases. 
Perhaps add something like the following to malloc/malloc.c, to make sure the 
problem is harder to ignore on such platforms?

/* malloc.c assumes that ptrdiff_t has no more bits than size_t.
    Although this assumption could be removed without hurting typical-case
    performance, doing this is low priority since the assumption holds
    on all current glibc platforms.  */
#include <verify.h>
verify (PTRDIFF_MAX <= SIZE_MAX / 2);


> +  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))

> +    {

> +      __set_errno (ENOMEM);

> +      return NULL;

> +    }


Why is the '- padsize' needed in these cases? All the relevant arithmetic is 
done using size_t, so overflow can't occur when PTRDIFF_MAX - padsize < bytes <= 
PTRDIFF_MAX, under the PTRDIFF_MAX <= SIZE_MAX / 2 assumption that the code is 
already making elsewhere.


> +  if (INT_MULTIPLY_WRAPV (n, elem_size, &bytes)

> +      || __glibc_unlikely (bytes > PTRDIFF_MAX - padsize))


This can be made a bit simpler by changing the type of this particular 'bytes' 
variable to ptrdiff_t and then by removing the "|| __glibc_unlikely (bytes > 
PTRDIFF_MAX - padsize)", since the "- padsize" isn't needed and the 
INT_MULTIPLY_WRAPV will check for exceeding PTRDIFF_MAX if 'bytes' is of type 
ptrdiff_t.
Joseph Myers Dec. 21, 2018, 10:37 p.m. UTC | #7 
On Fri, 21 Dec 2018, Paul Eggert wrote:

> The patch assumes that PTRDIFF_MAX is well under SIZE_MAX. A while ago Joseph

> wrote that m32c sometimes has ptrdiff_t wider than size_t

> <https://gcc.gnu.org/ml/gcc-patches/2011-07/msg00483.html> and if someone ever

> does a glibc port to such a platform this issue will come up in unlikely

> cases. Perhaps add something like the following to malloc/malloc.c, to make

> sure the problem is harder to ignore on such platforms?


Such platforms are well outside the scope of glibc.  We can assume that 
PTRDIFF_MAX == SIZE_MAX / 2 (and that ptrdiff_t, size_t and pointers have 
the same power-of-2 width, which is at least 32).

> #include <verify.h>

> verify (PTRDIFF_MAX <= SIZE_MAX / 2);


Using verify.h in any glibc code not coming from gnulib would be odd; just 
use _Static_assert.

-- 
Joseph S. Myers
joseph@codesourcery.com
Paul Eggert Dec. 23, 2018, 2:17 a.m. UTC | #8 
Joseph Myers wrote:
> We can assume that

> PTRDIFF_MAX == SIZE_MAX / 2 (and that ptrdiff_t, size_t and pointers have

> the same power-of-2 width, which is at least 32).


OK, then we needn't bother with the static assertion. Is there a good place to 
document assumptions like this one, in the glibc manual I suppose?

> Using verify.h in any glibc code not coming from gnulib would be odd; just

> use _Static_assert.


Although the point is now moot for this patch, I prefer the readability of 
'verify'. Compare this:

verify (PTRDIFF_MAX <= SIZE_MAX / 2);

to this:

_Static_assert (PTRDIFF_MAX <= SIZE_MAX / 2,
                 "PTRDIFF_MAX is not more than half of SIZE_MAX");

With just one static assertion it's no big deal, but Gnulib has files with 
dozens and the readability savings add up.
Adhemerval Zanella Dec. 27, 2018, 11:55 a.m. UTC | #9 
On 21/12/2018 20:32, Paul Eggert wrote:
> The patch assumes that PTRDIFF_MAX is well under SIZE_MAX. A while ago Joseph wrote that m32c sometimes has ptrdiff_t wider than size_t <https://gcc.gnu.org/ml/gcc-patches/2011-07/msg00483.html> and if someone ever does a glibc port to such a platform this issue will come up in unlikely cases. Perhaps add something like the following to malloc/malloc.c, to make sure the problem is harder to ignore on such platforms?

> 

> /* malloc.c assumes that ptrdiff_t has no more bits than size_t.

>    Although this assumption could be removed without hurting typical-case

>    performance, doing this is low priority since the assumption holds

>    on all current glibc platforms.  */

> #include <verify.h>

> verify (PTRDIFF_MAX <= SIZE_MAX / 2);

> 

> 

>> +  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))

>> +    {

>> +      __set_errno (ENOMEM);

>> +      return NULL;

>> +    }

> 

> Why is the '- padsize' needed in these cases? All the relevant arithmetic is done using size_t, so overflow can't occur when PTRDIFF_MAX - padsize < bytes <= PTRDIFF_MAX, under the PTRDIFF_MAX <= SIZE_MAX / 2 assumption that the code is already making elsewhere.


I added for consistency so all internal allocation, after alignment and padding 
adjustment, will be also limited by PTRDIFF_MAX.  Although the idea is not to 
limit mmap/sbrk to such requirement I think it also helps sysmalloc to be more
conforming regarding the ptrdiff_t requirements.  However I did not made an
extensible analysis, so I don't have a strong opinion here.

> 

> 

>> +  if (INT_MULTIPLY_WRAPV (n, elem_size, &bytes)

>> +      || __glibc_unlikely (bytes > PTRDIFF_MAX - padsize))

> 

> This can be made a bit simpler by changing the type of this particular 'bytes' variable to ptrdiff_t and then by removing the "|| __glibc_unlikely (bytes > PTRDIFF_MAX - padsize)", since the "- padsize" isn't needed and the INT_MULTIPLY_WRAPV will check for exceeding PTRDIFF_MAX if 'bytes' is of type ptrdiff_t.
Paul Eggert Dec. 27, 2018, 4:25 p.m. UTC | #10 
Adhemerval Zanella wrote:
> I added for consistency so all internal allocation, after alignment and padding

> adjustment, will be also limited by PTRDIFF_MAX.


It's not a big deal, but as I see it the tradeoff here is:

* Limiting the size to (PTRDIFF_MAX - padsize) means that internal allocators 
are free to use ptrdiff_t in size calculation, e.g., when subtracting pointers. 
Currently I don't think they don't do that but maybe I'm wrong, or maybe they 
will do that someday, so it's safer to impose the stricter limit

* Merely limiting the size to PTRDIFF_MAX is easier to explain to users, and is 
a tiny bit faster because at the machine level it's faster to compare to 
0x7fff..ffff than it is to compare to 0x7fff...fff1 (or whatever) because you 
can just look at the sign bit.

I could see it going either way.
Adhemerval Zanella Dec. 27, 2018, 7:16 p.m. UTC | #11 
On 27/12/2018 14:25, Paul Eggert wrote:
> Adhemerval Zanella wrote:

>> I added for consistency so all internal allocation, after alignment and padding

>> adjustment, will be also limited by PTRDIFF_MAX.

> 

> It's not a big deal, but as I see it the tradeoff here is:

> 

> * Limiting the size to (PTRDIFF_MAX - padsize) means that internal allocators are free to use ptrdiff_t in size calculation, e.g., when subtracting pointers. Currently I don't think they don't do that but maybe I'm wrong, or maybe they will do that someday, so it's safer to impose the stricter limit

> 

> * Merely limiting the size to PTRDIFF_MAX is easier to explain to users, and is a tiny bit faster because at the machine level it's faster to compare to 0x7fff..ffff than it is to compare to 0x7fff...fff1 (or whatever) because you can just look at the sign bit.

> 

> I could see it going either way.


As Wilco has pointed out, internally sysmalloc will align up PTRDIFF_MAX - C
for C < pagesize() to PTRDIFF_MAX + 1. One option is to move the check size
on sysmalloc instead (before mmap/sbrk or morecore calls).
Florian Weimer Dec. 27, 2018, 7:39 p.m. UTC | #12 
I think this is more a kernel matter: The kernel should create a
PROT_NONE mapping spanning two pages in the middle of the address
space, for those architectures where this mapping can actually be
created (so not x86-64, for example).  This could be controlled by a
personality flag.  (It wouldn't have to be a PROT_NONE mapping, just
something that can only be overridden using MAP_FIXED.)

On the other hand, this is primarily a C/C++ language issue (and a GCC
problem), so maybe it's not appropriate to change kernel behavior
because userspace could execute code written in other languages and
compiled by other compilers.
Joseph Myers Dec. 31, 2018, 5:34 p.m. UTC | #13 
On Sat, 22 Dec 2018, Paul Eggert wrote:

> Joseph Myers wrote:

> > We can assume that

> > PTRDIFF_MAX == SIZE_MAX / 2 (and that ptrdiff_t, size_t and pointers have

> > the same power-of-2 width, which is at least 32).

> 

> OK, then we needn't bother with the static assertion. Is there a good place to

> document assumptions like this one, in the glibc manual I suppose?


I don't know of such a list of assumptions on the environment glibc 
supports.  I listed various other such requirements in 
<https://sourceware.org/ml/libc-alpha/2015-07/msg00761.html>.  (Of course, 
glibc code should still prefer to use logically appropriate interfaces to 
make the code clearer to the reader; for example, all glibc systems have 
size_t, ptrdiff_t, pointers and long the same width, but size_t, 
ptrdiff_t, intptr_t or uintptr_t should be used as appropriate instead of 
long if the intent is some kind of pointer-sized integer.)

> Although the point is now moot for this patch, I prefer the readability of

> 'verify'. Compare this:

> 

> verify (PTRDIFF_MAX <= SIZE_MAX / 2);

> 

> to this:

> 

> _Static_assert (PTRDIFF_MAX <= SIZE_MAX / 2,

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

> 

> With just one static assertion it's no big deal, but Gnulib has files with

> dozens and the readability savings add up.


In about three years' time we'll be able to require GCC 9 or later to 
build glibc and thus use single-argument _Static_assert (which C2x has 
added from C++ and which I've added support for to GCC 9).

-- 
Joseph S. Myers
joseph@codesourcery.com
diff mbox series

Patch

diff --git a/malloc/hooks.c b/malloc/hooks.c
index ae7305b036..b77c908e28 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -227,12 +227,6 @@  malloc_check (size_t sz, const void *caller)
 {
   void *victim;
 
-  if (sz + 1 == 0)
-    {
-      __set_errno (ENOMEM);
-      return NULL;
-    }
-
   __libc_lock_lock (main_arena.mutex);
   top_check ();
   victim = _int_malloc (&main_arena, sz + 1);
@@ -269,11 +263,6 @@  realloc_check (void *oldmem, size_t bytes, const void *caller)
   void *newmem = 0;
   unsigned char *magic_p;
 
-  if (bytes + 1 == 0)
-    {
-      __set_errno (ENOMEM);
-      return NULL;
-    }
   if (oldmem == 0)
     return malloc_check (bytes, NULL);
 
@@ -289,7 +278,7 @@  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);
+  nb = request2size (bytes + 1);
   __libc_lock_lock (main_arena.mutex);
 
   if (chunk_is_mmapped (oldp))
@@ -320,8 +309,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);
     }
 
@@ -362,13 +349,6 @@  memalign_check (size_t alignment, size_t bytes, const void *caller)
       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))
     {
diff --git a/malloc/malloc.c b/malloc/malloc.c
index c33709e966..bb80809983 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -247,6 +247,9 @@ 
 /* For SINGLE_THREAD_P.  */
 #include <sysdep-cancel.h>
 
+/* For INT_MULTIPLY_WRAPV.  */
+#include <intprops.h>
+
 /*
   Debugging:
 
@@ -1187,39 +1190,15 @@  nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   ((uintptr_t)(MALLOC_ALIGNMENT == 2 * SIZE_SZ ? (p) : chunk2mem (p)) \
    & MALLOC_ALIGN_MASK)
 
+/* pad request bytes into a usable size -- internal version */
 
-/*
-   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))
+#define padsize (SIZE_SZ + MALLOC_ALIGN_MASK)
 
-/* pad request bytes into a usable size -- internal version */
+#define request2size(req)                     \
+  (((req) + padsize < MINSIZE)  ?             \
+   MINSIZE :                                  \
+   ((req) + padsize) & ~MALLOC_ALIGN_MASK)
 
-#define request2size(req)                                         \
-  (((req) + SIZE_SZ + MALLOC_ALIGN_MASK < MINSIZE)  ?             \
-   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;			    \
-    }				    \
-})
 
 /*
    --------------- Physical chunk operations ---------------
@@ -3109,14 +3088,20 @@  __libc_malloc (size_t bytes)
   mstate ar_ptr;
   void *victim;
 
+  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
+    {
+      __set_errno (ENOMEM);
+      return NULL;
+    }
+
   void *(*hook) (size_t, const void *)
     = atomic_forced_read (__malloc_hook);
   if (__builtin_expect (hook != NULL, 0))
     return (*hook)(bytes, RETURN_ADDRESS (0));
+
 #if USE_TCACHE
   /* int_free also calls request2size, be careful to not pad twice.  */
-  size_t tbytes;
-  checked_request2size (bytes, tbytes);
+  size_t tbytes = request2size (bytes);
   size_t tc_idx = csize2tidx (tbytes);
 
   MAYBE_INIT_TCACHE ();
@@ -3213,6 +3198,12 @@  __libc_realloc (void *oldmem, size_t bytes)
 
   void *newp;             /* chunk to return */
 
+  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
+    {
+      __set_errno (ENOMEM);
+      return 0;
+    }
+
   void *(*hook) (void *, size_t, const void *) =
     atomic_forced_read (__realloc_hook);
   if (__builtin_expect (hook != NULL, 0))
@@ -3253,7 +3244,7 @@  __libc_realloc (void *oldmem, size_t bytes)
       && !DUMPED_MAIN_ARENA_CHUNK (oldp))
       malloc_printerr ("realloc(): invalid pointer");
 
-  checked_request2size (bytes, nb);
+  nb = request2size (bytes);
 
   if (chunk_is_mmapped (oldp))
     {
@@ -3342,6 +3333,12 @@  _mid_memalign (size_t alignment, size_t bytes, void *address)
   mstate ar_ptr;
   void *p;
 
+  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
+    {
+      __set_errno (ENOMEM);
+      return 0;
+    }
+
   void *(*hook) (size_t, size_t, const void *) =
     atomic_forced_read (__memalign_hook);
   if (__builtin_expect (hook != NULL, 0))
@@ -3363,14 +3360,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))
     {
@@ -3432,7 +3421,7 @@  __libc_pvalloc (size_t bytes)
   size_t rounded_bytes = ALIGN_UP (bytes, pagesize);
 
   /* Check for overflow.  */
-  if (bytes > SIZE_MAX - 2 * pagesize - MINSIZE)
+  if (__glibc_unlikely (bytes > PTRDIFF_MAX - 2 * pagesize - MINSIZE))
     {
       __set_errno (ENOMEM);
       return 0;
@@ -3452,24 +3441,18 @@  __libc_calloc (size_t n, size_t elem_size)
   unsigned long nclears;
   INTERNAL_SIZE_T *d;
 
-  /* 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 (INT_MULTIPLY_WRAPV (n, elem_size, &bytes)
+      || __glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
     {
-      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 +3460,6 @@  __libc_calloc (size_t n, size_t elem_size)
       return memset (mem, 0, sz);
     }
 
-  sz = bytes;
-
   MAYBE_INIT_TCACHE ();
 
   if (SINGLE_THREAD_P)
@@ -3621,16 +3602,7 @@  _int_malloc (mstate av, size_t bytes)
   size_t tcache_unsorted_count;	    /* count of unsorted chunks processed */
 #endif
 
-  /*
-     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
-     that are so large that they wrap around zero when padded and
-     aligned.
-   */
-
-  checked_request2size (bytes, nb);
+  nb = request2size (bytes);
 
   /* There are no usable arenas.  Fall back to sysmalloc to get a chunk from
      mmap.  */
@@ -4741,22 +4713,18 @@  _int_memalign (mstate av, size_t alignment, size_t bytes)
   unsigned long remainder_size;   /* its size */
   INTERNAL_SIZE_T size;
 
-
-
-  checked_request2size (bytes, nb);
+  if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize))
+    {
+      __set_errno (ENOMEM);
+      return 0;
+    }
 
   /*
      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;
-    }
+  nb = request2size (bytes);
 
   /* Call malloc with worst case padding to hit alignment. */
 
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..7e4721b78c 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 substraction within the object might overflow ptrdiff_t
+     resulting in undefined behaviour.  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..303c90ff11 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,13 +571,14 @@  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
 /* Allocate memory of SIZE bytes with an alignment of ALIGNMENT.  */
 extern int posix_memalign (void **__memptr, size_t __alignment, size_t __size)
-     __THROW __nonnull ((1)) __wur;
+     __THROW __nonnull ((1)) __attribute_alloc_size__ ((3)) __wur;
 #endif
 
 #ifdef __USE_ISOC11