[Tarantool-patches] [PATCH luajit] FFI: Workaround for platform dlerror() returning NULL.
Sergey Bronnikov
sergeyb at tarantool.org
Fri Sep 13 17:45:25 MSK 2024
Hi, Sergey,
thanks for the patch! LGTM with a minor comment below.
On 13.09.2024 11:05, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Contributed by mcclure.
>
> (cherry picked from commit 478bcfe52a653bf338f17690147fa9f5793f5b42)
>
> The `ffi.load()` implementation assumes the string returned from
> `dlerror()` is non-NULL and immediately dereferences it. This may lead
> to a crash on some platforms like Android (Oculus Quest) where it is
> possible.
According to a POSIX standard, it is not Android-specific behaviour [1]:
> If no dynamic linking errors have occurred since the last invocation
of /dlerror/(), /dlerror/() shall return NULL.
1. https://pubs.opengroup.org/onlinepubs/009695399/functions/dlerror.html
>
> This patch adds the corresponding check and the default "dlopen failed"
> error message.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#10199
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-522-fix-dlerror-return-null
> Related Issues:
> *https://github.com/LuaJIT/LuaJIT/pull/522
> *https://github.com/tarantool/tarantool/issues/10199
>
> src/lj_clib.c | 3 ++-
> test/tarantool-tests/CMakeLists.txt | 1 +
> .../lj-522-fix-dlerror-return-null.test.lua | 27 +++++++++++++++++++
> .../CMakeLists.txt | 1 +
> .../mydlerror.c | 6 +++++
> .../lj-522-fix-dlerror-return-null/script.lua | 10 +++++++
> 6 files changed, 47 insertions(+), 1 deletion(-)
> create mode 100644 test/tarantool-tests/lj-522-fix-dlerror-return-null.test.lua
> create mode 100644 test/tarantool-tests/lj-522-fix-dlerror-return-null/CMakeLists.txt
> create mode 100644 test/tarantool-tests/lj-522-fix-dlerror-return-null/mydlerror.c
> create mode 100644 test/tarantool-tests/lj-522-fix-dlerror-return-null/script.lua
>
> diff --git a/src/lj_clib.c b/src/lj_clib.c
> index 2f11b2e9..9716684c 100644
> --- a/src/lj_clib.c
> +++ b/src/lj_clib.c
> @@ -119,12 +119,13 @@ static void *clib_loadlib(lua_State *L, const char *name, int global)
> RTLD_LAZY | (global?RTLD_GLOBAL:RTLD_LOCAL));
> if (!h) {
> const char *e, *err = dlerror();
> - if (*err == '/' && (e = strchr(err, ':')) &&
> + if (err && *err == '/' && (e = strchr(err, ':')) &&
> (name = clib_resolve_lds(L, strdata(lj_str_new(L, err, e-err))))) {
> h = dlopen(name, RTLD_LAZY | (global?RTLD_GLOBAL:RTLD_LOCAL));
> if (h) return h;
> err = dlerror();
> }
> + if (!err) err = "dlopen failed";
> lj_err_callermsg(L, err);
> }
> return h;
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index e3750bf3..0bd3e6fc 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -28,6 +28,7 @@ add_subdirectory(gh-5813-resolving-of-c-symbols/stripped)
> add_subdirectory(gh-6098-fix-side-exit-patching-on-arm64)
> add_subdirectory(gh-6189-cur_L)
> add_subdirectory(lj-416-xor-before-jcc)
> +add_subdirectory(lj-522-fix-dlerror-return-null)
> add_subdirectory(lj-549-bytecode-loader)
> add_subdirectory(lj-551-bytecode-c-broken-macro)
> add_subdirectory(lj-601-fix-gc-finderrfunc)
> diff --git a/test/tarantool-tests/lj-522-fix-dlerror-return-null.test.lua b/test/tarantool-tests/lj-522-fix-dlerror-return-null.test.lua
> new file mode 100644
> index 00000000..f9f4af6a
> --- /dev/null
> +++ b/test/tarantool-tests/lj-522-fix-dlerror-return-null.test.lua
> @@ -0,0 +1,27 @@
> +local tap = require('tap')
> +local test = tap.test('lj-522-fix-dlerror-return-null'):skipcond({
> + -- XXX: Unfortunately, it's too hard to overload (or even
> + -- impossible, who knows, since Cupertino fellows do not
> + -- provide any information about their system) something from
> + -- dyld sharing cache (includes the <libdyld.dylib> library
> + -- providing `dlerror()`).
> + -- All in all, this test checks the part that is common for all
> + -- platforms, so it's not vital to run this test on macOS since
> + -- everything can be checked on Linux in a much easier way.
> + ['<dlerror> cannot be overridden on macOS'] = jit.os == 'OSX',
> +})
> +
> +test:plan(1)
> +
> +-- `makecmd()` runs <%testname%/script.lua> by
> +-- `LUAJIT_TEST_BINARY` with the given environment and launch
> +-- options.
> +local script = require('utils').exec.makecmd(arg, {
> + env = { LD_PRELOAD = 'mydlerror.so' },
> + redirect = '2>&1',
> +})
> +
> +local output = script()
> +test:like(output, 'dlopen failed', 'correct error message')
> +
> +test:done(true)
> diff --git a/test/tarantool-tests/lj-522-fix-dlerror-return-null/CMakeLists.txt b/test/tarantool-tests/lj-522-fix-dlerror-return-null/CMakeLists.txt
> new file mode 100644
> index 00000000..903da4d3
> --- /dev/null
> +++ b/test/tarantool-tests/lj-522-fix-dlerror-return-null/CMakeLists.txt
> @@ -0,0 +1 @@
> +BuildTestCLib(mydlerror mydlerror.c)
> diff --git a/test/tarantool-tests/lj-522-fix-dlerror-return-null/mydlerror.c b/test/tarantool-tests/lj-522-fix-dlerror-return-null/mydlerror.c
> new file mode 100644
> index 00000000..89e881e4
> --- /dev/null
> +++ b/test/tarantool-tests/lj-522-fix-dlerror-return-null/mydlerror.c
> @@ -0,0 +1,6 @@
> +#include <stddef.h>
> +
> +char *dlerror(void)
> +{
> + return NULL;
> +}
> diff --git a/test/tarantool-tests/lj-522-fix-dlerror-return-null/script.lua b/test/tarantool-tests/lj-522-fix-dlerror-return-null/script.lua
> new file mode 100644
> index 00000000..b2f673c9
> --- /dev/null
> +++ b/test/tarantool-tests/lj-522-fix-dlerror-return-null/script.lua
> @@ -0,0 +1,10 @@
> +local ffi = require('ffi')
> +
> +-- Overloaded `dlerror()` returns NULL after trying to load an
> +-- unexisting file.
> +local res, errmsg = pcall(ffi.load, 'lj-522-fix-dlerror-return-null-unexisted')
> +
> +assert(not res, 'pcall should fail')
> +
> +-- Return the error message to be checked by the TAP.
> +io.write(errmsg)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20240913/172c5be9/attachment.htm>
More information about the Tarantool-patches
mailing list