From: Waldemar Kozaczuk <jwkozac...@gmail.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master
elf: fix thread visiblity issue
The commit fe37650fbbcd352806bab3e9e70949a65fadf29b to address
the issue #1067 relaxed the elf visibility rules while being loaded
to child threads so that they can resolve symbols if any threads
are created by the INIT functions.
Unfortunately inadvertently that patch allowed access to elf
ebjects too early in some cases and would lead to a crash. Here is a
concrete scenario when this would happen with the 'cli' app:
1. Thread A resolves objects of the httpserver app.
2. Thread A starts the httpserver app on new thread B.
3. Thread A continues to load objects of the cli app.
4. Thread B tries to resolve some symbols through elf_resolve_pltgot() for
example
and ends up attempting to look up symbols of cli object that is just
being loaded
by thread A and is not "ready" yet. However because thread B is a child
of A
it can do so.
Clearly the example above illustrates a hole in the original patch.
This patch fills this hole by restricting back the access to the symbols
of the object being loaded to the thread doing it only as it used to be be.
And it only relaxes the access to child threads for the brief time while
object
is being initialized.
It does so by introducing the elf access visibility levels:
- Public
- ThreadOnly
- ThreadAndItsChildren
The Public and ThreadOnly and equivalent to the old - nullptr, non-nullptr
visibility attribute.
The ThreadAndItsChildren is only set to for the period the INIT functions
are being called.
References #1067
Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
Message-Id: <20200111180020.22335-1-jwkozac...@gmail.com>
---
diff --git a/core/elf.cc b/core/elf.cc
--- a/core/elf.cc
+++ b/core/elf.cc
@@ -122,7 +122,8 @@ object::object(program& prog, std::string pathname)
, _dynamic_table(nullptr)
, _module_index(_prog.register_dtv(this))
, _is_executable(false)
- , _visibility(nullptr)
+ , _visibility_thread(nullptr)
+ , _visibility_level(VisibilityLevel::Public)
{
elf_debug("Instantiated\n");
}
@@ -140,22 +141,34 @@ ulong object::module_index() const
bool object::visible(void) const
{
- auto v = _visibility.load(std::memory_order_acquire);
- if (v == nullptr) {
+ auto level = _visibility_level.load(std::memory_order_acquire);
+ if (level == VisibilityLevel::Public) {
return true;
}
+
+ auto thread = _visibility_thread.load(std::memory_order_acquire);
+ if (!thread) { //Unlikely, but ...
+ return true;
+ }
+
+ if (level == VisibilityLevel::ThreadOnly) {
+ return thread == sched::thread::current();
+ }
+
+ // Is current thread the same as "thread" or is it a child ?`
for (auto t = sched::thread::current(); t != nullptr; t = t->parent())
{
- if (t == v) {
+ if (t == thread) {
return true;
}
}
return false;
}
-void object::setprivate(bool priv)
+void object::set_visibility(elf::VisibilityLevel visibilityLevel)
{
- _visibility.store(priv ? sched::thread::current() : nullptr,
+ _visibility_thread.store(visibilityLevel == VisibilityLevel::Public ?
nullptr : sched::thread::current(),
std::memory_order_release);
+ _visibility_level.store(visibilityLevel, std::memory_order_release);
}
@@ -1323,7 +1336,7 @@ program::load_object(std::string name,
std::vector<std::string> extra_path,
auto ef = std::shared_ptr<object>(new file(*this, f, name),
[=](object *obj) { remove_object(obj); });
ef->set_base(_next_alloc);
- ef->setprivate(true);
+ ef->set_visibility(ThreadOnly);
// We need to push the object at the end of the list (so that the
main
// shared object gets searched before the shared libraries it
uses),
// with one exception: the kernel needs to remain at the end of the
@@ -1395,13 +1408,13 @@ void program::init_library(int argc, char** argv)
// first) and finally make the loaded objects visible in search
order.
auto size = loaded_objects.size();
for (unsigned i = 0; i < size; i++) {
- loaded_objects[i]->setprivate(true);
+ loaded_objects[i]->set_visibility(ThreadAndItsChildren);
}
for (int i = size - 1; i >= 0; i--) {
loaded_objects[i]->run_init_funcs(argc, argv);
}
for (unsigned i = 0; i < size; i++) {
- loaded_objects[i]->setprivate(false);
+ loaded_objects[i]->set_visibility(Public);
}
_loaded_objects_stack.pop();
}
diff --git a/include/osv/elf.hh b/include/osv/elf.hh
--- a/include/osv/elf.hh
+++ b/include/osv/elf.hh
@@ -329,6 +329,12 @@ struct [[gnu::packed]] Elf64_Shdr {
Elf64_Xword sh_entsize; /* Size of entries, if section has table */
};
+enum VisibilityLevel {
+ Public,
+ ThreadOnly,
+ ThreadAndItsChildren
+};
+
class object: public std::enable_shared_from_this<elf::object> {
public:
explicit object(program& prog, std::string pathname);
@@ -452,10 +458,11 @@ protected:
return _static_tls_offset + get_tls_size();
}
private:
- std::atomic<void*> _visibility;
+ std::atomic<void*> _visibility_thread;
+ std::atomic<VisibilityLevel> _visibility_level;
bool visible(void) const;
public:
- void setprivate(bool);
+ void set_visibility(VisibilityLevel);
};
class file : public object {
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/000000000000a4cba6059bed0dd0%40google.com.
