Tighten up vfprintf width, precision, and total length overflow handling.

	With help from Paul Eggert, Carlos O'Donell, and Roland McGrath.
	* stdio-common/printf-parse.h (read_int): Change return type to
	'int', return -1 on INT_MAX overflow.
	* stdio-common/vfprintf.c (vfprintf): Validate width and precision
	against overflow of INT_MAX.  Set errno to EOVERFLOW when 'done'
	overflows INT_MAX.  Check for overflow of in-format-string precision
	values properly.  Use EOVERFLOW rather than ERANGE throughout.  Use
	SIZE_MAX not INT_MAX for integer overflow test.
	* stdio-common/printf-parsemb.c: If read_int signals an overflow,
	skip the construct in the format string but do not record anything.
	* stdio-common/bug22.c: Adjust to test both width/prevision
	INT_MAX overflow as well as total length INT_MAX overflow.  Check
	explicitly for proper errno values.

5 files changed, 147 insertions, 39 deletions

diff --git a/ChangeLog b/ChangeLog
index ca85e54..8705429 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2012-04-02  David S. Miller  <davem@davemloft.net>
+
+	With help from Paul Eggert, Carlos O'Donell, and Roland McGrath.
+	* stdio-common/printf-parse.h (read_int): Change return type to
+	'int', return -1 on INT_MAX overflow.
+	* stdio-common/vfprintf.c (vfprintf): Validate width and precision
+	against overflow of INT_MAX.  Set errno to EOVERFLOW when 'done'
+	overflows INT_MAX.  Check for overflow of in-format-string precision
+	values properly.  Use EOVERFLOW rather than ERANGE throughout.  Use
+	SIZE_MAX not INT_MAX for integer overflow test.
+	* stdio-common/printf-parsemb.c: If read_int signals an overflow,
+	skip the construct in the format string but do not record anything.
+	* stdio-common/bug22.c: Adjust to test both width/prevision
+	INT_MAX overflow as well as total length INT_MAX overflow.  Check
+	explicitly for proper errno values.
+
 2012-04-02  Thomas Schwinge  <thomas@codesourcery.com>
 
 	* string/test-memcmp.c [! WIDE]: #include <limits.h> for CHAR_MIN,
diff --git a/stdio-common/bug22.c b/stdio-common/bug22.c
index 2228388..efd9501 100644
--- a/stdio-common/bug22.c
+++ b/stdio-common/bug22.c
@@ -1,12 +1,22 @@
 /* BZ #5424 */
 #include <stdio.h>
+#include <errno.h>
 
+/* INT_MAX + 1 */
 #define N 2147483648
 
+/* (INT_MAX / 2) + 2 */
+#define N2 1073741825
+
+/* INT_MAX - 3 */
+#define N3 2147483644
+
 #define STRINGIFY(S) #S
 #define MAKE_STR(S) STRINGIFY(S)
 
 #define SN MAKE_STR(N)
+#define SN2 MAKE_STR(N2)
+#define SN3 MAKE_STR(N3)
 
 static int
 do_test (void)
@@ -20,11 +30,25 @@ do_test (void)
       return 1;
     }
 
-  ret = fprintf (fp, "%" SN "d%" SN "d", 1, 1);
+  ret = fprintf (fp, "%" SN "d", 1);
+  printf ("ret = %d\n", ret);
+  if (ret != -1 || errno != EOVERFLOW)
+	  return 1;
+
+  ret = fprintf (fp, "%." SN "d", 1);
+  printf ("ret = %d\n", ret);
+  if (ret != -1 || errno != EOVERFLOW)
+	  return 1;
+
+  ret = fprintf (fp, "%." SN3 "d", 1);
+  printf ("ret = %d\n", ret);
+  if (ret != -1 || errno != EOVERFLOW)
+	  return 1;
 
+  ret = fprintf (fp, "%" SN2 "d%" SN2 "d", 1, 1);
   printf ("ret = %d\n", ret);
 
-  return ret != -1;
+  return ret != -1 || errno != EOVERFLOW;
 }
 
 #define TIMEOUT 30
diff --git a/stdio-common/printf-parse.h b/stdio-common/printf-parse.h
index 72665dc..3aa0274 100644
--- a/stdio-common/printf-parse.h
+++ b/stdio-common/printf-parse.h
@@ -68,16 +68,27 @@ union printf_arg
 #ifndef DONT_NEED_READ_INT
 /* Read a simple integer from a string and update the string pointer.
    It is assumed that the first character is a digit.  */
-static unsigned int
+static int
 read_int (const UCHAR_T * *pstr)
 {
-  unsigned int retval = **pstr - L_('0');
+  int retval = **pstr - L_('0');
 
   while (ISDIGIT (*++(*pstr)))
-    {
-      retval *= 10;
-      retval += **pstr - L_('0');
-    }
+    if (retval >= 0)
+      {
+	if (INT_MAX / 10 < retval)
+	  retval = -1;
+	else
+	  {
+	    int digit = **pstr - L_('0');
+
+	    retval *= 10;
+	    if (INT_MAX - digit < retval)
+	      retval = -1;
+	    else
+	      retval += digit;
+	  }
+      }
 
   return retval;
 }
diff --git a/stdio-common/printf-parsemb.c b/stdio-common/printf-parsemb.c
index 2bdb5e6..a45ac74 100644
--- a/stdio-common/printf-parsemb.c
+++ b/stdio-common/printf-parsemb.c
@@ -87,12 +87,15 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
 
       n = read_int (&format);
 
-      if (n > 0 && *format == L_('$'))
+      if (n != 0 && *format == L_('$'))
 	/* Is positional parameter.  */
 	{
 	  ++format;		/* Skip the '$'.  */
-	  spec->data_arg = n - 1;
-	  *max_ref_arg = MAX (*max_ref_arg, n);
+	  if (n != -1)
+	    {
+	      spec->data_arg = n - 1;
+	      *max_ref_arg = MAX (*max_ref_arg, n);
+	    }
 	}
       else
 	/* Oops; that was actually the width and/or 0 padding flag.
@@ -160,10 +163,13 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
 	  /* The width argument might be found in a positional parameter.  */
 	  n = read_int (&format);
 
-	  if (n > 0 && *format == L_('$'))
+	  if (n != 0 && *format == L_('$'))
 	    {
-	      spec->width_arg = n - 1;
-	      *max_ref_arg = MAX (*max_ref_arg, n);
+	      if (n != -1)
+		{
+		  spec->width_arg = n - 1;
+		  *max_ref_arg = MAX (*max_ref_arg, n);
+		}
 	      ++format;		/* Skip '$'.  */
 	    }
 	}
@@ -177,9 +183,13 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
 	}
     }
   else if (ISDIGIT (*format))
-    /* Constant width specification.  */
-    spec->info.width = read_int (&format);
+    {
+      int n = read_int (&format);
 
+      /* Constant width specification.  */
+      if (n != -1)
+	spec->info.width = n;
+    }
   /* Get the precision.  */
   spec->prec_arg = -1;
   /* -1 means none given; 0 means explicit 0.  */
@@ -196,10 +206,13 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
 	    {
 	      n = read_int (&format);
 
-	      if (n > 0 && *format == L_('$'))
+	      if (n != 0 && *format == L_('$'))
 		{
-		  spec->prec_arg = n - 1;
-		  *max_ref_arg = MAX (*max_ref_arg, n);
+		  if (n != -1)
+		    {
+		      spec->prec_arg = n - 1;
+		      *max_ref_arg = MAX (*max_ref_arg, n);
+		    }
 		  ++format;
 		}
 	    }
@@ -213,7 +226,12 @@ __parse_one_specmb (const UCHAR_T *format, size_t posn,
 	    }
 	}
       else if (ISDIGIT (*format))
-	spec->info.prec = read_int (&format);
+	{
+	  int n = read_int (&format);
+
+	  if (n != -1)
+	    spec->info.prec = n;
+	}
       else
 	/* "%.?" is treated like "%.0?".  */
 	spec->info.prec = 0;
diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index 1e90483..463f9c0 100644
--- a/stdio-common/vfprintf.c
+++ b/stdio-common/vfprintf.c
@@ -67,10 +67,10 @@
   do {									      \
     unsigned int _val = val;						      \
     assert ((unsigned int) done < (unsigned int) INT_MAX);		      \
-    if (__builtin_expect ((unsigned int) INT_MAX - (unsigned int) done	      \
-			  < _val, 0))					      \
+    if (__builtin_expect (INT_MAX - done < _val, 0))			      \
       {									      \
 	done = -1;							      \
+	 __set_errno (EOVERFLOW);					      \
 	goto all_done;							      \
       }									      \
     done += _val;							      \
@@ -141,12 +141,17 @@
   do									      \
     {									      \
       assert ((size_t) done <= (size_t) INT_MAX);			      \
-      if ((size_t) PUT (s, (String), (Len)) != (size_t) (Len)		      \
-	  || (size_t) INT_MAX - (size_t) done < (size_t) (Len))		      \
+      if ((size_t) PUT (s, (String), (Len)) != (size_t) (Len))		      \
 	{								      \
 	  done = -1;							      \
 	  goto all_done;						      \
 	}								      \
+      if (__builtin_expect (INT_MAX - done < (Len), 0))			      \
+      {									      \
+	done = -1;							      \
+	 __set_errno (EOVERFLOW);					      \
+	goto all_done;							      \
+      }									      \
       done += (Len);							      \
     }									      \
   while (0)
@@ -1435,10 +1440,21 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 	const UCHAR_T *tmp;	/* Temporary value.  */
 
 	tmp = ++f;
-	if (ISDIGIT (*tmp) && read_int (&tmp) && *tmp == L_('$'))
-	  /* The width comes from a positional parameter.  */
-	  goto do_positional;
+	if (ISDIGIT (*tmp))
+	  {
+	    int pos = read_int (&tmp);
 
+	    if (pos == -1)
+	      {
+		__set_errno (EOVERFLOW);
+		done = -1;
+		goto all_done;
+	      }
+
+	    if (pos && *tmp == L_('$'))
+	      /* The width comes from a positional parameter.  */
+	      goto do_positional;
+	  }
 	width = va_arg (ap, int);
 
 	/* Negative width means left justified.  */
@@ -1449,9 +1465,9 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 	    left = 1;
 	  }
 
-	if (__builtin_expect (width >= (size_t) -1 / sizeof (CHAR_T) - 32, 0))
+	if (__builtin_expect (width >= INT_MAX / sizeof (CHAR_T) - 32, 0))
 	  {
-	    __set_errno (ERANGE);
+	    __set_errno (EOVERFLOW);
 	    done = -1;
 	    goto all_done;
 	  }
@@ -1481,9 +1497,10 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
     LABEL (width):
       width = read_int (&f);
 
-      if (__builtin_expect (width >= (size_t) -1 / sizeof (CHAR_T) - 32, 0))
+      if (__builtin_expect (width == -1
+			    || width >= INT_MAX / sizeof (CHAR_T) - 32, 0))
 	{
-	  __set_errno (ERANGE);
+	  __set_errno (EOVERFLOW);
 	  done = -1;
 	  goto all_done;
 	}
@@ -1518,10 +1535,21 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 	  const UCHAR_T *tmp;	/* Temporary value.  */
 
 	  tmp = ++f;
-	  if (ISDIGIT (*tmp) && read_int (&tmp) > 0 && *tmp == L_('$'))
-	    /* The precision comes from a positional parameter.  */
-	    goto do_positional;
+	  if (ISDIGIT (*tmp))
+	    {
+	      int pos = read_int (&tmp);
+
+	      if (pos == -1)
+		{
+		  __set_errno (EOVERFLOW);
+		  done = -1;
+		  goto all_done;
+		}
 
+	      if (pos && *tmp == L_('$'))
+		/* The precision comes from a positional parameter.  */
+		goto do_positional;
+	    }
 	  prec = va_arg (ap, int);
 
 	  /* If the precision is negative the precision is omitted.  */
@@ -1529,15 +1557,26 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 	    prec = -1;
 	}
       else if (ISDIGIT (*f))
-	prec = read_int (&f);
+	{
+	  prec = read_int (&f);
+
+	  /* The precision was specified in this case as an extremely
+	     large positive value.  */
+	  if (prec == -1)
+	    {
+	      __set_errno (EOVERFLOW);
+	      done = -1;
+	      goto all_done;
+	    }
+	}
       else
 	prec = 0;
       if (prec > width
 	  && prec > sizeof (work_buffer) / sizeof (work_buffer[0]) - 32)
 	{
-	  if (__builtin_expect (prec >= (size_t) -1 / sizeof (CHAR_T) - 32, 0))
+	  if (__builtin_expect (prec >= INT_MAX / sizeof (CHAR_T) - 32, 0))
 	    {
-	      __set_errno (ERANGE);
+	      __set_errno (EOVERFLOW);
 	      done = -1;
 	      goto all_done;
 	    }
@@ -1710,9 +1749,9 @@ do_positional:
 		     + sizeof (*args_type));
 
     /* Check for potential integer overflow.  */
-    if (__builtin_expect (nargs > SIZE_MAX / bytes_per_arg, 0))
+    if (__builtin_expect (nargs > INT_MAX / bytes_per_arg, 0))
       {
-	 __set_errno (ERANGE);
+	 __set_errno (EOVERFLOW);
 	 done = -1;
 	 goto all_done;
       }