[PATCH] elf: Fix leak in _dl_scope_free

[ningle]

>> _dl_scope_free uses a free-list to delay freeing scope pointers until
>> THREAD_GSCOPE_WAIT has ensured safety.  In the multi-threaded case,
>> pointers are queued in dl_scope_free_list until the list is full.
>>
>> When the free-list is already full on entry, the current code calls
>> THREAD_GSCOPE_WAIT and frees all pointers in the list, but ignores the
>> pointer passed to this _dl_scope_free call (OLD): it is neither added
>> to the list nor freed, causing a leak.
>>
>> Fix this by, after flushing the full list, storing OLD in list[0] and
>> setting count to 1, so OLD becomes the first element of the next
>> batch and will be freed in a later flush.
>>
>> * elf/dl-XXX.c (_dl_scope_free): When the free-list is full, flush it,
>>   then enqueue OLD as the first element instead of dropping it.
>>
>> Signed-off-by: ningle <ning.le@h3c.com>
>> ---
>>  elf/dl-scope.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/elf/dl-scope.c b/elf/dl-scope.c
>> index 4fc028e6..2d666227 100644
>> --- a/elf/dl-scope.c
>> +++ b/elf/dl-scope.c
>> @@ -50,7 +50,11 @@ _dl_scope_free (void *old)
>>      {
>>        THREAD_GSCOPE_WAIT ();
>>        while (fsl->count > 0)
>> -	free (fsl->list[--fsl->count]);
>> +        {
>> +          free (fsl->list[--fsl->count]);
>> +        }
>> +      fsl->list[0] = old;
>> +      fsl->count = 1;
>>        return 1;
>>      }
>>    return 0;

>There's an older patch for this:

>  [PATCH] elf: Fix memory leaks for dl_scope_free_list (bug 26641)
>  <https://inbox.sourceware.org/libc-alpha/20230906104939.718623-1-peadar@arista.com/>

>Why do you delay freeing the old pointer?

Thanks for the feedback and clarification.

We have observed real memory growth in production-like workloads under some
non-standard but valid usage patterns, and traced it to the `_dl_scope_free`
freelist leak. I then checked current glibc master and saw that this code path
is still unchanged, so I prepared this patch.

The delayed free of `old` in my version is not for safety or performance
reasons, but simply a design choice to keep `old` going through the same
batching mechanism as the other cached entries. Freeing `old` immediately after
`THREAD_GSCOPE_WAIT` (as in the earlier patch) is equally correct, and I am
happy to switch to that behaviour if it is preferred. My main goal is to get
the leak fixed.