This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: vfprintf typing problem


From: "Carlos O'Donell" <carlos@systemhalted.org>
Date: Sat, 31 Mar 2012 18:44:58 -0400

> Sorry, I should have said that the rest of the patch looks good to me.
> 
> Thanks for slogging through this.

Great, here's what I checked in:

--------------------
[PATCH] Tighten up vfprintf width, precision, and total length overflow handling.

	* 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.
	* 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.
---
 ChangeLog                   |   12 ++++++++++++
 stdio-common/bug22.c        |   28 ++++++++++++++++++++++++++--
 stdio-common/printf-parse.h |    6 ++++--
 stdio-common/vfprintf.c     |   33 +++++++++++++++++++++++++--------
 4 files changed, 67 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index dca8ed9..ca0ed63 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2012-03-31  David S. Miller  <davem@davemloft.net>
+
+	* 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.
+	* 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-03-31  Siddhesh Poyarekar  <siddhesh@redhat.com>
 
 	* resolv/nss_dns/dns-host.c: Merge copyright years.
diff --git a/stdio-common/bug22.c b/stdio-common/bug22.c
index 2228388..cb99dfe 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 != ERANGE)
+	  return 1;
+
+  ret = fprintf (fp, "%." SN "d", 1);
+  printf ("ret = %d\n", ret);
+  if (ret != -1 || errno != ERANGE)
+	  return 1;
+
+  ret = fprintf (fp, "%." SN3 "d", 1);
+  printf ("ret = %d\n", ret);
+  if (ret != -1 || errno != ERANGE)
+	  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..bcf914b 100644
--- a/stdio-common/printf-parse.h
+++ b/stdio-common/printf-parse.h
@@ -68,7 +68,7 @@ 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');
@@ -76,10 +76,12 @@ read_int (const UCHAR_T * *pstr)
   while (ISDIGIT (*++(*pstr)))
     {
       retval *= 10;
+      if (retval > INT_MAX)
+	return -1;
       retval += **pstr - L_('0');
     }
 
-  return retval;
+  return (int) retval;
 }
 #endif
 
diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index 1e90483..4287291 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)
@@ -1449,7 +1454,7 @@ 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);
 	    done = -1;
@@ -1481,7 +1486,8 @@ 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);
 	  done = -1;
@@ -1529,13 +1535,24 @@ 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 (ERANGE);
+	      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);
 	      done = -1;
-- 
1.7.9.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]