Skip to content

fix: skip /tmp extraction when profiling libraries are installed#529

Open
r1viollet wants to merge 3 commits intomainfrom
r1viollet/fix-tmp-extraction
Open

fix: skip /tmp extraction when profiling libraries are installed#529
r1viollet wants to merge 3 commits intomainfrom
r1viollet/fix-tmp-extraction

Conversation

@r1viollet
Copy link
Copy Markdown
Collaborator

@r1viollet r1viollet commented Mar 30, 2026

What does this PR do?

When ddprof is installed as a package, skip writing executables and shared libraries to /tmp. Security scanners like CrowdStrike Falcon flag new executables in /tmp, and the extraction is pointless when
the files already sit in standard system paths.

The loader constructor tries dlopen with the bare library name before falling back to /tmp extraction. After a successful bare-name dlopen, it verifies the loaded library's build version matches via a
new ddprof_profiling_version() symbol to prevent version mismatches between a stale system install and the injected library.

Version macros (VER_MAJ/MIN/PATCH/REV) are now propagated to all profiling library targets so the version string is consistent across the loader, embedded lib, and shared lib.

Two bugs affecting Alpine Linux (musl) are also fixed:

ensure_loader_symbols_promoted() failed on musl. The loader re-opens itself with RTLD_NOLOAD | RTLD_GLOBAL to promote ddprof_lib_state (initial-exec TLS) to global scope before loading the embedded .so.
On musl, RTLD_NOLOAD with a bare SONAME always fails because musl tracks loaded libraries by full path, not SONAME. Fixed by using dladdr() to obtain the loader's actual on-disk path at runtime.

ddprof binary not found when using an installed libdd_profiling-embedded.so. The standalone embedded lib does not contain the ddprof binary (profiler_exe_data() returns size=0), so the previous fallback
left DD_PROFILING_NATIVE_DDPROF_EXE unset. The loader now searches for ddprof relative to its own path: <loader_dir>/../bin/ddprof (package install layout) and <loader_dir>/ddprof (flat build layout),
falling back to extraction from the loader's own embedded data.

The loader() constructor is refactored into three helpers (try_load_installed_profiling_lib, setup_ddprof_exe_for_installed_lib, load_embedded_profiling_lib) and a new test (loader_installed_lib-ut.sh)
covers both path layouts.

Motivation

fix for the following issue

Additional Notes

NA

How to test the change?

Describe here in detail how the change can be validated. This is a great section to call out specific tests you've added or improved, or to acknowledge code sections which are particularly difficult to test.

@r1viollet r1viollet marked this pull request as ready for review March 30, 2026 11:30
@r1viollet r1viollet requested a review from nsavoire as a code owner March 30, 2026 11:30
src/lib/loader.c Outdated
"library. Remove or upgrade the installed package to "
"avoid this warning.\n",
k_libdd_profiling_embedded_name);
dlclose(s_profiling_lib_handle);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using directly dlclose might cause issues

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

src/lib/loader.c Outdated
// search path (or anywhere in LD_LIBRARY_PATH / DT_RPATH / ldconfig
// cache), dlopen finds it without extracting anything to /tmp.
s_profiling_lib_handle =
my_dlopen(k_libdd_profiling_embedded_name, RTLD_LOCAL | RTLD_NOW);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my_dlopen will print an error upon failure (common case)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I could have a silent version of this for custom errors

} // namespace
} // namespace ddprof

const char *ddprof_profiling_version() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns a slightly different version than str_version which is surprising

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps factorize code between dd_profiling.cc / loader.c / version.cc with a version macro ?

@nsavoire
Copy link
Copy Markdown
Collaborator

exec_ddprof gains a similar PATH fallback (execvp), scoped to only fire when no explicit exe path was set, avoiding version mismatches between the extracted .so and a different ddprof binary found in PATH.

I don't see this part ?

@r1viollet
Copy link
Copy Markdown
Collaborator Author

exec_ddprof gains a similar PATH fallback (execvp), scoped to only fire when no explicit exe path was set, avoiding version mismatches between the extracted .so and a different ddprof binary found in PATH.

I don't see this part ?

sorry, I removed that part, I am not sure we want this

@r1viollet
Copy link
Copy Markdown
Collaborator Author

Setting as draft until I get time to get back to this and fix some of the comments from Nicolas.

@r1viollet r1viollet marked this pull request as draft March 30, 2026 12:56
src/lib/loader.c Outdated
ensure_loader_symbols_promoted();

s_profiling_lib_handle = my_dlopen(lib_profiling_path, RTLD_LOCAL | RTLD_NOW);
free(lib_profiling_path);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: free can be moved at the end if (!s_profiling_lib_handle) { scope and strdup at line 354 deleted.

} // namespace
} // namespace ddprof

const char *ddprof_profiling_version() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps factorize code between dd_profiling.cc / loader.c / version.cc with a version macro ?

# The fake lib (built by cmake) is in the test dir. Put it on LD_LIBRARY_PATH
# so the loader's bare-name dlopen finds it instead of the real embedded lib.
LD_LIBRARY_PATH="${FAKE_LIB_DIR}" LD_PRELOAD="${SHARED_LIB}" \
"${BUILDDIR}/test/simple_malloc-static" --loop 10 --spin 10 \
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: test implicitly depends on simple_malloc-static target

When ddprof is installed as a package, skip writing executables and
shared libraries to /tmp. Security scanners like CrowdStrike Falcon
flag new executables in /tmp, and the extraction is pointless when the
files already sit in standard system paths.

The loader constructor tries dlopen with the bare library name before
falling back to /tmp extraction. After a successful bare-name dlopen,
it verifies the loaded library's build version matches via a new
ddprof_profiling_version() symbol to prevent version mismatches between
a stale system install and the injected library.

Version macros (VER_MAJ/MIN/PATCH/REV) are now propagated to all
profiling library targets so the version string is consistent across
the loader, embedded lib, and shared lib.

Based on original work by Xavier Roche <xavier.roche@algolia.com>.

Co-Authored-By: Xavier Roche <xavier.roche@algolia.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@r1viollet r1viollet force-pushed the r1viollet/fix-tmp-extraction branch from 26099e7 to a95cde6 Compare March 31, 2026 13:40
r1viollet and others added 2 commits March 31, 2026 16:59
…alled lib

Split the monolithic loader() constructor into three focused helpers:
- try_load_installed_profiling_lib(): silent dlopen + version check
- setup_ddprof_exe_for_installed_lib(): find ddprof via relative paths
- load_embedded_profiling_lib(): extract lib+exe to /tmp

Fix ensure_loader_symbols_promoted() to use dladdr() for the RTLD_NOLOAD
call so it works on musl (which tracks libs by full path, not SONAME).

Fix exe discovery when using an installed libdd_profiling-embedded.so:
the standalone lib does not embed the ddprof binary, so the loader now
searches for it relative to its own path:
  1. <loader_dir>/../bin/ddprof  (standard install layout)
  2. <loader_dir>/ddprof         (flat build/dev layout)
with a fallback to extracting from the loader's own embedded data.

Add loader_installed_lib-ut.sh to test both layout cases.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@r1viollet r1viollet marked this pull request as ready for review March 31, 2026 16:58
@r1viollet
Copy link
Copy Markdown
Collaborator Author

this would require a documentation update once we merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants