This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [patch] Fix BZ 23187 -- stack overflow for many Phdr[]s
On 05/22/2018 03:49 PM, Adhemerval Zanella wrote:
>
>
> On 21/05/2018 17:34, Paul Pluzhnikov wrote:
>> Greetings,
>>
>> Attached patch fixes several instances of unconstrained stack allocation,
>> which causes ld.so to overflow stack and crash while processing DSO with
>> 1000s of Phdr[]s, and adds a test case.
>>
>> 2018-05-21 Paul Pluzhnikov <ppluzhnikov@google.com>
>>
>> [BZ #23187]
>> * elf/Makefile (tst-many-phdrs): New test.
>> * elf/tst-many-phdrs.c: New.
>> * elf/dl-load.c (_dl_map_object_from_fd): Constrain stack
>> allocation.
>
> Looks like the CL is mangled somehow, 'allocation' should be aligned with tab.
>
>> @@ -960,7 +966,18 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>> phdr = (void *) (fbp->buf + header->e_phoff);
>> else
>> {
>> - phdr = alloca (maplength);
>> + if (maplength < __MAX_ALLOCA_CUTOFF)
>> + phdr = alloca (maplength);
>> + else
>> + {
>> + phdr_malloced = malloc (maplength);
>> + if (phdr_malloced == NULL)
>> + {
>> + errstring = N_("cannot allocate memory for program header");
>> + goto call_lose_errno;
>> + }
>> + phdr = phdr_malloced;
>> + }
>
> I think we should avoid use alloca in newer code in favor of current
> framework that tries to leverage this kind of behaviour (initial stack
> allocation with potential heap usage). They also have the advantage of
> avoid using __MAX_ALLOCA_CUTOFF, which is kind arbitrary and do not
> really address total stack usage over multiple function calls.
>
> What about use dynarray for this as below. The initial sizes are still
> arbitraty and should cover most common cases, but I do not have a strong
> opinion here (they have total stack usage of 384 bytes for loadcmd_list
> and 896 for phdr_list on 64 bits architectures).
>
+1.
Should we use a scratch buffer? Which has constant size and can be expanded
only if required?
--
Cheers,
Carlos.