Use `DEEPBIND` flag when loading external modules using `dlopen` by rjd15372 · Pull Request #1703 · valkey-io/valkey

rjd15372 · 2025-02-10T14:48:39Z

The current flags used in dlopen to load external modules don't allow modules to define symbols that are already defined by the valkey server binary because the symbol resolution first looks in the server memory and only if it does not find anything, it looks in the module (shared library) memory.

This might become a problem if, for instance, we try to implement a new scripting engine based on a newer version of Lua. The Lua interpreter library shares many symbol names with the Lua interpreter included in the Valkey server binary.

To fix the above problem, this PR adds the flag RTLD_DEEPBIND to the flags used in dlopen on systems that support it, which changes the symbol resolution strategy to look for the symbol in the module memory first, if the code executing is from the module.

The current flags used in `dlopen` to load external modules don't allow modules to define symbols that are already defined by the valkey server binary because the symbol resolution first looks in the server memory and only if it does not find anything, it looks in the module (shared library) memory. This might become a problem if, for instance, we try to implement a new scripting engine based on a newer version of Lua. The Lua interpreter library shares many symbol names with the Lua interpreter included in the Valkey server binary. To fix the above problem, this PR adds the flag `RTLD_DEEPBIND` to the flags used in `dlopen`, which changes the symbol resolution strategy to look for the symbol in the module memory first, if the code executing is from the module. Signed-off-by: Ricardo Dias <[email protected]>

codecov · 2025-02-10T15:04:16Z

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 71.11%. Comparing base (3828936) to head (a221102).
Report is 5 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/module.c	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           unstable    #1703   +/-   ##
=========================================
  Coverage     71.10%   71.11%           
=========================================
  Files           123      123           
  Lines         65523    65532    +9     
=========================================
+ Hits          46590    46600   +10     
+ Misses        18933    18932    -1

Files with missing lines	Coverage Δ
src/module.c	`9.61% <0.00%> (+<0.01%)`	⬆️

... and 12 files with indirect coverage changes

Signed-off-by: Ricardo Dias <[email protected]>

src/module.c

Signed-off-by: Ricardo Dias <[email protected]>

zuiderkwast · 2025-02-11T07:48:10Z

@rjd15372 Daily job failed:

https://github.com/valkey-io/valkey/actions/runs/13253032412/job/36994775025

