<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi, Sergey,</p>
<p>thanks for the patch! LGTM with a minor comment below.<br>
</p>
<div class="moz-cite-prefix">On 13.09.2024 11:05, Sergey Kaplun
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20240913080550.22599-1-skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">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.</pre>
</blockquote>
<p>According to a POSIX standard, it is not Android-specific
behaviour [1]:</p>
<p>> If no dynamic linking errors have occurred since the last
invocation of <i>dlerror</i>(), <i>dlerror</i>() shall return
NULL.</p>
<p>1.
<a class="moz-txt-link-freetext" href="https://pubs.opengroup.org/onlinepubs/009695399/functions/dlerror.html">https://pubs.opengroup.org/onlinepubs/009695399/functions/dlerror.html</a><br>
</p>
<blockquote type="cite"
cite="mid:20240913080550.22599-1-skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">
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: <a class="moz-txt-link-freetext" href="https://github.com/tarantool/luajit/tree/skaplun/lj-522-fix-dlerror-return-null">https://github.com/tarantool/luajit/tree/skaplun/lj-522-fix-dlerror-return-null</a>
Related Issues:
* <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/pull/522">https://github.com/LuaJIT/LuaJIT/pull/522</a>
* <a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/10199">https://github.com/tarantool/tarantool/issues/10199</a>
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)
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>