malloc: make malloc fail with requests larger than PTRDIFF_MAX (BZ#23741)

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, and memalign) and limit
maximum requested allocation size to up PTRDIFF_MAX (taking into
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 the 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 issue mmap with values larger than
PTRDIFF_T depending on the requested size.

The __attribute_alloc_size__ is for functions that return a pointer only,
which means it cannot be applied to posix_memalign (see remarks in GCC
PR#87683 [5]). The runtimes checks to limit maximum requested allocation
size does applies to posix_memalign.

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

[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
[5] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87683

	[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 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): Use _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): 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 files changed, 48 insertions, 64 deletions

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 801ba1f499..0e3d4dd516 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,18 @@ 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 ---------------
@@ -3037,6 +3023,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))
@@ -3044,7 +3033,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 ();
@@ -3181,7 +3174,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))
     {
@@ -3291,13 +3288,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))
@@ -3357,14 +3347,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);
 }
@@ -3374,30 +3366,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;
@@ -3405,8 +3391,6 @@ __libc_calloc (size_t n, size_t elem_size)
       return memset (mem, 0, sz);
     }
 
-  sz = bytes;
-
   MAYBE_INIT_TCACHE ();
 
   if (SINGLE_THREAD_P)
@@ -3553,12 +3537,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.  */
@@ -4680,21 +4668,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));