Skip to content

Commit

Permalink
Validate defined versions in prelink_image
Browse files Browse the repository at this point in the history 
Validate the list of defined versions explicitly, during library
prelinking, rather than implicitly as part of constructing the
VersionTracker in soinfo::link_image.

Doing the validation upfront allows removing the symbol lookup failure
code paths, which only happen on a library with invalid version
information.

Helps on the walleye 64-bit linker relocation benchmark (146.2ms ->
131.6ms)

Bug: none
Test: bionic unit tests
Change-Id: Id17508aba3af2863909f0526897c4277419322b7
  • Loading branch information
@rprichard
rprichard committed Jan 7, 2020
1 parent ae320cd commit 0e12cce
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 90 deletions.
69 changes: 29 additions & 40 deletions linker/linker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -508,42 +508,29 @@ bool soinfo_do_lookup(soinfo* si_from, const char* name, const version_info* vi,
*/
if (si_from->has_DT_SYMBOLIC) {
DEBUG("%s: looking up %s in local scope (DT_SYMBOLIC)", si_from->get_realpath(), name);
if (!si_from->find_symbol_by_name(symbol_name, vi, &s)) {
return false;
}

s = si_from->find_symbol_by_name(symbol_name, vi);
if (s != nullptr) {
*si_found_in = si_from;
}
}

// 1. Look for it in global_group
if (s == nullptr) {
bool error = false;
global_group.visit([&](soinfo* global_si) {
DEBUG("%s: looking up %s in %s (from global group)",
si_from->get_realpath(), name, global_si->get_realpath());
if (!global_si->find_symbol_by_name(symbol_name, vi, &s)) {
error = true;
return false;
}

s = global_si->find_symbol_by_name(symbol_name, vi);
if (s != nullptr) {
*si_found_in = global_si;
return false;
}

return true;
});

if (error) {
return false;
}
}

// 2. Look for it in the local group
if (s == nullptr) {
bool error = false;
local_group.visit([&](soinfo* local_si) {
if (local_si == si_from && si_from->has_DT_SYMBOLIC) {
// we already did this - skip
Expand All @@ -552,22 +539,14 @@ bool soinfo_do_lookup(soinfo* si_from, const char* name, const version_info* vi,

DEBUG("%s: looking up %s in %s (from local group)",
si_from->get_realpath(), name, local_si->get_realpath());
if (!local_si->find_symbol_by_name(symbol_name, vi, &s)) {
error = true;
return false;
}

s = local_si->find_symbol_by_name(symbol_name, vi);
if (s != nullptr) {
*si_found_in = local_si;
return false;
}

return true;
});

if (error) {
return false;
}
}

if (s != nullptr) {
Expand Down Expand Up @@ -863,11 +842,7 @@ static const ElfW(Sym)* dlsym_handle_lookup(android_namespace_t* ns,
return kWalkSkip;
}

if (!current_soinfo->find_symbol_by_name(symbol_name, vi, &result)) {
result = nullptr;
return kWalkStop;
}

result = current_soinfo->find_symbol_by_name(symbol_name, vi);
if (result != nullptr) {
*found = current_soinfo;
return kWalkStop;
Expand Down Expand Up @@ -947,10 +922,7 @@ static const ElfW(Sym)* dlsym_linear_lookup(android_namespace_t* ns,
continue;
}

if (!si->find_symbol_by_name(symbol_name, vi, &s)) {
return nullptr;
}

s = si->find_symbol_by_name(symbol_name, vi);
if (s != nullptr) {
*found = si;
break;
Expand Down Expand Up @@ -2819,25 +2791,38 @@ static bool for_each_verdef(const soinfo* si, F functor) {
return true;
}

bool find_verdef_version_index(const soinfo* si, const version_info* vi, ElfW(Versym)* versym) {
ElfW(Versym) find_verdef_version_index(const soinfo* si, const version_info* vi) {
if (vi == nullptr) {
*versym = kVersymNotNeeded;
return true;
return kVersymNotNeeded;
}

*versym = kVersymGlobal;
ElfW(Versym) result = kVersymGlobal;

return for_each_verdef(si,
if (!for_each_verdef(si,
[&](size_t, const ElfW(Verdef)* verdef, const ElfW(Verdaux)* verdaux) {
if (verdef->vd_hash == vi->elf_hash &&
strcmp(vi->name, si->get_string(verdaux->vda_name)) == 0) {
*versym = verdef->vd_ndx;
result = verdef->vd_ndx;
return true;
}

return false;
}
);
)) {
// verdef should have already been validated in prelink_image.
async_safe_fatal("invalid verdef after prelinking: %s, %s",
si->get_realpath(), linker_get_error_buffer());
}

return result;
}

// Validate the library's verdef section. On error, returns false and invokes DL_ERR.
bool validate_verdef_section(const soinfo* si) {
return for_each_verdef(si,
[&](size_t, const ElfW(Verdef)*, const ElfW(Verdaux)*) {
return false;
});
}

bool VersionTracker::init_verdef(const soinfo* si_from) {
Expand Down Expand Up @@ -3842,6 +3827,10 @@ bool soinfo::prelink_image() {
// Don't call add_dlwarning because a missing DT_SONAME isn't important enough to show in the UI
}

// Validate each library's verdef section once, so we don't have to validate
// it each time we look up a symbol with a version.
if (!validate_verdef_section(this)) return false;

flags_ |= FLAG_PRELINKED;
return true;
}
Expand Down
2 changes: 2 additions & 0 deletions linker/linker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,3 +204,5 @@ struct address_space_params {
size_t reserved_size = 0;
bool must_use_address = false;
};

ElfW(Versym) find_verdef_version_index(const soinfo* si, const version_info* vi);
3 changes: 1 addition & 2 deletions linker/linker_cfi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,7 @@ static soinfo* find_libdl(soinfo* solist) {

static uintptr_t soinfo_find_symbol(soinfo* si, const char* s) {
SymbolName name(s);
const ElfW(Sym) * sym;
if (si->find_symbol_by_name(name, nullptr, &sym) && sym) {
if (const ElfW(Sym)* sym = si->find_symbol_by_name(name, nullptr)) {
return si->resolve_symbol_address(sym);
}
return 0;
Expand Down
58 changes: 16 additions & 42 deletions linker/linker_soinfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@

#include <async_safe/log.h>

#include "linker.h"
#include "linker_config.h"
#include "linker_debug.h"
#include "linker_globals.h"
#include "linker_logger.h"
#include "linker_utils.h"

// TODO(dimitry): These functions are currently located in linker.cpp - find a better place for it
bool find_verdef_version_index(const soinfo* si, const version_info* vi, ElfW(Versym)* versym);
ElfW(Addr) call_ifunc_resolver(ElfW(Addr) resolver_addr);
int get_application_target_sdk_version();

Expand Down Expand Up @@ -135,22 +135,6 @@ size_t soinfo::get_verdef_cnt() const {
return 0;
}

bool soinfo::find_symbol_by_name(SymbolName& symbol_name,
const version_info* vi,
const ElfW(Sym)** symbol) const {
uint32_t symbol_index;
bool success =
is_gnu_hash() ?
gnu_lookup(symbol_name, vi, &symbol_index) :
elf_lookup(symbol_name, vi, &symbol_index);

if (success) {
*symbol = symbol_index == 0 ? nullptr : symtab_ + symbol_index;
}

return success;
}

static bool is_symbol_global_and_defined(const soinfo* si, const ElfW(Sym)* s) {
if (ELF_ST_BIND(s->st_info) == STB_GLOBAL ||
ELF_ST_BIND(s->st_info) == STB_WEAK) {
Expand All @@ -177,18 +161,19 @@ static inline bool check_symbol_version(const ElfW(Versym) verneed,
verneed == (*verdef & ~kVersymHiddenBit);
}

bool soinfo::gnu_lookup(SymbolName& symbol_name,
const version_info* vi,
uint32_t* symbol_index) const {
const ElfW(Sym)* soinfo::find_symbol_by_name(SymbolName& symbol_name,
const version_info* vi) const {
return is_gnu_hash() ? gnu_lookup(symbol_name, vi) : elf_lookup(symbol_name, vi);
}

const ElfW(Sym)* soinfo::gnu_lookup(SymbolName& symbol_name, const version_info* vi) const {
uint32_t hash = symbol_name.gnu_hash();
uint32_t h2 = hash >> gnu_shift2_;

uint32_t bloom_mask_bits = sizeof(ElfW(Addr))*8;
uint32_t word_num = (hash / bloom_mask_bits) & gnu_maskwords_;
ElfW(Addr) bloom_word = gnu_bloom_filter_[word_num];

*symbol_index = 0;

TRACE_TYPE(LOOKUP, "SEARCH %s in %s@%p (gnu)",
symbol_name.get_name(), get_realpath(), reinterpret_cast<void*>(base));

Expand All @@ -197,7 +182,7 @@ bool soinfo::gnu_lookup(SymbolName& symbol_name,
TRACE_TYPE(LOOKUP, "NOT FOUND %s in %s@%p",
symbol_name.get_name(), get_realpath(), reinterpret_cast<void*>(base));

return true;
return nullptr;
}

// bloom test says "probably yes"...
Expand All @@ -207,7 +192,7 @@ bool soinfo::gnu_lookup(SymbolName& symbol_name,
TRACE_TYPE(LOOKUP, "NOT FOUND %s in %s@%p",
symbol_name.get_name(), get_realpath(), reinterpret_cast<void*>(base));

return true;
return nullptr;
}

// lookup versym for the version definition in this library
Expand All @@ -216,10 +201,7 @@ bool soinfo::gnu_lookup(SymbolName& symbol_name,
// which implies that the default version can be accepted; the second case results in
// verneed = 1 (kVersymGlobal) and implies that we should ignore versioned symbols
// for this library and consider only *global* ones.
ElfW(Versym) verneed = 0;
if (!find_verdef_version_index(this, vi, &verneed)) {
return false;
}
const ElfW(Versym) verneed = find_verdef_version_index(this, vi);

do {
ElfW(Sym)* s = symtab_ + n;
Expand All @@ -235,30 +217,24 @@ bool soinfo::gnu_lookup(SymbolName& symbol_name,
TRACE_TYPE(LOOKUP, "FOUND %s in %s (%p) %zd",
symbol_name.get_name(), get_realpath(), reinterpret_cast<void*>(s->st_value),
static_cast<size_t>(s->st_size));
*symbol_index = n;
return true;
return symtab_ + n;
}
} while ((gnu_chain_[n++] & 1) == 0);

TRACE_TYPE(LOOKUP, "NOT FOUND %s in %s@%p",
symbol_name.get_name(), get_realpath(), reinterpret_cast<void*>(base));

return true;
return nullptr;
}

bool soinfo::elf_lookup(SymbolName& symbol_name,
const version_info* vi,
uint32_t* symbol_index) const {
const ElfW(Sym)* soinfo::elf_lookup(SymbolName& symbol_name, const version_info* vi) const {
uint32_t hash = symbol_name.elf_hash();

TRACE_TYPE(LOOKUP, "SEARCH %s in %s@%p h=%x(elf) %zd",
symbol_name.get_name(), get_realpath(),
reinterpret_cast<void*>(base), hash, hash % nbucket_);

ElfW(Versym) verneed = 0;
if (!find_verdef_version_index(this, vi, &verneed)) {
return false;
}
const ElfW(Versym) verneed = find_verdef_version_index(this, vi);

for (uint32_t n = bucket_[hash % nbucket_]; n != 0; n = chain_[n]) {
ElfW(Sym)* s = symtab_ + n;
Expand All @@ -276,17 +252,15 @@ bool soinfo::elf_lookup(SymbolName& symbol_name,
symbol_name.get_name(), get_realpath(),
reinterpret_cast<void*>(s->st_value),
static_cast<size_t>(s->st_size));
*symbol_index = n;
return true;
return symtab_ + n;
}
}

TRACE_TYPE(LOOKUP, "NOT FOUND %s in %s@%p %x %zd",
symbol_name.get_name(), get_realpath(),
reinterpret_cast<void*>(base), hash, hash % nbucket_);

*symbol_index = 0;
return true;
return nullptr;
}

ElfW(Sym)* soinfo::find_symbol_by_address(const void* addr) {
Expand Down
10 changes: 4 additions & 6 deletions linker/linker_soinfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,7 @@ struct soinfo {

soinfo_list_t& get_parents();

bool find_symbol_by_name(SymbolName& symbol_name,
const version_info* vi,
const ElfW(Sym)** symbol) const;
const ElfW(Sym)* find_symbol_by_name(SymbolName& symbol_name, const version_info* vi) const;

ElfW(Sym)* find_symbol_by_address(const void* addr);
ElfW(Addr) resolve_symbol_address(const ElfW(Sym)* s) const;
Expand Down Expand Up @@ -310,10 +308,10 @@ struct soinfo {
bool is_image_linked() const;
void set_image_linked();

bool elf_lookup(SymbolName& symbol_name, const version_info* vi, uint32_t* symbol_index) const;
ElfW(Sym)* elf_addr_lookup(const void* addr);
bool gnu_lookup(SymbolName& symbol_name, const version_info* vi, uint32_t* symbol_index) const;
const ElfW(Sym)* gnu_lookup(SymbolName& symbol_name, const version_info* vi) const;
const ElfW(Sym)* elf_lookup(SymbolName& symbol_name, const version_info* vi) const;
ElfW(Sym)* gnu_addr_lookup(const void* addr);
ElfW(Sym)* elf_addr_lookup(const void* addr);

bool lookup_version_info(const VersionTracker& version_tracker, ElfW(Word) sym,
const char* sym_name, const version_info** vi);
Expand Down

0 comments on commit 0e12cce

Please sign in to comment.