==89691==You are trying to dlopen a /home/runner/work/valkey/valkey/tests/modules/reply.so shared library with RTLD_DEEPBIND flag which is incompatible with sanitizer runtime (see google/sanitizers#611 for details). If you want to run /home/runner/work/valkey/valkey/tests/modules/reply.so library under sanitizers please remove RTLD_DEEPBIND from dlopen flags.

enjoy-binbin · 2025-02-11T08:35:36Z

I see we did skip it, is there anything missing in the condition?

    int dlopen_flags = RTLD_NOW | RTLD_LOCAL;
#if (defined(__linux__) || defined(__FreeBSD__)) && !defined(__SANITIZE_ADDRESS__)
    /* RTLD_DEEPBIND, which is required for loading modules that contains the
     * same symbols, does not work with ASAN. Therefore, we exclude
     * RTLD_DEEPBIND when doing test builds with ASAN.
     * See https://github.com/google/sanitizers/issues/611 for more details.
     *
     * This flag is also currently only available in Linux and FreeBSD. */
    dlopen_flags |= RTLD_DEEPBIND;
#endif

zuiderkwast · 2025-02-11T08:38:46Z

@enjoy-binbin see this: #1707

Fixes the failure trying to use dlopen with DEEPBIND with ASAN when compiling with Clang: ==90510==You are trying to dlopen a /home/runner/work/valkey/valkey/tests/modules/keyspecs.so shared library with RTLD_DEEPBIND flag which is incompatible with sanitizer runtime (see google/sanitizers#611 for details). If you want to run /home/runner/work/valkey/valkey/tests/modules/keyspecs.so library under sanitizers please remove RTLD_DEEPBIND from dlopen flags. https://github.com/valkey-io/valkey/actions/runs/13261241213/job/37018133361 The previous check only covers GCC. Additionally, don't require GLIBC when FreeBSD is used. FreeBSD has it's own libc which supports DEEPBIND according to its docs. Follow-up of #1703, #1707 Signed-off-by: Viktor Söderqvist <[email protected]>

…key-io#1703) The current flags used in `dlopen` to load external modules don't allow modules to define symbols that are already defined by the valkey server binary because the symbol resolution first looks in the server memory and only if it does not find anything, it looks in the module (shared library) memory. This might become a problem if, for instance, we try to implement a new scripting engine based on a newer version of Lua. The Lua interpreter library shares many symbol names with the Lua interpreter included in the Valkey server binary. To fix the above problem, this PR adds the flag `RTLD_DEEPBIND` to the flags used in `dlopen` on systems that support it, which changes the symbol resolution strategy to look for the symbol in the module memory first, if the code executing is from the module. --------- Signed-off-by: Ricardo Dias <[email protected]>

Fixes the failure trying to use dlopen with DEEPBIND with ASAN when compiling with Clang: ==90510==You are trying to dlopen a /home/runner/work/valkey/valkey/tests/modules/keyspecs.so shared library with RTLD_DEEPBIND flag which is incompatible with sanitizer runtime (see google/sanitizers#611 for details). If you want to run /home/runner/work/valkey/valkey/tests/modules/keyspecs.so library under sanitizers please remove RTLD_DEEPBIND from dlopen flags. https://github.com/valkey-io/valkey/actions/runs/13261241213/job/37018133361 The previous check only covers GCC. Additionally, don't require GLIBC when FreeBSD is used. FreeBSD has it's own libc which supports DEEPBIND according to its docs. Follow-up of valkey-io#1703, valkey-io#1707 Signed-off-by: Viktor Söderqvist <[email protected]>

…key-io#1703) The current flags used in `dlopen` to load external modules don't allow modules to define symbols that are already defined by the valkey server binary because the symbol resolution first looks in the server memory and only if it does not find anything, it looks in the module (shared library) memory. This might become a problem if, for instance, we try to implement a new scripting engine based on a newer version of Lua. The Lua interpreter library shares many symbol names with the Lua interpreter included in the Valkey server binary. To fix the above problem, this PR adds the flag `RTLD_DEEPBIND` to the flags used in `dlopen` on systems that support it, which changes the symbol resolution strategy to look for the symbol in the module memory first, if the code executing is from the module. --------- Signed-off-by: Ricardo Dias <[email protected]>

Fixes the failure trying to use dlopen with DEEPBIND with ASAN when compiling with Clang: ==90510==You are trying to dlopen a /home/runner/work/valkey/valkey/tests/modules/keyspecs.so shared library with RTLD_DEEPBIND flag which is incompatible with sanitizer runtime (see google/sanitizers#611 for details). If you want to run /home/runner/work/valkey/valkey/tests/modules/keyspecs.so library under sanitizers please remove RTLD_DEEPBIND from dlopen flags. https://github.com/valkey-io/valkey/actions/runs/13261241213/job/37018133361 The previous check only covers GCC. Additionally, don't require GLIBC when FreeBSD is used. FreeBSD has it's own libc which supports DEEPBIND according to its docs. Follow-up of valkey-io#1703, valkey-io#1707 Signed-off-by: Viktor Söderqvist <[email protected]>

…key-io#1703) The current flags used in `dlopen` to load external modules don't allow modules to define symbols that are already defined by the valkey server binary because the symbol resolution first looks in the server memory and only if it does not find anything, it looks in the module (shared library) memory. This might become a problem if, for instance, we try to implement a new scripting engine based on a newer version of Lua. The Lua interpreter library shares many symbol names with the Lua interpreter included in the Valkey server binary. To fix the above problem, this PR adds the flag `RTLD_DEEPBIND` to the flags used in `dlopen` on systems that support it, which changes the symbol resolution strategy to look for the symbol in the module memory first, if the code executing is from the module. --------- Signed-off-by: Ricardo Dias <[email protected]>

Fixes the failure trying to use dlopen with DEEPBIND with ASAN when compiling with Clang: ==90510==You are trying to dlopen a /home/runner/work/valkey/valkey/tests/modules/keyspecs.so shared library with RTLD_DEEPBIND flag which is incompatible with sanitizer runtime (see google/sanitizers#611 for details). If you want to run /home/runner/work/valkey/valkey/tests/modules/keyspecs.so library under sanitizers please remove RTLD_DEEPBIND from dlopen flags. https://github.com/valkey-io/valkey/actions/runs/13261241213/job/37018133361 The previous check only covers GCC. Additionally, don't require GLIBC when FreeBSD is used. FreeBSD has it's own libc which supports DEEPBIND according to its docs. Follow-up of valkey-io#1703, valkey-io#1707 Signed-off-by: Viktor Söderqvist <[email protected]>

rjd15372 requested review from madolson and zuiderkwast February 10, 2025 14:48

rjd15372 added 2 commits February 10, 2025 15:12

Fix macosx build error.

8594cb9

Signed-off-by: Ricardo Dias <[email protected]>

Rename OSX to macos

0b59455

Signed-off-by: Ricardo Dias <[email protected]>

zuiderkwast approved these changes Feb 10, 2025

View reviewed changes

zuiderkwast reviewed Feb 10, 2025

View reviewed changes

src/module.c Outdated Show resolved Hide resolved

Changed condition to only use DEEPBIND in Linux and FreeBSD builds

a221102

Signed-off-by: Ricardo Dias <[email protected]>

zuiderkwast merged commit 244c570 into valkey-io:unstable Feb 10, 2025
50 checks passed

zuiderkwast added the release-notes This issue should get a line item in the release notes label Feb 10, 2025

zuiderkwast mentioned this pull request Feb 11, 2025

Correctly detect address sanitizer with Clang #1712

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `DEEPBIND` flag when loading external modules using `dlopen`#1703

Use `DEEPBIND` flag when loading external modules using `dlopen`#1703
zuiderkwast merged 4 commits intovalkey-io:unstablefrom
rjd15372:dlopen

rjd15372 commented Feb 10, 2025 •

edited by zuiderkwast

Loading

Uh oh!

codecov bot commented Feb 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

zuiderkwast commented Feb 11, 2025

Uh oh!

enjoy-binbin commented Feb 11, 2025

Uh oh!

zuiderkwast commented Feb 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

rjd15372 commented Feb 10, 2025 • edited by zuiderkwast Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

zuiderkwast commented Feb 11, 2025

Uh oh!

enjoy-binbin commented Feb 11, 2025

Uh oh!

zuiderkwast commented Feb 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rjd15372 commented Feb 10, 2025 •

edited by zuiderkwast

Loading

codecov bot commented Feb 10, 2025 •

edited

Loading