ldso: protect object list with mutex

When we allowed symbol resolution during exceptions, we used the shared
object lock to protect ELF object list manipulation (e.g., dlopen,
dclose) when executing exception unwinding code in the linker.
Unfortunately, sometimes libraries that are loaded by 'dlopen' may raise
exceptions in the process, leading to a deadlock within the unwind code.
In order to resolve this, we now protect the object list operations
(i.e., enqueue, removal, iteration) by a separate mutex. This allows
the shared object interface to throw exceptions.

issue #4071

repos/base/src/lib/ldso/exception.cc

@@ -38,22 +38,22 @@ extern "C" int dl_iterate_phdr(int (*callback) (Phdr_info *info, size_t size, vo
 	int err = 0;
 	Phdr_info info;
 
-	/* forbid object list manipulation during traversal */
+	bool last = false;
-	Mutex::Guard guard(Linker::shared_object_mutex());
+	Linker::for_each_object([&] (Linker::Object &obj) {
 
-	for (Object *e = obj_list_head();e; e = e->next_obj()) {
+		if (last) return;
 
-		info.addr  = e->reloc_base();
+		info.addr  = obj.reloc_base();
-		info.name  = e->name();
+		info.name  = obj.name();
-		info.phdr  = e->file()->phdr.phdr;
+		info.phdr  = obj.file()->phdr.phdr;
-		info.phnum = e->file()->phdr.count;
+		info.phnum = obj.file()->phdr.count;
 
 		if (verbose_exception)
-			log(e->name(), " reloc ", Hex(e->reloc_base()));
+			log(obj.name(), " reloc ", Hex(obj.reloc_base()));
 
 		if ((err = callback(&info, sizeof(Phdr_info), data)))
-			break;
+			last = true;
-	}
+	});
 
 	return err;
 }
@@ -89,27 +89,28 @@ extern "C" unsigned long dl_unwind_find_exidx(unsigned long pc, int *pcount)
 	enum { EXIDX_ENTRY_SIZE = 8 };
 	*pcount = 0;
 
-	/* forbid object list manipulation during traversal */
+	unsigned long value = 0;
-	Mutex::Guard guard(Linker::shared_object_mutex());
 
-	for (Object *e = obj_list_head(); e; e = e->next_obj()) {
+	Linker::for_each_object([&] (Object &obj) {
+
+		if (value) return;
 
 		/* address of first PT_LOAD header */
-		addr_t base = e->reloc_base() + e->file()->start;
+		addr_t base = obj.reloc_base() + obj.file()->start;
 
 		/* is the 'pc' somewhere within this ELF image */
-		if ((pc < base) || (pc >= base + e->file()->size))
+		if ((pc < base) || (pc >= base + obj.file()->size))
-			continue;
+			return;
 
 		/* retrieve PHDR of exception-table segment */
-		Elf::Phdr const *exidx = phdr_exidx(e->file());
+		Elf::Phdr const *exidx = phdr_exidx(obj.file());
 		if (!exidx)
-			continue;
+			return;
 
 		*pcount = exidx->p_memsz / EXIDX_ENTRY_SIZE;
-		return exidx->p_vaddr + e->reloc_base();
+		value = exidx->p_vaddr + obj.reloc_base();
-	}
+	});
 
-	return 0;
+	return value;
 }
 

repos/base/src/lib/ldso/include/linker.h

@@ -115,11 +115,6 @@ namespace Linker {
 	Object &load(Env &, Allocator &md_alloc, char const *path, Dependency &dep,
 	             Keep keep);
 
-	/**
-	 * Returns the head of the global object list
-	 */
-	Object *obj_list_head();
-
 	/**
 	 * Returns the root-dependeny of the dynamic binary
 	 */
@@ -151,6 +146,7 @@ class Linker::Object : private Fifo<Object>::Element,
 	public:
 
 		typedef String<128> Name;
+		class   Object_list;
 
 	protected:
 
@@ -162,6 +158,8 @@ class Linker::Object : private Fifo<Object>::Element,
 		File const *_file       { nullptr };
 		Elf::Addr   _reloc_base { 0 };
 
+		static Object_list &_object_list();
+
 	public:
 
 		void init(Name const &name, Elf::Addr reloc_base)
@@ -177,6 +175,41 @@ class Linker::Object : private Fifo<Object>::Element,
 			_reloc_base = file.reloc_base;
 		}
 
