1d4c55
commit e1c0c00cc2bdd147bfcf362ada1443bee90465ec
1d4c55
Author: Joseph Myers <joseph@codesourcery.com>
1d4c55
Date:   Tue Jul 7 14:54:12 2020 +0000
1d4c55
1d4c55
    Remove most vfprintf width/precision-dependent allocations (bug 14231, bug 26211).
1d4c55
    
1d4c55
    The vfprintf implementation (used for all printf-family functions)
1d4c55
    contains complicated logic to allocate internal buffers of a size
1d4c55
    depending on the width and precision used for a format, using either
1d4c55
    malloc or alloca depending on that size, and with consequent checks
1d4c55
    for size overflow and allocation failure.
1d4c55
    
1d4c55
    As noted in bug 26211, the version of that logic used when '$' plus
1d4c55
    argument number formats are in use is missing the overflow checks,
1d4c55
    which can result in segfaults (quite possibly exploitable, I didn't
1d4c55
    try to work that out) when the width or precision is in the range
1d4c55
    0x7fffffe0 through 0x7fffffff (maybe smaller values as well in the
1d4c55
    wprintf case on 32-bit systems, when the multiplication by sizeof
1d4c55
    (CHAR_T) can overflow).
1d4c55
    
1d4c55
    All that complicated logic in fact appears to be useless.  As far as I
1d4c55
    can tell, there has been no need (outside the floating-point printf
1d4c55
    code, which does its own allocations) for allocations depending on
1d4c55
    width or precision since commit
1d4c55
    3e95f6602b226e0de06aaff686dc47b282d7cc16 ("Remove limitation on size
1d4c55
    of precision for integers", Sun Sep 12 21:23:32 1999 +0000).  Thus,
1d4c55
    this patch removes that logic completely, thereby fixing both problems
1d4c55
    with excessive allocations for large width and precision for
1d4c55
    non-floating-point formats, and the problem with missing overflow
1d4c55
    checks with such allocations.  Note that this does have the
1d4c55
    consequence that width and precision up to INT_MAX are now allowed
1d4c55
    where previously INT_MAX / sizeof (CHAR_T) - EXTSIZ or more would have
1d4c55
    been rejected, so could potentially expose any other overflows where
1d4c55
    the value would previously have been rejected by those removed checks.
1d4c55
    
1d4c55
    I believe this completely fixes bugs 14231 and 26211.
1d4c55
    
1d4c55
    Excessive allocations are still possible in the floating-point case
1d4c55
    (bug 21127), as are other integer or buffer overflows (see bug 26201).
1d4c55
    This does not address the cases where a precision larger than INT_MAX
1d4c55
    (embedded in the format string) would be meaningful without printf's
1d4c55
    return value overflowing (when it's used with a string format, or %g
1d4c55
    without the '#' flag, so the actual output will be much smaller), as
1d4c55
    mentioned in bug 17829 comment 8; using size_t internally for
1d4c55
    precision to handle that case would be complicated by struct
1d4c55
    printf_info being a public ABI.  Nor does it address the matter of an
1d4c55
    INT_MIN width being negated (bug 17829 comment 7; the same logic
1d4c55
    appears a second time in the file as well, in the form of multiplying
1d4c55
    by -1).  There may be other sources of memory allocations with malloc
1d4c55
    in printf functions as well (bug 24988, bug 16060).  From inspection,
1d4c55
    I think there are also integer overflows in two copies of "if ((width
1d4c55
    -= len) < 0)" logic (where width is int, len is size_t and a very long
1d4c55
    string could result in spurious padding being output on a 32-bit
1d4c55
    system before printf overflows the count of output characters).
1d4c55
    
1d4c55
    Tested for x86-64 and x86.
1d4c55
    
1d4c55
    (cherry picked from commit 6caddd34bd7ffb5ac4f36c8e036eee100c2cc535)
