Sourceware Bugzilla – Bug 13446
crash in vfprintf with more than 64 format args and format specifiers
Last modified: 2011-12-18 09:10:10 UTC
Created attachment 6077 [details]
A user found this in an application that uses format specifiers and
motif was initializing.
Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
#0 0x0000000000000000 in ?? ()
#1 0x00007ffff7a91ec8 in _IO_vfprintf_internal (s=<optimized out>, format=<optimized out>, ap=<optimized out>)
#2 0x00007ffff7ab5024 in __IO_vsprintf (string=0x603020 "",
format=0x601080 "%%P%%S:%s/%%L/%%T/%%N/%%P%%S:%s/%%l_%%t/%%T/%%N/%%P%%S:%s/%%l/%%T/%%N/%%P%%S:%s/%%T/%%N/%%P%%S:%s/%%L/%%T/%%P%%S:%s/%%l_%%t/%%T/%%P%%S:%s/%%l/%%T/%%P%%S:%s/%%T/%%P%%S:%s/%%P%%S:%s/%%L/%%T/%%N/%%P%%S:%"..., args=0x7fffffffc6c8) at iovsprintf.c:43
#3 0x00007ffff7a9c0a7 in __sprintf (s=<optimized out>, format=<optimized out>) at sprintf.c:34
#4 0x00000000004009d3 in main (argc=1, argv=0x7fffffffd928) at mtest.c:98
Created attachment 6078 [details]
# patch to fix printf when register_printf_function is in use and the format to
# printf contains more than 64 elements.
# extend_alloca modifies nspecs_max (2nd arg) with the size of the space, not the
# number of elements. Use a temporary to store the return value. Explicitly set the correct size.
Thanks, this patch fixes indeed the testcase.
The following comment was made to the downstream bugreport https://bugzilla.novell.com/show_bug.cgi?id=733140#c6 by Christoph Bartoschek:
I would suggest the following patch instead. This way nsize is initialized with
the correct number of bytes. And nspecs_max uses the whole given buffer.
diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index 753a5ac..6e026ae 100644
@@ -1683,8 +1683,9 @@ do_positional:
/* Extend the array of format specifiers. */
struct printf_spec *old = specs;
- specs = extend_alloca (specs, nspecs_max,
- 2 * nspecs_max * sizeof (*specs));
+ size_t nsize = nspecs_max * sizeof(*specs);
+ specs = extend_alloca (specs, nsize, 2 * nsize);
+ nspecs_max = nsize/sizeof(*specs);
/* Copy the old array's elements to the new space. */
memmove (specs, old, nspecs * sizeof (struct printf_spec));
nsize does not always represent the correct size of the returned object,
due to the way that extend_alloca (an internal glibc macro) is implemented.
I wouldn't recommend this latest proposal.
I suggest that the patch from #3 is used.
- extend_alloca sets the second parameter to a value that can safely be used. There is no possibility that nsize is larger than the usable buffer size after extend_alloca.
- If one does not want to use the extending feature of extend_alloca then one
should just use alloca.
I checked in a correct patch.
The patch is wrong, you need to increase nspecs_size in the for loop if you do a new alloca call.
Otherwise you call alloca every time with nspecs_size*2 - and eventually override the array
specs = extend_alloca (specs, nspecs_size, 2 * nspecs_size);
a line with:
nspecs_size = 2 * nspecs_size;
That's what extend_alloca does.