+		class Object_list
+		{
+			private:
+
+				Fifo<Object> _fifo  { };
+				Mutex        _mutex { };
+
+			public:
+
+				void enqueue(Object &obj)
+				{
+					Mutex::Guard guard(_mutex);
+					_fifo.enqueue(obj);
+				}
+
+				void remove(Object &obj)
+				{
+					Mutex::Guard guard(_mutex);
+					_fifo.remove(obj);
+				}
+
+				template <typename FUNC>
+				void for_each(FUNC const &func)
+				{
+					Mutex::Guard guard(_mutex);
+					_fifo.for_each(func);
+				}
+		};
+
+		template <typename FUNC>
+		static void with_object_list(FUNC const func)
+		{
+			func(_object_list());
+		}
+
 		virtual ~Object() { }
 
 		Elf::Addr        reloc_base() const { return _reloc_base; }
@@ -190,15 +223,11 @@ class Linker::Object : private Fifo<Object>::Element,
 		virtual void relocate(Bind) = 0;
 
 		virtual bool keep() const = 0;
+		virtual void force_keep() = 0;
 
 		virtual void load() = 0;
 		virtual bool unload() { return false;}
 
-		/**
-		 * Next object in global object list
-		 */
-		virtual Object *next_obj() const = 0;
-
 		/**
 		 * Next object in initialization list
 		 */
@@ -228,6 +257,19 @@ class Linker::Object : private Fifo<Object>::Element,
 };
 
 
+namespace Linker {
+	/**
+	 * Apply func to each object
+	 */
+	template <typename FUNC>
+	void for_each_object(FUNC const &func)
+	{
+		Object::with_object_list([&] (Object::Object_list &list) {
+			list.for_each(func); });
+	}
+}
+
+
 /**
  * Dependency of object
  */

repos/base/src/lib/ldso/main.cc

@@ -75,10 +75,18 @@ Genode::Mutex &Linker::shared_object_mutex()
 	return _mutex;
 }
 
+
 /**************************************************************
  ** ELF object types (shared object, dynamic binaries, ldso  **
  **************************************************************/
 
+Object::Object_list &Object::_object_list()
+{
+	static Object_list _list;
+	return _list;
+}
+
+
 /**
  * The actual ELF object, one per file
  */
@@ -134,7 +142,8 @@ class Linker::Elf_object : public Object, private Fifo<Elf_object>::Element
 		{
 			/* register for static construction and relocation */
 			Init::list()->insert(this);
-			obj_list()->enqueue(*this);
+			with_object_list([&] (Object_list &list) {
+				list.enqueue(*this); });
 
 			/* add to link map */
 			Debug::state_change(Debug::ADD, nullptr);
@@ -156,7 +165,8 @@ class Linker::Elf_object : public Object, private Fifo<Elf_object>::Element
 			Debug::state_change(Debug::CONSISTENT, nullptr);
 
 			/* remove from loaded objects list */
-			obj_list()->remove(*this);
+			with_object_list([&] (Object_list &list) {
+				list.remove(*this); });
 			Init::list()->remove(this);
 		}
 
@@ -201,7 +211,7 @@ class Linker::Elf_object : public Object, private Fifo<Elf_object>::Element
 			Link_map::make_first(&_map);
 		}
 
-		void force_keep() { _keep = KEEP; }
+		void force_keep() override { _keep = KEEP; }
 
 		Link_map const &link_map() const override { return _map; }
 		Dynamic  const &dynamic()  const override { return _dyn; }
@@ -234,22 +244,6 @@ class Linker::Elf_object : public Object, private Fifo<Elf_object>::Element
 		 */
 		Object *next_init() const override { return _next_object(); }
 
-		/**
-		 * Next in object list
-		 */
-		Object *next_obj() const override {
-			return Fifo<Elf_object>::Element::next();
-		}
-
-		/**
-		 * Object list
-		 */
-		static Fifo<Elf_object> *obj_list()
-		{
-			static Fifo<Elf_object> _list;
-			return &_list;
-		}
-
 		void load()   override { _ref_count++; }
 		bool unload() override { return (_keep == DONT_KEEP) && !(--_ref_count); }
 
@@ -331,7 +325,8 @@ Linker::Ld &Linker::Ld::linker()
 	{
 		Ld_vtable()
 		{
-			Elf_object::obj_list()->enqueue(*this);
+			with_object_list([&] (Object_list &list) {
+				list.enqueue(*this); });
 			plt_setup();
 		}
 	};