1d4c55
1d4c55
diff --git a/stdio-common/Makefile b/stdio-common/Makefile
1d4c55
index 51062a7dbf698931..d76b47bd5f932f69 100644
1d4c55
--- a/stdio-common/Makefile
1d4c55
+++ b/stdio-common/Makefile
1d4c55
@@ -64,6 +64,7 @@ tests := tstscanf test_rdwr test-popen tstgetln test-fseek \
1d4c55
 	 tst-scanf-round \
1d4c55
 	 tst-renameat2 \
1d4c55
 	 tst-printf-bz25691 \
1d4c55
+	 tst-vfprintf-width-prec-alloc
1d4c55
 
1d4c55
 test-srcs = tst-unbputc tst-printf tst-printfsz-islongdouble
1d4c55
 
1d4c55
diff --git a/stdio-common/bug22.c b/stdio-common/bug22.c
1d4c55
index b26399acb7dfc775..e12b01731e1b4ac8 100644
1d4c55
--- a/stdio-common/bug22.c
1d4c55
+++ b/stdio-common/bug22.c
1d4c55
@@ -42,7 +42,7 @@ do_test (void)
1d4c55
 
1d4c55
   ret = fprintf (fp, "%." SN3 "d", 1);
1d4c55
   printf ("ret = %d\n", ret);
1d4c55
-  if (ret != -1 || errno != EOVERFLOW)
1d4c55
+  if (ret != N3)
1d4c55
 	  return 1;
1d4c55
 
1d4c55
   ret = fprintf (fp, "%" SN2 "d%" SN2 "d", 1, 1);
1d4c55
diff --git a/stdio-common/tst-vfprintf-width-prec-alloc.c b/stdio-common/tst-vfprintf-width-prec-alloc.c
1d4c55
new file mode 100644
1d4c55
index 0000000000000000..0a74b53a3389d699
1d4c55
--- /dev/null
1d4c55
+++ b/stdio-common/tst-vfprintf-width-prec-alloc.c
1d4c55
@@ -0,0 +1,41 @@
1d4c55
+/* Test large width or precision does not involve large allocation.
1d4c55
+   Copyright (C) 2020 Free Software Foundation, Inc.
1d4c55
+   This file is part of the GNU C Library.
1d4c55
+
1d4c55
+   The GNU C Library is free software; you can redistribute it and/or
1d4c55
+   modify it under the terms of the GNU Lesser General Public
1d4c55
+   License as published by the Free Software Foundation; either
1d4c55
+   version 2.1 of the License, or (at your option) any later version.
1d4c55
+
1d4c55
+   The GNU C Library is distributed in the hope that it will be useful,
1d4c55
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
1d4c55
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
1d4c55
+   Lesser General Public License for more details.
1d4c55
+
1d4c55
+   You should have received a copy of the GNU Lesser General Public
1d4c55
+   License along with the GNU C Library; if not, see
1d4c55
+   <https://www.gnu.org/licenses/>.  */
1d4c55
+
1d4c55
+#include <stdio.h>
1d4c55
+#include <sys/resource.h>
1d4c55
+#include <support/check.h>
1d4c55
+
1d4c55
+char test_string[] = "test";
1d4c55
+
1d4c55
+static int
1d4c55
+do_test (void)
1d4c55
+{
1d4c55
+  struct rlimit limit;
1d4c55
+  TEST_VERIFY_EXIT (getrlimit (RLIMIT_AS, &limit) == 0);
1d4c55
+  limit.rlim_cur = 200 * 1024 * 1024;
1d4c55
+  TEST_VERIFY_EXIT (setrlimit (RLIMIT_AS, &limit) == 0);
1d4c55
+  FILE *fp = fopen ("/dev/null", "w");
1d4c55
+  TEST_VERIFY_EXIT (fp != NULL);
1d4c55
+  TEST_COMPARE (fprintf (fp, "%1000000000d", 1), 1000000000);
1d4c55
+  TEST_COMPARE (fprintf (fp, "%.1000000000s", test_string), 4);
1d4c55
+  TEST_COMPARE (fprintf (fp, "%1000000000d %1000000000d", 1, 2), 2000000001);
1d4c55
+  TEST_COMPARE (fprintf (fp, "%2$.*1$s", 0x7fffffff, test_string), 4);
1d4c55
+  return 0;
1d4c55
+}
1d4c55
+
1d4c55
+#include <support/test-driver.c>
1d4c55
diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
1d4c55
index dab56b6ba2c7bdbe..6b83ba91a12cdcd5 100644
1d4c55
--- a/stdio-common/vfprintf.c
1d4c55
+++ b/stdio-common/vfprintf.c
1d4c55
@@ -42,10 +42,6 @@
1d4c55
 
