[PATCH] elf: ignore versioning table if symbol is looked up by object itself

[Waldemar Kozaczuk]

The commit https://github.com/cloudius-systems/osv/commit/ed1eed7a567ec17138c65f0a5628c2311603c712
enhanced dynamic linker to skip old symbols per versions table. Unfortunately
the relevant code does not take into account whether the old symbol is being
looked up by the object itself.

This sentence from https://www.akkadia.org/drepper/symbol-versioning -
"If the highest bit (bit 15) is set this is a hidden symbol which cannot
be referenced from outside the object." - seems to indicate the old
symbols should be visible to the object itself.

This patch enhances lookup method to a help track "who" is looking
and if it is the object itself, the the versions table is ignored.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
arch/x64/arch-elf.cc | 12 ++++++------
core/elf.cc | 24 ++++++++++++------------
include/osv/elf.hh | 8 ++++----
libc/dlfcn.cc | 4 ++--
4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/arch/x64/arch-elf.cc b/arch/x64/arch-elf.cc
index 9e793760..09b520d0 100644
--- a/arch/x64/arch-elf.cc
+++ b/arch/x64/arch-elf.cc
@@ -71,7 +71,7 @@ bool object::arch_relocate_rela(u32 type, u32 sym, void *addr,
break;
}
case R_X86_64_64: {
- auto _sym = symbol(sym, true);
+ auto _sym = symbol(sym, this, true);
if (_sym.symbol) {
*static_cast<void**>(addr) = _sym.relocated_addr() + addend;
} else {
@@ -84,7 +84,7 @@ bool object::arch_relocate_rela(u32 type, u32 sym, void *addr,
break;
case R_X86_64_JUMP_SLOT:
case R_X86_64_GLOB_DAT: {
- auto _sym = symbol(sym, true);
+ auto _sym = symbol(sym, this, true);
if (_sym.symbol) {
*static_cast<void**>(addr) = _sym.relocated_addr();
} else {
@@ -112,20 +112,20 @@ bool object::arch_relocate_rela(u32 type, u32 sym, void *addr,
} else {
// The thread-local variable being accessed is located
// in DIFFERENT shared object that the caller
- *static_cast<u64*>(addr) = symbol(sym).obj->_module_index;
+ *static_cast<u64*>(addr) = symbol(sym, this).obj->_module_index;
}
break;
case R_X86_64_DTPOFF64:
// The thread-local variable being accessed is located
// in DIFFERENT shared object that the caller
- *static_cast<u64*>(addr) = symbol(sym).symbol->st_value;
+ *static_cast<u64*>(addr) = symbol(sym, this).symbol->st_value;
break;
case R_X86_64_TPOFF64:
// This type is intended to resolve symbols of thread-local variables in static TLS
// accessed in initial-exec mode and is handled to calculate the virtual address of
// target thread-local variable
if (sym) {
- auto sm = symbol(sym);
+ auto sm = symbol(sym, this);
ulong tls_offset;
if (sm.obj->is_executable()) {
// If this is an executable (pie or position-dependant one)
@@ -158,7 +158,7 @@ bool object::arch_relocate_rela(u32 type, u32 sym, void *addr,

bool object::arch_relocate_jump_slot(u32 sym, void *addr, Elf64_Sxword addend, bool ignore_missing)
{
- auto _sym = symbol(sym, ignore_missing);
+ auto _sym = symbol(sym, this, ignore_missing);
if (_sym.symbol) {
*static_cast<void**>(addr) = _sym.relocated_addr();
return true;
diff --git a/core/elf.cc b/core/elf.cc
index a61bb035..d7255a8e 100644
--- a/core/elf.cc
+++ b/core/elf.cc
@@ -144,7 +144,7 @@ void object::setprivate(bool priv)
template <>
void* object::lookup(const char* symbol)
{
- symbol_module sm{lookup_symbol(symbol), this};
+ symbol_module sm{lookup_symbol(symbol, false), this};
if (!sm.symbol || sm.symbol->st_shndx == SHN_UNDEF) {
return nullptr;
}
@@ -623,14 +623,14 @@ static std::string demangle(const char *name) {
return ret;
}

-symbol_module object::symbol(unsigned idx, bool ignore_missing)
+symbol_module object::symbol(unsigned idx, object *looking, bool ignore_missing)
{
auto symtab = dynamic_ptr<Elf64_Sym>(DT_SYMTAB);
assert(dynamic_val(DT_SYMENT) == sizeof(Elf64_Sym));
auto sym = &symtab[idx];
auto nameidx = sym->st_name;
auto name = dynamic_ptr<const char>(DT_STRTAB) + nameidx;
- auto ret = _prog.lookup(name);
+ auto ret = _prog.lookup(name, looking);
auto binding = symbol_binding(*sym);
if (!ret.symbol && binding == STB_WEAK) {
return symbol_module(sym, this);
@@ -659,7 +659,7 @@ symbol_module object::symbol_other(unsigned idx)
for (auto module : ml.objects) {
if (module == this)
continue; // do not match this module
- if (auto sym = module->lookup_symbol(name)) {
+ if (auto sym = module->lookup_symbol(name, false)) {
ret = symbol_module(sym, module);
break;
}
@@ -761,7 +761,7 @@ void* object::resolve_pltgot(unsigned index)
u32 type = info & 0xffffffff;
assert(type == ARCH_JUMP_SLOT);
void *addr = _base + slot.r_offset;
- auto sm = symbol(sym);
+ auto sm = symbol(sym, this);

if (sm.obj != this) {
WITH_LOCK(_used_by_resolve_plt_got_mutex) {
@@ -833,7 +833,7 @@ dl_new_hash(const char *s)
return h & 0xffffffff;
}

-Elf64_Sym* object::lookup_symbol_gnu(const char* name)
+Elf64_Sym* object::lookup_symbol_gnu(const char* name, bool self_lookup)
{
auto symtab = dynamic_ptr<Elf64_Sym>(DT_SYMTAB);
auto strtab = dynamic_ptr<char>(DT_STRTAB);
@@ -857,7 +857,7 @@ Elf64_Sym* object::lookup_symbol_gnu(const char* name)
if (idx == 0) {
return nullptr;
}
- auto version_symtab = dynamic_exists(DT_VERSYM) ? dynamic_ptr<Elf64_Versym>(DT_VERSYM) : nullptr;
+ auto version_symtab = (!self_lookup && dynamic_exists(DT_VERSYM)) ? dynamic_ptr<Elf64_Versym>(DT_VERSYM) : nullptr;
do {
if ((chains[idx] & ~1) != (hashval & ~1)) {
continue;
@@ -872,14 +872,14 @@ Elf64_Sym* object::lookup_symbol_gnu(const char* name)
return nullptr;
}

-Elf64_Sym* object::lookup_symbol(const char* name)
+Elf64_Sym* object::lookup_symbol(const char* name, bool self_lookup)
{
if (!visible()) {
return nullptr;
}
Elf64_Sym* sym;
if (dynamic_exists(DT_GNU_HASH)) {
- sym = lookup_symbol_gnu(name);
+ sym = lookup_symbol_gnu(name, self_lookup);
} else {
sym = lookup_symbol_old(name);
}
@@ -1428,14 +1428,14 @@ void program::del_debugger_obj(object* obj)
}
}

-symbol_module program::lookup(const char* name)
+symbol_module program::lookup(const char* name, object* looking)
{
trace_elf_lookup(name);
symbol_module ret(nullptr,nullptr);
with_modules([&](const elf::program::modules_list &ml)
{
for (auto module : ml.objects) {
- if (auto sym = module->lookup_symbol(name)) {
+ if (auto sym = module->lookup_symbol(name, looking == module)) {
ret = symbol_module(sym, module);
return;
}
@@ -1446,7 +1446,7 @@ symbol_module program::lookup(const char* name)

void* program::do_lookup_function(const char* name)
{
- auto sym = lookup(name);
+ auto sym = lookup(name, nullptr);
if (!sym.symbol) {
throw std::runtime_error("symbol not found " + demangle(name));
}
diff --git a/include/osv/elf.hh b/include/osv/elf.hh
index 7043ec74..289c1f0e 100644
--- a/include/osv/elf.hh
+++ b/include/osv/elf.hh
@@ -340,7 +340,7 @@ public:
void set_dynamic_table(Elf64_Dyn* dynamic_table);
void* base() const;
void* end() const;
- Elf64_Sym* lookup_symbol(const char* name);
+ Elf64_Sym* lookup_symbol(const char* name, bool self_lookup);
void load_segments();
void unload_segments();
void fix_permissions();
@@ -382,7 +382,7 @@ protected:
unsigned get_segment_mmap_permissions(const Elf64_Phdr& phdr);
private:
Elf64_Sym* lookup_symbol_old(const char* name);
- Elf64_Sym* lookup_symbol_gnu(const char* name);
+ Elf64_Sym* lookup_symbol_gnu(const char* name, bool self_lookup);
template <typename T>
T* dynamic_ptr(unsigned tag);
Elf64_Xword dynamic_val(unsigned tag);
@@ -391,7 +391,7 @@ private:
std::vector<const char*> dynamic_str_array(unsigned tag);
Elf64_Dyn& dynamic_tag(unsigned tag);
Elf64_Dyn* _dynamic_tag(unsigned tag);
- symbol_module symbol(unsigned idx, bool ignore_missing = false);
+ symbol_module symbol(unsigned idx, object *looking, bool ignore_missing = false);
symbol_module symbol_other(unsigned idx);
Elf64_Xword symbol_tls_module(unsigned idx);
void relocate_rela();
@@ -575,7 +575,7 @@ public:
*/
void set_search_path(std::initializer_list<std::string> path);

- symbol_module lookup(const char* symbol);
+ symbol_module lookup(const char* symbol, object* looking);
template <typename T>
T* lookup_function(const char* symbol);

diff --git a/libc/dlfcn.cc b/libc/dlfcn.cc
index 8cfb7b2a..7b185d3e 100644
--- a/libc/dlfcn.cc
+++ b/libc/dlfcn.cc
@@ -68,13 +68,13 @@ void* dlsym(void* handle, const char* name)
elf::symbol_module sym;
auto program = elf::get_program();
if ((program == handle) || (handle == RTLD_DEFAULT)) {
- sym = program->lookup(name);
+ sym = program->lookup(name, nullptr);
} else if (handle == RTLD_NEXT) {
// FIXME: implement
abort();
} else {
auto obj = *reinterpret_cast<std::shared_ptr<elf::object>*>(handle);
- sym = { obj->lookup_symbol(name), obj.get() };
+ sym = { obj->lookup_symbol(name, true), obj.get() };
}
if (!sym.obj || !sym.symbol) {
dlerror_fmt("dlsym: symbol %s not found", name);
--
2.20.1

[Waldek Kozaczuk]

This patch touches many places in the dynamic linker code so I wonder if there is a simpler way to accomplish the same thing.

I am also not 100% use I am passing the right "looking" object in all places.

Waldek