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@davemloft.net>
Date: Wed, 28 Mar 2012 19:26:04 -0400 (EDT)
> From: Roland McGrath <roland@hack.frob.com>
> Date: Wed, 28 Mar 2012 15:25:24 -0700 (PDT)
>
>> In the C standard (C99 and C11), I don't see anything that says anything
>> one way or another about the valid range of field width specifiers when
>> they appear directly in the format string.
>
> See the comments in the glibc bug #5424 fixed by bug22.c, it
> references:
>
> http://www.opengroup.org/platform/resolutions/bwg98-006.html
>
> and also recent POSIX drafts.
I propose the following patch.
At the points where the "(size_t) -1 / sizeof(CHAT_T)" checks exist
the 'width' has been "normalized" to be positive. If it was negative,
that state has been recorded in the 'left' variable.
So we can just directly sanity check whether width (plus the 32-bytes
of slack) is >= INT_MAX.
Therefore we have two levels of protection in this code:
1) Individual field widths are validated against INT_MAX
2) The total result (stored in 'done') is validated against
overflowing INT_MAX as it accumulates, via the done_add() macro.
I've also adjusted bug22.c so that it tests both of those cases.
2012-03-30 David S. Miller <davem@davemloft.net>
* stdio-common/vfprintf.c (vfprintf): Validate width against
overflow of INT_MAX. Set errno to EOVERFLOW when 'done' overflows
INT_MAX.
* stdio-common/bug22.c: Adjust to test both width INT_MAX overflow
as well as total length INT_MAX overflow. Check explicitly for
proper errno values.
diff --git a/stdio-common/bug22.c b/stdio-common/bug22.c
index 2228388..6b6bb43 100644
--- a/stdio-common/bug22.c
+++ b/stdio-common/bug22.c
@@ -1,12 +1,18 @@
/* BZ #5424 */
#include <stdio.h>
+#include <errno.h>
+/* INT_MAX + 1 */
#define N 2147483648
+/* (INT_MAX / 2) + 2 */
+#define N2 1073741825
+
#define STRINGIFY(S) #S
#define MAKE_STR(S) STRINGIFY(S)
#define SN MAKE_STR(N)
+#define SN2 MAKE_STR(N2)
static int
do_test (void)
@@ -20,11 +26,15 @@ 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, "%" 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/vfprintf.c b/stdio-common/vfprintf.c
index 1e90483..4875fe0 100644
--- a/stdio-common/vfprintf.c
+++ b/stdio-common/vfprintf.c
@@ -71,6 +71,7 @@
< _val, 0)) \
{ \
done = -1; \
+ __set_errno (EOVERFLOW); \
goto all_done; \
} \
done += _val; \
@@ -141,12 +142,18 @@
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 ((unsigned int) INT_MAX - (unsigned int) done \
+ < (Len), 0)) \
+ { \
+ done = -1; \
+ __set_errno (EOVERFLOW); \
+ goto all_done; \
+ } \
done += (Len); \
} \
while (0)
@@ -1449,7 +1456,8 @@ 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 ((unsigned int) width >= INT_MAX
+ / sizeof (CHAR_T) - 32, 0))
{
__set_errno (ERANGE);
done = -1;
@@ -1481,7 +1489,7 @@ 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 ((unsigned int) width >= INT_MAX / sizeof (CHAR_T) - 32, 0))
{
__set_errno (ERANGE);
done = -1;