Message ID | 20230906104939.718623-1-peadar@arista.com |
---|---|
State | New |
Headers | show |
Series | elf: Fix memory leaks for dl_scope_free_list (bug 26641) | expand |
* Peter Edwards via Libc-alpha: > _dl_scope_free is used to maintain a cache of scope lists, so a number > of them can be free'd all at once (thus avoiding contention on a lock) > > Once 50 scopes have been accumulated, the next call to _dl_scope_free > will free the accumulated 50 entries. However, it does *not* free the > just-passed entry. > > Secondly, there's a __libc_freeres function for releasing the > _dl_scope_free list when tearing down libc's innards. This just free's > the actual list, but not its content. Update it to also free the content. > > This fixes valgrind observed leaks from dl_open_worker where these lists > are allocated. These two fixes are very different, maybe put them into separate patches? And __libc_freeres changes are much less risky than other changes. The _dl_scope_free fix should also reference a bug in Bugzilla because it's a repeatable memory leak with potential application impact. How difficult is it to trigger the missed free in _dl_scope_free? Maybe it's possible to write an mtrace-based test case? Thanks, Florian
diff --git a/elf/dl-libc.c b/elf/dl-libc.c index c12e52f330..0b2e30ea13 100644 --- a/elf/dl-libc.c +++ b/elf/dl-libc.c @@ -329,7 +329,12 @@ __dl_libc_freemem (void) malloc), and in the static library it's in .bss space. */ free_slotinfo (&GL(dl_tls_dtv_slotinfo_list)->next); - void *scope_free_list = GL(dl_scope_free_list); + struct dl_scope_free_list *scope_free_list = GL(dl_scope_free_list); GL(dl_scope_free_list) = NULL; - free (scope_free_list); + if (scope_free_list != NULL) + { + while (scope_free_list->count > 0) + free (scope_free_list->list[--scope_free_list->count]); + free (scope_free_list); + } } diff --git a/elf/dl-scope.c b/elf/dl-scope.c index 2d4653c6e6..dba77068bd 100644 --- a/elf/dl-scope.c +++ b/elf/dl-scope.c @@ -51,6 +51,7 @@ _dl_scope_free (void *old) THREAD_GSCOPE_WAIT (); while (fsl->count > 0) free (fsl->list[--fsl->count]); + free (old); return 1; } return 0;
_dl_scope_free is used to maintain a cache of scope lists, so a number of them can be free'd all at once (thus avoiding contention on a lock) Once 50 scopes have been accumulated, the next call to _dl_scope_free will free the accumulated 50 entries. However, it does *not* free the just-passed entry. Secondly, there's a __libc_freeres function for releasing the _dl_scope_free list when tearing down libc's innards. This just free's the actual list, but not its content. Update it to also free the content. This fixes valgrind observed leaks from dl_open_worker where these lists are allocated. Signed-off-by: Peter Edwards <peadar@arista.com> --- elf/dl-libc.c | 9 +++++++-- elf/dl-scope.c | 1 + 2 files changed, 8 insertions(+), 2 deletions(-)