Re: [PATCH v2] vfprintf-internal: Replace alloca with malloc.

[Carlos O'Donell <carlos@redhat.com>]

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);
>  }

-- 
Cheers,
Carlos.