[PATCH] Fix printf_unknown and do_positional (BZ #4514)

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

In printf_unknown we use work_buffer only for _itoa_word, printing
info->width or info->prec value.  So we only need enough bytes
to write those numbers in decimal, rather than info->width bytes + 32
(info->spec was a typo for info->prec I believe).

Now, when I wrote a testcase for this and in addition to the bugzilla
provided testcase used another format string afterwards, I discovered
another bug.  While the normal non-positional loop reinitializes
workend at the beginning of each iteration and frees workstart if
it was malloced at the end of each iteration, do_positional loop
only frees workstart at the end of each loop, without resetting
workend at the start of the loop.  This can be reproduced even without
any unknown format spec, just with positional args (see first test in the
testcase below).  Another problem is that do_positional loop uses its
own workstart variable which shadows the function's one.  This can lead to
memory leaks if e.g. outchar decides to goto all_done.

I believe we also have memory leaks if *printf call is cancelled while
it has some memory (workstart resp. string (string_malloced != 0)) malloced,
but I haven't addressed this in the patch below.

Also, wonder if we shouldn't compute the sum of allocaed memory sizes
within whole *printf and use it for __libc_use_alloca instead of testing
sizes separately.  Or don't throw away the so far largest work buffer
allocated through alloca, but instead reuse it if it is big enough.
Otherwise, e.g. printf ("%.*s%.*s%.*s%.*s%.*s%.*s%.*s%.*s%.*s%.*s%.*s%.*s",
			N, "", N, "", N, "", N, "", N, "", N, "",
			N, "", N, "", N, "", N, "", N, "", N, "");
for N such that __libc_use_alloca (N + 32) && !__libc_use_alloca ((N + 32) * 12)
could overflow the stack.

2007-05-21  Jakub Jelinek  <jakub@redhat.com>

	[BZ #4514]
	* stdio-common/vfprintf.c (vfprintf): Don't shadow workstart variable,
	reinitialize workend at the start of each do_positional format spec
	loop, free workstart before do_positional loops.
	(printf_unknown): Fix size of work_buffer.
	* stdio-common/tst-sprintf.c (main): Add 3 new testcases.

--- libc/stdio-common/vfprintf.c.jj	2007-05-09 13:26:13.000000000 +0200
+++ libc/stdio-common/vfprintf.c	2007-05-21 14:28:39.000000000 +0200
@@ -1627,6 +1627,9 @@ do_positional:
     /* Just a counter.  */
     size_t cnt;
 
+    if (__builtin_expect (workstart != NULL, 0))
+      free (workstart);
+    workstart = NULL;
 
     if (grouping == (const char *) -1)
       {
@@ -1801,7 +1804,9 @@ do_positional:
 	int use_outdigits = specs[nspecs_done].info.i18n;
 	char pad = specs[nspecs_done].info.pad;
 	CHAR_T spec = specs[nspecs_done].info.spec;
-	CHAR_T *workstart = NULL;
+
+	workstart = NULL;
+	workend = &work_buffer[sizeof (work_buffer) / sizeof (CHAR_T)];
 
 	/* Fill in last information.  */
 	if (specs[nspecs_done].width_arg != -1)
@@ -1926,7 +1931,7 @@ printf_unknown (FILE *s, const struct pr
 
 {
   int done = 0;
-  CHAR_T work_buffer[MAX (info->width, info->spec) + 32];
+  CHAR_T work_buffer[MAX (sizeof (info->width), sizeof (info->prec)) * 3];
   CHAR_T *const workend
     = &work_buffer[sizeof (work_buffer) / sizeof (CHAR_T)];
   register CHAR_T *w;
--- libc/stdio-common/tst-sprintf.c.jj	2007-01-25 10:58:43.000000000 +0100
+++ libc/stdio-common/tst-sprintf.c	2007-05-21 14:55:09.000000000 +0200
@@ -37,5 +37,26 @@ main (void)
       free (dst);
     }
 
+  if (sprintf (buf, "%1$d%3$.*2$s%4$d", 7, 67108863, "x", 8) != 3
+      || strcmp (buf, "7x8") != 0)
+    {
+      printf ("sprintf (buf, \"%%1$d%%3$.*2$s%%4$d\", 7, 67108863, \"x\", 8) produced `%s' output", buf);
+      result = 1;
+    }
+
+  if (sprintf (buf, "%67108863.16\"%d", 7) != 14
+      || strcmp (buf, "%67108863.16\"7") != 0)
+    {
+      printf ("sprintf (buf, \"%%67108863.16\\\"%%d\", 7) produced `%s' output", buf);
+      result = 1;
+    }
+
+  if (sprintf (buf, "%*\"%d", 0x3ffffff, 7) != 11
+      || strcmp (buf, "%67108863\"7") != 0)
+    {
+      printf ("sprintf (buf, \"%%*\\\"%%d\", 0x3ffffff, 7) produced `%s' output", buf);
+      result = 1;
+    }
+
   return result;
 }

	Jakub