This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: vfprintf typing problem
- From: David Miller <davem at davemloft dot net>
- To: carlos at systemhalted dot org
- Cc: eggert at cs dot ucla dot edu, libc-alpha at sourceware dot org
- Date: Sat, 31 Mar 2012 21:06:42 -0400 (EDT)
- Subject: Re: vfprintf typing problem
- References: <CADZpyizqQWqsCqCkb8+yX+VGAQt4Zq8dCpVQDhzd-8-_YGzG7A@mail.gmail.com><20120331.182538.20629350961321931.davem@davemloft.net><CADZpyixPs415NPaWRc0NYqsTnE5b2MQJLg5H88m4GFtMderspg@mail.gmail.com>
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