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

[Adhemerval Zanella Netto]

On 12/02/26 15:56, Florian Weimer wrote:
> * 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.

Just to make it clear that I fully agree that we should remove the possible
unbounded or caller provided alloca/VLA (including ELF files).  My point
is just using some data structure like obstack or similar to use as scratch
buffer might be simpler than trying to refactoring the code.