Re: [PATCH 11/20] elf: Fix alloca size in _dl_debug_vdprintf

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 28/10/22 11:43, Szabolcs Nagy wrote:
> The 10/28/2022 10:56, Adhemerval Zanella Netto wrote:
>> On 27/10/22 12:33, Szabolcs Nagy via Libc-alpha wrote:
>>> The alloca size did not consider the optional width parameter for
>>> padding which could cause buffer underflow. The width is currently used
>>> e.g. by _dl_map_object_from_fd which passes 2 * sizeof(void *) which
>>> can be larger than the alloca buffer size on targets where
>>> sizeof(void *) >= 2 * sizeof(unsigned long).
>>>
>>> Even if large width is not used on existing targets it is better to fix
>>> the formatting code to avoid surprises.
>>> ---
>>>  elf/dl-printf.c | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/elf/dl-printf.c b/elf/dl-printf.c
>>> index 429d2e80c2..00c114002c 100644
>>> --- a/elf/dl-printf.c
>>> +++ b/elf/dl-printf.c
>>> @@ -163,8 +163,11 @@ _dl_debug_vdprintf (int fd, int tag_p, const char *fmt, va_list arg)
>>>  		/* We use alloca() to allocate the buffer with the most
>>>  		   pessimistic guess for the size.  Using alloca() allows
>>>  		   having more than one integer formatting in a call.  */
>>> -		char *buf = (char *) alloca (1 + 3 * sizeof (unsigned long int));
>>> -		char *endp = &buf[1 + 3 * sizeof (unsigned long int)];
>>> +		int size = 1 + 3 * sizeof (unsigned long int);
>>> +		if (width + 1 > size)
>>> +		  size = width + 1;
>>> +		char *buf = (char *) alloca (size);
>>> +		char *endp = &buf[size];
>>>  		char *cp = _itoa (num, endp, *fmt == 'x' ? 16 : 10, 0);
>>>  
>>>  		/* Pad to the width the user specified.  */
>>
>>
>> Would be better to just limit a maximum width and use a fixed-size buffer instead
>> (and assert if size is larger)? 
> 
> i already committed this. i think it's safe:
> it's internal api and using huge paddings is unlikely.

The idea is to remove all internal alloca usage where possible,
and I think this is a good spot to continue the work.