diff options
| -rw-r--r-- | ChangeLog | 16 | ||||
| -rw-r--r-- | stdio-common/bug22.c | 28 | ||||
| -rw-r--r-- | stdio-common/printf-parse.h | 23 | ||||
| -rw-r--r-- | stdio-common/printf-parsemb.c | 42 | ||||
| -rw-r--r-- | stdio-common/vfprintf.c | 77 |
5 files changed, 147 insertions, 39 deletions
@@ -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 2228388b47..efd9501368 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 72665dcec2..3aa0274249 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 2bdb5e65ab..a45ac74e06 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 1e904833a3..463f9c0062 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; } |