Skip to content

Commit

Permalink
FFI: Workaround for platform dlerror() returning NULL.
Browse files Browse the repository at this point in the history
Contributed by mcclure.

(cherry picked from commit 478bcfe)

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.

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
  • Loading branch information
Mike Pall authored and Buristan committed Sep 13, 2024
1 parent d9e19ae commit 2dc2c33
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/lj_clib.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions test/tarantool-tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 27 additions & 0 deletions test/tarantool-tests/lj-522-fix-dlerror-return-null.test.lua
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
BuildTestCLib(mydlerror mydlerror.c)
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <stddef.h>

char *dlerror(void)
{
return NULL;
}
10 changes: 10 additions & 0 deletions test/tarantool-tests/lj-522-fix-dlerror-return-null/script.lua
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 2dc2c33

Please sign in to comment.