1d4c55
 #include <libioP.h>
1d4c55
 
1d4c55
-/* In some cases we need extra space for all the output which is not
1d4c55
-   counted in the width of the string. We assume 32 characters is
1d4c55
-   enough.  */
1d4c55
-#define EXTSIZ		32
1d4c55
 #define ARGCHECK(S, Format) \
1d4c55
   do									      \
1d4c55
     {									      \
1d4c55
@@ -1295,7 +1291,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
1d4c55
 
1d4c55
   /* Buffer intermediate results.  */
1d4c55
   CHAR_T work_buffer[WORK_BUFFER_SIZE];
1d4c55
-  CHAR_T *workstart = NULL;
1d4c55
   CHAR_T *workend;
1d4c55
 
1d4c55
   /* We have to save the original argument pointer.  */
1d4c55
@@ -1404,7 +1399,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
1d4c55
       UCHAR_T pad = L_(' ');/* Padding character.  */
1d4c55
       CHAR_T spec;
1d4c55
 
1d4c55
-      workstart = NULL;
1d4c55
       workend = work_buffer + WORK_BUFFER_SIZE;
1d4c55
 
1d4c55
       /* Get current character in format string.  */
1d4c55
@@ -1496,31 +1490,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
1d4c55
 	    pad = L_(' ');
1d4c55
 	    left = 1;
1d4c55
 	  }
1d4c55
-
1d4c55
-	if (__glibc_unlikely (width >= INT_MAX / sizeof (CHAR_T) - EXTSIZ))
1d4c55
-	  {
1d4c55
-	    __set_errno (EOVERFLOW);
1d4c55
-	    done = -1;
1d4c55
-	    goto all_done;
1d4c55
-	  }
1d4c55
-
1d4c55
-	if (width >= WORK_BUFFER_SIZE - EXTSIZ)
1d4c55
-	  {
1d4c55
-	    /* We have to use a special buffer.  */
1d4c55
-	    size_t needed = ((size_t) width + EXTSIZ) * sizeof (CHAR_T);
1d4c55
-	    if (__libc_use_alloca (needed))
1d4c55
-	      workend = (CHAR_T *) alloca (needed) + width + EXTSIZ;
1d4c55
-	    else
1d4c55
-	      {
1d4c55
-		workstart = (CHAR_T *) malloc (needed);
1d4c55
-		if (workstart == NULL)
1d4c55
-		  {
1d4c55
-		    done = -1;
1d4c55
-		    goto all_done;
1d4c55
-		  }
1d4c55
-		workend = workstart + width + EXTSIZ;
1d4c55
-	      }
1d4c55
-	  }
1d4c55
       }
1d4c55
       JUMP (*f, step1_jumps);
1d4c55
 
1d4c55
@@ -1528,31 +1497,13 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
1d4c55
     LABEL (width):
1d4c55
       width = read_int (&f);
1d4c55
 
