| Message ID | 20230510153230.2207571-1-josimmon@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] vfprintf-internal: Replace alloca with malloc. | expand |
On 5/10/23 11:32, Joe Simmons-Talbott via Libc-alpha wrote: > Avoid potential stack overflow from unbounded alloca. This fails to apply in pre-commit CI e.g. error: corrupt patch at line 145 How did you generate this patch? > --- > Changes to v1: > * Don't check pointer before calling free() > * Move deallocation code after the all_done label > > stdio-common/vfprintf-internal.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c > index c76c06e49b..58c21481ce 100644 > --- a/stdio-common/vfprintf-internal.c > +++ b/stdio-common/vfprintf-internal.c > @@ -1001,6 +1001,7 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format, > scratch_buffer_init (&specsbuf); > struct printf_spec *specs = specsbuf.data; > size_t specs_limit = specsbuf.length / sizeof (specs[0]); > + bool malloced_pa_user = false; > > /* Used as a backing store for args_value, args_size, args_type > below. */ > @@ -1171,7 +1172,10 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format, > else if (__glibc_unlikely (__printf_va_arg_table != NULL) > && __printf_va_arg_table[args_type[cnt] - PA_LAST] != NULL) > { > - args_value[cnt].pa_user = alloca (args_size[cnt]); > + args_value[cnt].pa_user = malloc (args_size[cnt]); > + if (args_value[cnt].pa_user == NULL) > + break; > + malloced_pa_user = true; > (*__printf_va_arg_table[args_type[cnt] - PA_LAST]) > (args_value[cnt].pa_user, ap_savep); > } > @@ -1334,7 +1338,11 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format, > (specs[nspecs_done].next_fmt > - specs[nspecs_done].end_of_fmt)); > } > all_done: > + if (malloced_pa_user) > + for (cnt = 0; cnt < nargs; ++cnt) > + free (args_value[cnt].pa_user); > scratch_buffer_free (&argsbuf); > scratch_buffer_free (&specsbuf); > }
On Mon, May 15, 2023 at 02:35:36PM -0400, Carlos O'Donell wrote: > On 5/10/23 11:32, Joe Simmons-Talbott via Libc-alpha wrote: > > Avoid potential stack overflow from unbounded alloca. > > This fails to apply in pre-commit CI e.g. error: corrupt patch at line 145 > > How did you generate this patch? I used 'git format-patch' but then removed a blank line that the patch added during 'git send-email --annotate'. Sorry. I'll post a v3 shortly. Thanks, Joe
diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c index c76c06e49b..58c21481ce 100644 --- a/stdio-common/vfprintf-internal.c +++ b/stdio-common/vfprintf-internal.c @@ -1001,6 +1001,7 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format, scratch_buffer_init (&specsbuf); struct printf_spec *specs = specsbuf.data; size_t specs_limit = specsbuf.length / sizeof (specs[0]); + bool malloced_pa_user = false; /* Used as a backing store for args_value, args_size, args_type below. */ @@ -1171,7 +1172,10 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format, else if (__glibc_unlikely (__printf_va_arg_table != NULL) && __printf_va_arg_table[args_type[cnt] - PA_LAST] != NULL) { - args_value[cnt].pa_user = alloca (args_size[cnt]); + args_value[cnt].pa_user = malloc (args_size[cnt]); + if (args_value[cnt].pa_user == NULL) + break; + malloced_pa_user = true; (*__printf_va_arg_table[args_type[cnt] - PA_LAST]) (args_value[cnt].pa_user, ap_savep); } @@ -1334,7 +1338,11 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format, (specs[nspecs_done].next_fmt - specs[nspecs_done].end_of_fmt)); } all_done: + if (malloced_pa_user) + for (cnt = 0; cnt < nargs; ++cnt) + free (args_value[cnt].pa_user); scratch_buffer_free (&argsbuf); scratch_buffer_free (&specsbuf); } -- 2.39.2