Skip to content

Commit

Permalink
LibCoredump: Respect coredump's LD_LIBRARY_PATH when searching libraries
Browse files Browse the repository at this point in the history
Previously, we would only resolve libraries from `/usr/lib`, which is
not the only path from which the crashed process could've loaded the
libraries from.
  • Loading branch information
speles authored and awesomekling committed Feb 14, 2022
1 parent 334ed92 commit a666140
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 19 deletions.
8 changes: 3 additions & 5 deletions Userland/Libraries/LibCoredump/Backtrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@

namespace Coredump {

ELFObjectInfo const* Backtrace::object_info_for_region(MemoryRegionInfo const& region)
ELFObjectInfo const* Backtrace::object_info_for_region(Reader const& coredump, MemoryRegionInfo const& region)
{
String path = region.object_name();
if (!path.starts_with('/') && Core::File::looks_like_shared_library(path))
path = LexicalPath::join("/usr/lib", path).string();
String path = coredump.resolve_object_path(region.object_name());

auto maybe_ptr = m_debug_info_cache.get(path);
if (maybe_ptr.has_value())
Expand Down Expand Up @@ -116,7 +114,7 @@ void Backtrace::add_entry(const Reader& coredump, FlatPtr ip)
// the PT_LOAD header for the .text segment isn't the first one
// in the object file.
auto region = coredump.first_region_for_object(object_name);
auto object_info = object_info_for_region(*region);
auto object_info = object_info_for_region(coredump, *region);
if (!object_info) {
m_entries.append({ ip, object_name, {}, {} });
return;
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibCoredump/Backtrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Backtrace {

private:
void add_entry(const Reader&, FlatPtr ip);
ELFObjectInfo const* object_info_for_region(MemoryRegionInfo const&);
ELFObjectInfo const* object_info_for_region(Reader const&, MemoryRegionInfo const&);

bool m_skip_loader_so { false };
ELF::Core::ThreadInfo m_thread_info;
Expand Down
59 changes: 46 additions & 13 deletions Userland/Libraries/LibCoredump/Reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
#include <AK/HashTable.h>
#include <AK/JsonObject.h>
#include <AK/JsonValue.h>
#include <AK/LexicalPath.h>
#include <LibCompress/Gzip.h>
#include <LibCore/File.h>
#include <LibCoredump/Reader.h>
#include <signal_numbers.h>
#include <string.h>
#include <unistd.h>

namespace Coredump {

Expand Down Expand Up @@ -277,13 +279,7 @@ const Reader::LibraryData* Reader::library_containing(FlatPtr address) const
return {};

auto name = region->object_name();

String path;
if (Core::File::looks_like_shared_library(name))
path = String::formatted("/usr/lib/{}", name);
else {
path = name;
}
String path = resolve_object_path(name);

if (!cached_libs.contains(path)) {
auto file_or_error = Core::MappedFile::map(path);
Expand All @@ -297,6 +293,48 @@ const Reader::LibraryData* Reader::library_containing(FlatPtr address) const
return lib_data;
}

String Reader::resolve_object_path(StringView name) const
{
// TODO: There are other places where similar method is implemented or would be useful.
// (e.g. UserspaceEmulator, LibSymbolication, Profiler, and DynamicLinker itself)
// We should consider creating unified implementation in the future.

if (name.starts_with('/') || !Core::File::looks_like_shared_library(name)) {
return name;
}

Vector<String> library_search_directories;

// If LD_LIBRARY_PATH is present, check its folders first
for (auto& environment_variable : process_environment()) {
auto prefix = "LD_LIBRARY_PATH="sv;
if (environment_variable.starts_with(prefix)) {
auto ld_library_path = environment_variable.substring_view(prefix.length());

// FIXME: This code won't handle folders with ":" in the name correctly.
for (auto directory : ld_library_path.split_view(':')) {
library_search_directories.append(directory);
}
}
}

// Add default paths that DynamicLinker uses
library_search_directories.append("/usr/lib/"sv);
library_search_directories.append("/usr/local/lib/"sv);

// Search for the first readable library file
for (auto& directory : library_search_directories) {
auto full_path = LexicalPath::join(directory, name).string();

if (access(full_path.characters(), R_OK) != 0)
continue;

return full_path;
}

return name;
}

void Reader::for_each_library(Function<void(LibraryInfo)> func) const
{
HashTable<String> libraries;
Expand All @@ -307,12 +345,7 @@ void Reader::for_each_library(Function<void(LibraryInfo)> func) const

libraries.set(name);

String path;
if (Core::File::looks_like_shared_library(name))
path = String::formatted("/usr/lib/{}", name);
else {
path = name;
}
String path = resolve_object_path(name);

func(LibraryInfo { name, path, static_cast<FlatPtr>(region.region_start) });
return IterationDecision::Continue;
Expand Down
2 changes: 2 additions & 0 deletions Userland/Libraries/LibCoredump/Reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ class Reader {
};
const LibraryData* library_containing(FlatPtr address) const;

String resolve_object_path(StringView object_name) const;

int process_pid() const;
u8 process_termination_signal() const;
String process_executable_path() const;
Expand Down

0 comments on commit a666140

Please sign in to comment.