1d4c55
-      if (__glibc_unlikely (width == -1
1d4c55
-			    || width >= INT_MAX / sizeof (CHAR_T) - EXTSIZ))
1d4c55
+      if (__glibc_unlikely (width == -1))
1d4c55
 	{
1d4c55
 	  __set_errno (EOVERFLOW);
1d4c55
 	  done = -1;
1d4c55
 	  goto all_done;
1d4c55
 	}
1d4c55
 
1d4c55
-      if (width >= WORK_BUFFER_SIZE - EXTSIZ)
1d4c55
-	{
1d4c55
-	  /* We have to use a special buffer.  */
1d4c55
-	  size_t needed = ((size_t) width + EXTSIZ) * sizeof (CHAR_T);
1d4c55
-	  if (__libc_use_alloca (needed))
1d4c55
-	    workend = (CHAR_T *) alloca (needed) + width + EXTSIZ;
1d4c55
-	  else
1d4c55
-	    {
1d4c55
-	      workstart = (CHAR_T *) malloc (needed);
1d4c55
-	      if (workstart == NULL)
1d4c55
-		{
1d4c55
-		  done = -1;
1d4c55
-		  goto all_done;
1d4c55
-		}
1d4c55
-	      workend = workstart + width + EXTSIZ;
1d4c55
-	    }
1d4c55
-	}
1d4c55
       if (*f == L_('$'))
1d4c55
 	/* Oh, oh.  The argument comes from a positional parameter.  */
1d4c55
 	goto do_positional;
1d4c55
@@ -1601,34 +1552,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
1d4c55
 	}
1d4c55
       else
1d4c55
 	prec = 0;
1d4c55
-      if (prec > width && prec > WORK_BUFFER_SIZE - EXTSIZ)
1d4c55
-	{
1d4c55
-	  /* Deallocate any previously allocated buffer because it is
1d4c55
-	     too small.  */
1d4c55
-	  if (__glibc_unlikely (workstart != NULL))
1d4c55
-	    free (workstart);
1d4c55
-	  workstart = NULL;
1d4c55
-	  if (__glibc_unlikely (prec >= INT_MAX / sizeof (CHAR_T) - EXTSIZ))
1d4c55
-	    {
1d4c55
-	      __set_errno (EOVERFLOW);
1d4c55
-	      done = -1;
1d4c55
-	      goto all_done;
1d4c55
-	    }
1d4c55
-	  size_t needed = ((size_t) prec + EXTSIZ) * sizeof (CHAR_T);
1d4c55
-
1d4c55
-	  if (__libc_use_alloca (needed))
1d4c55
-	    workend = (CHAR_T *) alloca (needed) + prec + EXTSIZ;
1d4c55
-	  else
1d4c55
-	    {
1d4c55
-	      workstart = (CHAR_T *) malloc (needed);
1d4c55
-	      if (workstart == NULL)
1d4c55
-		{
1d4c55
-		  done = -1;
1d4c55
-		  goto all_done;
1d4c55
-		}
1d4c55
-	      workend = workstart + prec + EXTSIZ;
1d4c55
-	    }
1d4c55
-	}
1d4c55
       JUMP (*f, step2_jumps);
1d4c55
 
1d4c55
       /* Process 'h' modifier.  There might another 'h' following.  */
1d4c55
@@ -1692,10 +1615,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
1d4c55
       /* The format is correctly handled.  */
1d4c55
       ++nspecs_done;
1d4c55
 
1d4c55
-      if (__glibc_unlikely (workstart != NULL))
1d4c55
-	free (workstart);
1d4c55
-      workstart = NULL;
1d4c55
-
1d4c55
       /* Look for next format specifier.  */
1d4c55
 #ifdef COMPILE_WPRINTF
1d4c55
       f = __find_specwc ((end_of_spec = ++f));
1d4c55
@@ -1713,18 +1632,11 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
1d4c55
 
1d4c55
   /* Hand off processing for positional parameters.  */
1d4c55
 do_positional:
