Malloc routines have overflow problems

[Jason Tishler]

Jeff,

On Tue, Aug 06, 2002 at 02:58:08PM -0400, J. Johnston wrote:
> Jason Tishler wrote:
> > My patch is a "superset" of Chris's and solves the overflow problem
> > in both malloc() and realloc().  Is this an acceptable solution?  If
> > so, then I will gladly supply a ChangeLog entry.  If not, what would
> > be?
> 
> A check should still be added because if sbrk is used as the
> underlying mechanism, it takes a signed argument.  If you roll over
> INT_MAX then you will be passing a negative value to sbrk and thereby
> asking to release storage.  A test could be added in malloc_extend_top
> to check against MORECORE_MAX which can be defaulted to INT_MAX.

Is the attached, hopefully less intrusive, patch more acceptable?  Or,
is it just more ugly? :,)

Thanks,
Jason
-------------- next part --------------
Index: mallocr.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdlib/mallocr.c,v
retrieving revision 1.6
diff -u -p -r1.6 mallocr.c
--- mallocr.c	13 Apr 2002 10:27:02 -0000	1.6
+++ mallocr.c	9 Aug 2002 16:15:18 -0000
@@ -271,6 +271,7 @@ extern "C" {
 #endif
 
 #include <stdio.h>    /* needed for malloc_stats */
+#include <limits.h>   /* needed for overflow checks */
 
 
 /*
@@ -1399,8 +1400,8 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-
 /* pad request bytes into a usable size */
 
 #define request2size(req) \
- (((long)((req) + (SIZE_SZ + MALLOC_ALIGN_MASK)) < \
-  (long)(MINSIZE + MALLOC_ALIGN_MASK)) ? ((MINSIZE + MALLOC_ALIGN_MASK) & ~(MALLOC_ALIGN_MASK)) : \
+ (((unsigned long)((req) + (SIZE_SZ + MALLOC_ALIGN_MASK)) < \
+  (unsigned long)(MINSIZE + MALLOC_ALIGN_MASK)) ? ((MINSIZE + MALLOC_ALIGN_MASK) & ~(MALLOC_ALIGN_MASK)) : \
    (((req) + (SIZE_SZ + MALLOC_ALIGN_MASK)) & ~(MALLOC_ALIGN_MASK)))
 
 /* Check if m has acceptable alignment */
@@ -2333,6 +2334,10 @@ Void_t* mALLOc(RARG bytes) RDECL size_t 
 
   INTERNAL_SIZE_T nb  = request2size(bytes);  /* padded request size; */
 
+  /* Check for overflow and just fail, if so. */
+  if (nb > INT_MAX)
+    return 0;
+
   MALLOC_LOCK;
 
   /* Check for exact match in a bin */
@@ -2791,6 +2796,10 @@ Void_t* rEALLOc(RARG oldmem, bytes) RDEC
 
 
   nb = request2size(bytes);
+
+  /* Check for overflow and just fail, if so. */
+  if (nb > INT_MAX)
+    return 0;
 
 #if HAVE_MMAP
   if (chunk_is_mmapped(oldp))