@@ -543,32 +538,28 @@ Object &Linker::load(Env &env, Allocator &md_alloc, char const *path,
                      Dependency &dep, Keep keep)
 {
 	Object *result = nullptr;
-	Elf_object::obj_list()->for_each([&] (Object &e) {
+	Object::with_object_list([&] (Object::Object_list &list) {
-		if (result == nullptr) {
+		list.for_each([&] (Object &obj) {
-			if (verbose_loading)
-				log("LOAD: ", Linker::file(path), " == ", e.name());
 
-			if (!strcmp(Linker::file(path), e.name())) {
+			if (result == nullptr) {
-				e.load();
+				if (verbose_loading)
-				result = &e;
+					log("LOAD: ", Linker::file(path), " == ", obj.name());
+
+				if (!strcmp(Linker::file(path), obj.name())) {
+					obj.load();
+					result = &obj;
+				}
 			}
-		}
+		});
 	});
+
 	if (result == nullptr)
 		result = new (md_alloc) Elf_object(env, md_alloc, path, dep, keep);
+
 	return *result;
 }
 
 
-Object *Linker::obj_list_head()
-{
-	Object *result = nullptr;
-	Elf_object::obj_list()->head([&result] (Object &obj) {
-		result = &obj; });
-	return result;
-}
-
-
 Elf::Sym const *Linker::lookup_symbol(unsigned sym_index, Dependency const &dep,
                                       Elf::Addr *base, bool undef, bool other)
 {
@@ -748,22 +739,24 @@ void Genode::exec_static_constructors()
 
 void Genode::Dynamic_linker::_for_each_loaded_object(Env &, For_each_fn const &fn)
 {
-	Elf_object::obj_list()->for_each([&] (Object const &obj) {
+	Object::with_object_list([&] (Object::Object_list &list) {
+		list.for_each([&] (Object const &obj) {
 
-		Elf_file const *elf_file_ptr =
+			Elf_file const *elf_file_ptr =
-			obj.file() ? dynamic_cast<Elf_file const *>(obj.file()) : nullptr;
+				obj.file() ? dynamic_cast<Elf_file const *>(obj.file()) : nullptr;
 
-		if (!elf_file_ptr)
+			if (!elf_file_ptr)
-			return;
+				return;
 
-		elf_file_ptr->with_rw_phdr([&] (Elf::Phdr const &phdr) {
+			elf_file_ptr->with_rw_phdr([&] (Elf::Phdr const &phdr) {
 
-			Object_info info { .name     = obj.name(),
+				Object_info info { .name     = obj.name(),
-			                   .ds_cap   = elf_file_ptr->rom_cap,
+				                   .ds_cap   = elf_file_ptr->rom_cap,
-			                   .rw_start = (void *)(obj.reloc_base() + phdr.p_vaddr),
+				                   .rw_start = (void *)(obj.reloc_base() + phdr.p_vaddr),
-			                   .rw_size  = phdr.p_memsz };
+				                   .rw_size  = phdr.p_memsz };
 
-			fn.supply_object_info(info);
+				fn.supply_object_info(info);
+			});
 		});
 	});
 }
@@ -771,9 +764,11 @@ void Genode::Dynamic_linker::_for_each_loaded_object(Env &, For_each_fn const &f
 
 void Dynamic_linker::keep(Env &, char const *binary)
 {
-	Elf_object::obj_list()->for_each([&] (Elf_object &obj) {
+	Object::with_object_list([&] (Object::Object_list &list) {
-		if (Object::Name(binary) == obj.name())
+		list.for_each([&] (Object &obj) {
-			obj.force_keep(); });
+			if (Object::Name(binary) == obj.name())
+				obj.force_keep(); });
+	});
 }
 
 
@@ -829,8 +824,10 @@ void Component::construct(Genode::Env &env)
 			    " .. ", Hex(Thread::stack_area_virtual_base() +
 			                Thread::stack_area_virtual_size() - 1),
 			    ": stack area");
-			Elf_object::obj_list()->for_each([] (Object const &obj) {
+			Object::with_object_list([] (Object::Object_list &list) {
-				dump_link_map(obj); });
+				list.for_each([] (Object const &obj) {
+					dump_link_map(obj); });
+			});
 		}
 	} catch (...) {  }
 

repos/base/src/lib/ldso/shared_object.cc

@@ -27,9 +27,16 @@ static Linker::Root_object const &to_root(void *h)
 
 static Linker::Object *find_obj(Genode::addr_t addr)
 {
-	for (Linker::Object *e = Linker::obj_list_head(); e; e = e->next_obj())
+	Linker::Object *elf = 0;
-		if (addr >= e->link_map().addr  && addr < e->link_map().addr + e->size())
+	Linker::for_each_object([&] (Linker::Object &e) {
-			return e;
+		if (elf) return;
+
+		if (addr >= e.link_map().addr  && addr < e.link_map().addr + e.size())
+			elf = &e;
+	});
+
+	if (elf)
+		return elf;
 
 	throw Genode::Address_info::Invalid_address();
 }