1d4c55
-  if (__glibc_unlikely (workstart != NULL))
1d4c55
-    {
1d4c55
-      free (workstart);
1d4c55
-      workstart = NULL;
1d4c55
-    }
1d4c55
   done = printf_positional (s, format, readonly_format, ap, &ap_save,
1d4c55
 			    done, nspecs_done, lead_str_end, work_buffer,
1d4c55
 			    save_errno, grouping, thousands_sep);
1d4c55
 
1d4c55
  all_done:
1d4c55
-  if (__glibc_unlikely (workstart != NULL))
1d4c55
-    free (workstart);
1d4c55
   /* Unlock the stream.  */
1d4c55
   _IO_funlockfile (s);
1d4c55
   _IO_cleanup_region_end (0);
1d4c55
@@ -1767,8 +1679,6 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
1d4c55
   /* Just a counter.  */
1d4c55
   size_t cnt;
1d4c55
 
1d4c55
-  CHAR_T *workstart = NULL;
1d4c55
-
1d4c55
   if (grouping == (const char *) -1)
1d4c55
     {
1d4c55
 #ifdef COMPILE_WPRINTF
1d4c55
@@ -1957,7 +1867,6 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
1d4c55
       char pad = specs[nspecs_done].info.pad;
1d4c55
       CHAR_T spec = specs[nspecs_done].info.spec;
1d4c55
 
1d4c55
-      workstart = NULL;
1d4c55
       CHAR_T *workend = work_buffer + WORK_BUFFER_SIZE;
1d4c55
 
1d4c55
       /* Fill in last information.  */
1d4c55
@@ -1991,27 +1900,6 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
1d4c55
 	  prec = specs[nspecs_done].info.prec;
1d4c55
 	}
1d4c55
 
1d4c55
-      /* Maybe the buffer is too small.  */
1d4c55
-      if (MAX (prec, width) + EXTSIZ > WORK_BUFFER_SIZE)
1d4c55
-	{
1d4c55
-	  if (__libc_use_alloca ((MAX (prec, width) + EXTSIZ)
1d4c55
-				 * sizeof (CHAR_T)))
1d4c55
-	    workend = ((CHAR_T *) alloca ((MAX (prec, width) + EXTSIZ)
1d4c55
-					  * sizeof (CHAR_T))
1d4c55
-		       + (MAX (prec, width) + EXTSIZ));
1d4c55
-	  else
1d4c55
-	    {
1d4c55
-	      workstart = (CHAR_T *) malloc ((MAX (prec, width) + EXTSIZ)
1d4c55
-					     * sizeof (CHAR_T));
1d4c55
-	      if (workstart == NULL)
1d4c55
-		{
1d4c55
-		  done = -1;
1d4c55
-		  goto all_done;
1d4c55
-		}
1d4c55
-	      workend = workstart + (MAX (prec, width) + EXTSIZ);
1d4c55
-	    }
1d4c55
-	}
1d4c55
-
1d4c55
       /* Process format specifiers.  */
1d4c55
       while (1)
1d4c55
 	{
1d4c55
@@ -2085,18 +1973,12 @@ printf_positional (FILE *s, const CHAR_T *format, int readonly_format,
1d4c55
 	  break;
1d4c55
 	}
1d4c55
 
1d4c55
-      if (__glibc_unlikely (workstart != NULL))
1d4c55
-	free (workstart);
1d4c55
-      workstart = NULL;
1d4c55
-
1d4c55
       /* Write the following constant string.  */
1d4c55
       outstring (specs[nspecs_done].end_of_fmt,
1d4c55
 		 specs[nspecs_done].next_fmt
1d4c55
 		 - specs[nspecs_done].end_of_fmt);
1d4c55
     }
1d4c55
  all_done:
1d4c55
-  if (__glibc_unlikely (workstart != NULL))
1d4c55
-    free (workstart);
1d4c55
   scratch_buffer_free (&argsbuf);
1d4c55
   scratch_buffer_free (&specsbuf);
1d4c55
   return done;