Bug 1451891 - Fix race conditions in __wrap_dlerror; r=glandium

__wrap_dlerror uses a single pointer for all threads, which means one
thread could get the dlerror result from another thread. Normally this
wouldn't cause crashes. However, because dlerror results come from a
per-thread buffer, if a thread exits and our saved dlerror result came
from that thread, the saved pointer could then refer to invalid memory.

The proper way to fix this is to use TLS and have a per-thread pointer
for __wrap_dlerror. However, instead of using up a TLS slot, this patch
keeps the single pointer for custom messages, and fallback to per-thread
dlerror call for system messages. While the race condition still exists,
I think the risk is acceptable. Even when races occur, they should no
longer cause crashes.

MozReview-Commit-ID: 4hGksidjiVz

--HG--
extra : rebase_source : 373000686c426b81ffd7cee88264e89b7a733957
This commit is contained in:
Jim Chen 2018-07-25 13:59:30 -04:00
parent 844b19fbfe
commit f61f04949b
2 changed files with 16 additions and 12 deletions

View file

@ -75,9 +75,13 @@ __wrap_dlopen(const char *path, int flags)
const char *
__wrap_dlerror(void)
{
const char *error = ElfLoader::Singleton.lastError;
ElfLoader::Singleton.lastError = nullptr;
return error;
const char* error = ElfLoader::Singleton.lastError.exchange(nullptr);
if (error) {
// Return a custom error if available.
return error;
}
// Or fallback to the system error.
return dlerror();
}
void *
@ -92,10 +96,8 @@ __wrap_dlsym(void *handle, const char *symbol)
return h->GetSymbolPtr(symbol);
}
void* sym = dlsym(handle, symbol);
ElfLoader::Singleton.lastError = dlerror();
return sym;
ElfLoader::Singleton.lastError = nullptr; // Use system dlerror.
return dlsym(handle, symbol);
}
int
@ -420,9 +422,9 @@ SystemElf::Load(const char *path, int flags)
return nullptr;
}
ElfLoader::Singleton.lastError = nullptr; // Use system dlerror.
void *handle = dlopen(path, flags);
DEBUG_LOG("dlopen(\"%s\", 0x%x) = %p", path, flags, handle);
ElfLoader::Singleton.lastError = dlerror();
if (handle) {
SystemElf *elf = new SystemElf(path, handle);
ElfLoader::Singleton.Register(elf);
@ -437,17 +439,17 @@ SystemElf::~SystemElf()
if (!dlhandle)
return;
DEBUG_LOG("dlclose(%p [\"%s\"])", dlhandle, GetPath());
ElfLoader::Singleton.lastError = nullptr; // Use system dlerror.
dlclose(dlhandle);
ElfLoader::Singleton.lastError = dlerror();
ElfLoader::Singleton.Forget(this);
}
void *
SystemElf::GetSymbolPtr(const char *symbol) const
{
ElfLoader::Singleton.lastError = nullptr; // Use system dlerror.
void *sym = dlsym(dlhandle, symbol);
DEBUG_LOG("dlsym(%p [\"%s\"], \"%s\") = %p", dlhandle, GetPath(), symbol, sym);
ElfLoader::Singleton.lastError = dlerror();
return sym;
}

View file

@ -8,6 +8,7 @@
#include <vector>
#include <dlfcn.h>
#include <signal.h>
#include "mozilla/Atomics.h"
#include "mozilla/RefCounted.h"
#include "mozilla/RefPtr.h"
#include "mozilla/UniquePtr.h"
@ -446,12 +447,13 @@ protected:
void Forget(LibHandle *handle);
void Forget(CustomElf *handle);
/* Last error. Used for dlerror() */
friend class SystemElf;
friend const char *__wrap_dlerror(void);
friend void *__wrap_dlsym(void *handle, const char *symbol);
friend int __wrap_dlclose(void *handle);
const char *lastError;
/* __wrap_dlerror() returns this custom last error if non-null or the system
* dlerror() value if this is null. Must refer to a string constant. */
mozilla::Atomic<const char*, mozilla::Relaxed> lastError;
private:
ElfLoader() : expect_shutdown(true), lastError(nullptr)