[PATCH] elf: Limit program header count to 64 [BZ #21265]

[Florian Weimer]

* Adhemerval Zanella Netto:

> On 12/02/26 14:05, Florian Weimer wrote:
>> * Adhemerval Zanella Netto:
>> 
>>>> Reviewed-by: gaoxiang <gaoxiang@kylinos.cn>
>>>> Signed-off-by: litenglong <litenglong@kylinos.cn>
>>>>
>>>> Change-Id: I38bcbb179fdfd5134f3a3d9b6a473f3cf62c7afe
>>>> ---
>>>>  elf/dl-load.c | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/elf/dl-load.c b/elf/dl-load.c
>>>> index 7355eef..7bd8033 100644
>>>> --- a/elf/dl-load.c
>>>> +++ b/elf/dl-load.c
>>>> @@ -1099,6 +1099,12 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>>>>  
>>>>    {
>>>>      /* Scan the program header table, collecting its load commands.  */
>>>> +    /* Reject unreasonable e_phnum values to prevent stack overflow via VLA. */
>>>> +    if (__glibc_unlikely (l->l_phnum > 64))
>>>> +      {
>>>> +        errstring = N_("too many program headers");
>>>> +        goto lose;
>>>> +      }
>>>
>>> This might potentially break loading of well-formed ELF objects. Also,
>>> there is other alloca that are controlled by ELF header information.
>>>
>>> The issue would be mitigated by -fstack-clash-protection, although it
>>> not supported on all ABIs. I think a best course of action is either
>>> use mmap direct to allocate a temporary buffer (maybe something similar
>>> to elf/dl-minimal-malloc.c, but with user-provided state).
>> 
>> I think we can iterate over the program headers in _dl_map_segments
>> (with a suitable abstraction).  There's no sorting involved, so we don't
>> need to make a copy of the load segments.  The additional performance
>> should be pretty minimal.
>
> For this case it might be feasible, but there are other places where we do
> allocate/VLA and removing them might not be that straightforward without
> some large changes.

I think many of these places have already been fixed, and we've got
patches for others.

I agree that building with -fstack-clash-protection (where there is
target support; the generic GCC implementation is broken) is a very good
idea, given that the overhead for it is so low.  But for the remaining
allocas/VLAs, we should still fix them if it's not too hideous.

Thanks,
Florian