[PATCH v2] libdwfl_stacktrace: fix races in ELF tracker cache
Aaron Merey
amerey@redhat.com
Tue Sep 22 02:12:19 GMT 2026
Hi Trithem,
On Mon, Sep 21, 2026 at 4:48 PM toufik TOUFIK <Toufik6957@outlook.com> wrote:
>
> Protect cached ELF entries while they are being consumed and retain Elf references under the libelf lock.
>
> Add a concurrent regression test covering finder/finder and finder/replacer access to the tracker cache.
>
> Build the regression test unconditionally and skip it at runtime when thread safety is disabled.
>
> Signed-off-by: Trithem <trithem90@outlook.com>
Thanks I've merged this patch. I also wrapped the lines of the commit
message to keep the length around 72 chars.
Aaron
> ---
> Changes in v2:
> - Build and register the concurrent tracker test unconditionally.
> - Skip it at runtime with check_thread_safety_enabled when thread safety is disabled.
>
> libdwfl_stacktrace/dwflst_tracker_find_elf.c | 26 +-
> libelf/libelfP.h | 13 +
> tests/Makefile.am | 15 +-
> tests/dwflst-tracker-concurrent.c | 306 +++++++++++++++++++
> tests/run-dwflst-tracker-concurrent.sh | 26 ++
> 5 files changed, 372 insertions(+), 14 deletions(-)
> create mode 100644 tests/dwflst-tracker-concurrent.c
> create mode 100755 tests/run-dwflst-tracker-concurrent.sh
>
> diff --git a/libdwfl_stacktrace/dwflst_tracker_find_elf.c b/libdwfl_stacktrace/dwflst_tracker_find_elf.c
> index 56e87787..cc156cfc 100644
> --- a/libdwfl_stacktrace/dwflst_tracker_find_elf.c
> +++ b/libdwfl_stacktrace/dwflst_tracker_find_elf.c
> @@ -33,8 +33,6 @@
>
> #include <sys/stat.h>
> #include "../libelf/libelfP.h"
> -/* XXX: Private header needed for Elf * ref_count field. */
> -/* TODO: Consider dup_elf() rather than direct ref_count access. */
>
> #include "libdwfl_stacktraceP.h"
>
> @@ -83,7 +81,6 @@ dwflst_tracker_find_cached_elf (Dwflst_Process_Tracker *tracker,
>
> rwlock_rdlock(tracker->elftab_lock);
> ent = dwflst_tracker_elftab_find(&tracker->elftab, hval);
> - rwlock_unlock(tracker->elftab_lock);
>
> /* Guard against collisions.
> TODO: Need proper chaining, dynamicsizehash_concurrent isn't really
> @@ -92,18 +89,23 @@ dwflst_tracker_find_cached_elf (Dwflst_Process_Tracker *tracker,
> rc = fstat(ent->fd, &sb);
> if (rc < 0 || strcmp (module_name, ent->module_name) != 0
> || ent->dev != sb.st_dev || ent->ino != sb.st_ino)
> - return -1;
> + {
> + rwlock_unlock(tracker->elftab_lock);
> + return -1;
> + }
>
> /* Verify that ent->fd has not been updated: */
> if (rc < 0 || ent->dev != sb.st_dev || ent->ino != sb.st_ino
> || ent->last_mtime != sb.st_mtime)
> - return -1;
> -
> - if (ent->elf != NULL)
> - ent->elf->ref_count++;
> - *elfp = ent->elf;
> - *file_name = strdup(ent->module_name);
> - return ent->fd;
> + {
> + rwlock_unlock(tracker->elftab_lock);
> + return -1;
> + }
> + *elfp = __libelf_keep (ent->elf);
> + *file_name = strdup (ent->module_name);
> + int fd = ent->fd;
> + rwlock_unlock(tracker->elftab_lock);
> + return fd;
> }
> INTDEF(dwflst_tracker_find_cached_elf)
>
> @@ -170,7 +172,7 @@ dwflst_tracker_cache_elf (Dwflst_Process_Tracker *tracker,
> elf_end(ent->elf);
> }
> if (elf != NULL && ent->elf != elf)
> - elf->ref_count++;
> + __libelf_keep (elf);
> ent->elf = elf;
> ent->fd = fd;
> if (rc == 0)
> diff --git a/libelf/libelfP.h b/libelf/libelfP.h
> index 11ef5989..2403d796 100644
> --- a/libelf/libelfP.h
> +++ b/libelf/libelfP.h
> @@ -486,6 +486,19 @@ extern int __elf64_updatefile (Elf *elf, int change_bo, size_t shnum)
> internal_function;
>
>
> +static inline Elf *
> +__libelf_keep (Elf *elf)
> +{
> + if (elf == NULL)
> + return NULL;
> +
> + rwlock_wrlock (elf->lock);
> + elf->ref_count++;
> + rwlock_unlock (elf->lock);
> +
> + return elf;
> +}
> +
> /* Alias for exported functions to avoid PLT entries, and
> rdlock/wrlock variants of these functions. */
> extern int __elf_end_internal (Elf *__elf) attribute_hidden;
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 137e9616..dd937122 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -748,8 +748,8 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
> run-eu-search-cfi.sh run-eu-search-macros.sh \
> run-eu-search-lines.sh run-eu-search-die.sh \
> run-dwelf-dwarf-debug-sup.sh \
> - testfile-dwarf5-ref-sup.bz2 testfile-dwarf5.sup.bz2
> -
> + testfile-dwarf5-ref-sup.bz2 testfile-dwarf5.sup.bz2 \
> + run-dwflst-tracker-concurrent.sh
>
> if USE_HELGRIND
> valgrind_cmd=valgrind -q --tool=helgrind --error-exitcode=1 --track-fds=yes \
> @@ -808,6 +808,17 @@ endif
> libebl = ../libebl/libebl.a ../backends/libebl_backends.a ../libcpu/libcpu.a
> libeu = ../lib/libeu.a
>
> +check_PROGRAMS += dwflst-tracker-concurrent
> +
> +TESTS += run-dwflst-tracker-concurrent.sh
> +
> +dwflst_tracker_concurrent_CPPFLAGS = $(AM_CPPFLAGS) \
> + -I$(top_srcdir)/libdwfl_stacktrace
> +
> +dwflst_tracker_concurrent_LDFLAGS = -pthread $(AM_LDFLAGS)
> +
> +dwflst_tracker_concurrent_LDADD = $(libdw) $(libelf)
> +
> arextract_LDADD = $(libelf)
> arsymtest_LDADD = $(libelf)
> ar_extract_ar_LDADD = $(libelf)
> diff --git a/tests/dwflst-tracker-concurrent.c b/tests/dwflst-tracker-concurrent.c
> new file mode 100644
> index 00000000..c1d0b9df
> --- /dev/null
> +++ b/tests/dwflst-tracker-concurrent.c
> @@ -0,0 +1,306 @@
> +/* Copyright (C) 2026 Trithem.
> +
> + Test concurrent libdwfl_stacktrace ELF tracker/cache handling.
> + This file is part of elfutils.
> +
> + This file is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as
> + published by the Free Software Foundation; either version 3 of
> + the License, or (at your option) any later version.
> +
> + elfutils is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <config.h>
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <pthread.h>
> +
> +#include <elf.h>
> +#include <libelf.h>
> +#include <libdw.h>
> +#include <libdwfl.h>
> +#include <libdwfl_stacktrace.h>
> +
> +static const Dwfl_Callbacks callbacks = {
> + .find_elf = NULL,
> + .find_debuginfo = NULL,
> + .section_address = NULL,
> + .debuginfo_path = NULL
> +};
> +
> +struct thread_context
> +{
> + Dwflst_Process_Tracker *tracker;
> + const char *module_name;
> + int successes;
> +};
> +
> +static void *
> +thread_work (void *arg)
> +{
> + struct thread_context *ctx = arg;
> +
> + for (int i = 0; i < 2000; i++)
> + {
> + char *file_name = NULL;
> + Elf *elf = NULL;
> +
> + int fd = dwflst_tracker_find_cached_elf
> + (ctx->tracker, ctx->module_name, ctx->module_name,
> + &file_name, &elf);
> +
> + if (fd < 0 || elf == NULL)
> + return NULL;
> +
> + free (file_name);
> + elf_end (elf);
> + ctx->successes++;
> + }
> +
> + return NULL;
> +}
> +
> +struct replacer_context
> +{
> + Dwflst_Process_Tracker *tracker;
> + const char *module_name;
> + int fd;
> + int successes;
> +};
> +
> +static void *
> +thread_replace (void *arg)
> +{
> + struct replacer_context *ctx = arg;
> +
> + for (int i = 0; i < 1000; i++)
> + {
> + Elf *new_elf = elf_begin (ctx->fd, ELF_C_READ, NULL);
> +
> + if (new_elf == NULL)
> + return NULL;
> +
> + if (!dwflst_tracker_cache_elf (ctx->tracker,
> + ctx->module_name,
> + ctx->module_name,
> + new_elf,
> + ctx->fd))
> + {
> + elf_end (new_elf);
> + return NULL;
> + }
> +
> + elf_end (new_elf);
> + ctx->successes++;
> + }
> +
> + return NULL;
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> + if (argc != 2)
> + {
> + fprintf (stderr, "Usage: %s ELF\n", argv[0]);
> + return 1;
> + }
> +
> + if (elf_version (EV_CURRENT) == EV_NONE)
> + {
> + fprintf (stderr, "elf_version: %s\n", elf_errmsg (-1));
> + return 1;
> + }
> +
> + int fd = open (argv[1], O_RDONLY);
> + if (fd < 0)
> + {
> + fprintf (stderr, "open: %s\n", strerror (errno));
> + return 1;
> + }
> +
> + Elf *elf = elf_begin (fd, ELF_C_READ, NULL);
> + if (elf == NULL)
> + {
> + fprintf (stderr, "elf_begin: %s\n", elf_errmsg (-1));
> + close (fd);
> + return 1;
> + }
> +
> + Dwflst_Process_Tracker *tracker = dwflst_tracker_begin (&callbacks);
> + if (tracker == NULL)
> + {
> + fprintf (stderr, "dwflst_tracker_begin failed\n");
> + elf_end (elf);
> + close (fd);
> + return 1;
> + }
> +
> + if (!dwflst_tracker_cache_elf (tracker, argv[1], argv[1], elf, fd))
> + {
> + fprintf (stderr, "dwflst_tracker_cache_elf failed\n");
> + dwflst_tracker_end (tracker);
> + elf_end (elf);
> + close (fd);
> + return 1;
> + }
> +
> + elf_end (elf);
> +
> + char *found_file_name = NULL;
> + Elf *found_elf = NULL;
> +
> + int found_fd = dwflst_tracker_find_cached_elf
> + (tracker, argv[1], argv[1], &found_file_name, &found_elf);
> +
> + if (found_fd < 0 || found_elf == NULL)
> + {
> + fprintf (stderr, "dwflst_tracker_find_cached_elf failed\n");
> + free (found_file_name);
> + if (found_elf != NULL)
> + elf_end (found_elf);
> + dwflst_tracker_end (tracker);
> + return 1;
> + }
> +
> + free (found_file_name);
> + elf_end (found_elf);
> +
> + pthread_t threads[2];
> + struct thread_context contexts[2];
> + int num_created = 0;
> + int test_failed = 0;
> +
> + /* Finder/finder: exercise concurrent retention of the cached Elf. */
> + for (int i = 0; i < 2; i++)
> + {
> + contexts[i].tracker = tracker;
> + contexts[i].module_name = argv[1];
> + contexts[i].successes = 0;
> + }
> +
> + for (int i = 0; i < 2; i++)
> + {
> + int ret = pthread_create (&threads[i], NULL, thread_work, &contexts[i]);
> + if (ret != 0)
> + {
> + fprintf (stderr, "Failed to create thread: %s\n", strerror (ret));
> + test_failed = 1;
> + break;
> + }
> +
> + num_created++;
> + }
> +
> + for (int i = 0; i < num_created; i++)
> + {
> + int ret = pthread_join (threads[i], NULL);
> + if (ret != 0)
> + {
> + fprintf (stderr, "Failed to join thread: %s\n", strerror (ret));
> + return 1;
> + }
> + }
> +
> + if (test_failed)
> + {
> +
> + dwflst_tracker_end (tracker);
> + return 1;
> + }
> +
> + if (contexts[0].successes != 2000
> + || contexts[1].successes != 2000)
> + {
> + fprintf (stderr, "thread test failed: %d %d\n",
> + contexts[0].successes, contexts[1].successes);
> +
> + dwflst_tracker_end (tracker);
> + return 1;
> + }
> +
> + pthread_t finder;
> + pthread_t replacer;
> +
> + struct thread_context finder_context;
> + struct replacer_context replacer_context;
> +
> + finder_context.tracker = tracker;
> + finder_context.module_name = argv[1];
> + finder_context.successes = 0;
> +
> + replacer_context.tracker = tracker;
> + replacer_context.module_name = argv[1];
> + replacer_context.fd = fd;
> + replacer_context.successes = 0;
> +
> + /* Finder/replacer: exercise concurrent access to the cache entry. */
> + int ret = pthread_create (&finder, NULL, thread_work, &finder_context);
> + if (ret != 0)
> + {
> + fprintf (stderr, "Failed to create finder thread: %s\n", strerror (ret));
> +
> + dwflst_tracker_end (tracker);
> + return 1;
> + }
> +
> + ret = pthread_create (&replacer, NULL, thread_replace, &replacer_context);
> + if (ret != 0)
> + {
> + fprintf (stderr, "Failed to create replacer thread: %s\n",
> + strerror (ret));
> +
> + ret = pthread_join (finder, NULL);
> + if (ret != 0)
> + {
> + fprintf (stderr, "Failed to join finder thread: %s\n",
> + strerror (ret));
> + return 1;
> + }
> +
> +
> + dwflst_tracker_end (tracker);
> + return 1;
> + }
> +
> + ret = pthread_join (finder, NULL);
> + if (ret != 0)
> + {
> + fprintf (stderr, "Failed to join finder thread: %s\n", strerror (ret));
> + return 1;
> + }
> +
> + ret = pthread_join (replacer, NULL);
> + if (ret != 0)
> + {
> + fprintf (stderr, "Failed to join replacer thread: %s\n", strerror (ret));
> + return 1;
> + }
> +
> + if (finder_context.successes != 2000
> + || replacer_context.successes != 1000)
> + {
> + fprintf (stderr, "finder/replacer test failed: %d %d\n",
> + finder_context.successes, replacer_context.successes);
> +
> + dwflst_tracker_end (tracker);
> + return 1;
> + }
> +
> +
> + dwflst_tracker_end (tracker);
> +
> + return 0;
> +}
> diff --git a/tests/run-dwflst-tracker-concurrent.sh b/tests/run-dwflst-tracker-concurrent.sh
> new file mode 100755
> index 00000000..5224d94a
> --- /dev/null
> +++ b/tests/run-dwflst-tracker-concurrent.sh
> @@ -0,0 +1,26 @@
> +#!/bin/sh
> +# Copyright (C) 2026 Trithem
> +# This file is part of elfutils.
> +#
> +# This file is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 3 of
> +# the License, or (at your option) any later version.
> +#
> +# elfutils is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see
> +# <http://www.gnu.org/licenses/>.
> +
> +. $srcdir/thread-safety-subr.sh
> +
> +check_thread_safety_enabled
> +
> +testrun ${abs_builddir}/dwflst-tracker-concurrent \
> + ${abs_builddir}/dwflst-tracker-concurrent
> +
> +exit 0
> --
> 2.53.0
>
>
More information about the Elfutils-devel
mailing list