Handle errors in shared object loading

options/rtld/generic/linker.cpp

@@ -9,6 +9,7 @@ enum {
 
 
 #include <frg/manual_box.hpp>
+#include <frg/scope_exit.hpp>
 #include <frg/small_vector.hpp>
 #include <mlibc/allocator.hpp>
 #include <mlibc/debug.hpp>
@@ -159,8 +160,10 @@ bool tryReadExactly(int fd, void *data, size_t length) {
 		if(mlibc::sys_read(fd, reinterpret_cast<char *>(data) + offset,
 				length - offset, &chunk))
 			return false;
-		__ensure(chunk > 0);
-		offset += chunk;
+		if (chunk > 0)
+			offset += chunk;
+		else
+			return false;
 	}
 	__ensure(offset == length);
 	return true;
@@ -254,17 +257,18 @@ frg::expected<LinkerError, SharedObject *> ObjectRepository::requestObjectWithNa
 	if (auto obj = findLoadedObject(name))
 		return obj;
 
-	auto tryToOpen = [&] (const char *path) {
+	auto tryToOpen = [&] (const char *path) -> frg::optional<int> {
 		int fd;
 		if(auto x = mlibc::sys_open(path, O_RDONLY, 0, &fd); x) {
-			return -1;
+			return frg::null_opt;
 		}
 		return fd;
 	};
 
 	// TODO(arsen): this process can probably undergo heavy optimization, by
 	// preprocessing the rpath only once on parse
-	auto processRpath = [&] (frg::string_view path) {
+	auto processRpath = [&] (frg::string_view path)
+	-> frg::expected<LinkerError, frg::tuple<frg::string<MemoryAllocator>, int>> {
 		frg::string<MemoryAllocator> sPath { getAllocator() };
 		if (path.starts_with("$ORIGIN")) {
 			frg::string_view dirname = origin->path;
@@ -283,79 +287,101 @@ frg::expected<LinkerError, SharedObject *> ObjectRepository::requestObjectWithNa
 			sPath += '/';
 		}
 		sPath += name;
+
 		if (logRpath)
 			mlibc::infoLogger() << "rtld: trying in rpath " << sPath << frg::endlog;
-		int fd = tryToOpen(sPath.data());
-		if (logRpath && fd >= 0)
+
+		auto fd = tryToOpen(sPath.data());
+		if (!fd)
+			return LinkerError::notFound;
+		if (logRpath)
 			mlibc::infoLogger() << "rtld: found in rpath" << frg::endlog;
-		return frg::tuple { fd, std::move(sPath) };
+		return frg::tuple { std::move(sPath), fd.value() };
+	};
+
+	if (createScope) {
+		__ensure(localScope == nullptr);
+
+		// TODO: Free this when the scope is no longer needed.
+		localScope = frg::construct<Scope>(getAllocator());
+	}
+
+	// Avoid leaking the localScope on error returns
+	frg::scope_exit localScopeGuard{[&]{
+		if (createScope)
+			frg::destruct(getAllocator(), localScope);
+	}};
+
+	__ensure(localScope != nullptr);
+
+	frg::expected<LinkerError, SharedObject *> res = LinkerError::notFound;
+
+	auto trySharedObjectSetup = [&] (frg::string<MemoryAllocator> path, int fd) -> frg::expected<LinkerError, SharedObject *> {
+		auto object = frg::construct<SharedObject>(getAllocator(),
+			name.data(), std::move(path), false, localScope, rts);
+
+		auto result = _fetchFromFile(object, fd);
+		closeOrDie(fd);
+		if(!result) {
+			if (verbose || stillSlightlyVerbose)
+				mlibc::infoLogger() << "rtld: failed to open " << name << frg::endlog;
+			frg::destruct(getAllocator(), object);
+			return result.error();
+		}
+		return object;
 	};
 
-	frg::string<MemoryAllocator> chosenPath { getAllocator() };
-	int fd = -1;
 	if (origin && origin->runPath) {
 		size_t start = 0;
 		size_t idx = 0;
 		frg::string_view rpath { origin->runPath };
+
 		auto next = [&] () {
 			idx = rpath.find_first(':', start);
 			if (idx == (size_t)-1)
 				idx = rpath.size();
 		};
+
 		for (next(); idx < rpath.size(); next()) {
 			auto path = rpath.sub_string(start, idx - start);
 			start = idx + 1;
-			auto [fd_, fullPath] = processRpath(path);
-			if (fd_ != -1) {
-				fd = fd_;
-				chosenPath = std::move(fullPath);
-				break;
+			auto rpathResult = processRpath(path);
+			if (rpathResult) {
+				res = trySharedObjectSetup(rpathResult.value().get<0>(), rpathResult.value().get<1>());
+				if (res)
+					break;
 			}
 		}
-		if (fd == -1) {
+		if (!res) {
 			auto path = rpath.sub_string(start, rpath.size() - start);
-			auto [fd_, fullPath] = processRpath(path);
-			if (fd_ != -1) {
-				fd = fd_;
-				chosenPath = std::move(fullPath);
-			}
+			auto rpathResult = processRpath(path);
+			if (rpathResult)
+				res = trySharedObjectSetup(rpathResult.value().get<0>(), rpathResult.value().get<1>());
 		}
 	} else if (logRpath) {
 		mlibc::infoLogger() << "rtld: no rpath set for object" << frg::endlog;
 	}
 
-	for(size_t i = 0; i < libraryPaths->size() && fd == -1; i++) {
+	for(size_t i = 0; i < libraryPaths->size() && !res; i++) {
 		auto ldPath = (*libraryPaths)[i];
 		auto path = frg::string<MemoryAllocator>{getAllocator(), ldPath} + '/' + name;
 		if(logLdPath)
 			mlibc::infoLogger() << "rtld: Trying to load " << name << " from ldpath " << ldPath << "/" << frg::endlog;
-		fd = tryToOpen(path.data());
-		if(fd >= 0) {
-			chosenPath = std::move(path);
-			break;
-		}
-	}
-	if(fd == -1)
-		return LinkerError::notFound;
 
-	if (createScope) {
-		__ensure(localScope == nullptr);
-
-		// TODO: Free this when the scope is no longer needed.
-		localScope = frg::construct<Scope>(getAllocator());
+		auto fd = tryToOpen(path.data());
+		if(fd) {
+			res = trySharedObjectSetup(path, fd.value());
+			if (res)
+				break;
+		}
 	}
 
-	__ensure(localScope != nullptr);
-
-	auto object = frg::construct<SharedObject>(getAllocator(),
-		name.data(), std::move(chosenPath), false, localScope, rts);
+	if(!res)
+		return res;
 
-	auto result = _fetchFromFile(object, fd);
-	closeOrDie(fd);
-	if(!result) {
-		frg::destruct(getAllocator(), object);
-		return result.error();
-	}
+	// the localScope is used by the SharedObject
+	localScopeGuard.release();
+	auto object = res.value();
 
 	_parseDynamic(object);
 	_parseVerdef(object);

tests/rtld/fail-load/fake/empty.c

@@ -0,0 +1 @@
+const char *empty = "";

tests/rtld/fail-load/fake/meson.build

@@ -0,0 +1,8 @@
+lib_fake = shared_library('fake', 'empty.c', install_dir: 'fake')
+lib_native_fake = shared_library('native-fake', 'empty.c', native: true, install_dir: 'fake')
+
+test_link_with += [lib_fake]
+test_native_link_with += [lib_native_fake]
+
+configure_file(input: 'fake-lib', output: 'libfoo.so', install_dir: 'fake', copy: true)
+configure_file(input: 'fake-lib', output: 'libnative-foo.so', install_dir: 'fake', copy: true)

tests/rtld/fail-load/meson.build

@@ -0,0 +1,6 @@
+subdir('fake')
+subdir('real')
+
+test_link_with += [lib_fake, libfoo]
+test_native_link_with += [lib_native_fake, libfoo_native]
+

tests/rtld/fail-load/real/libfoo.c

@@ -0,0 +1,3 @@
+int foo(void) {
+	return 69;
+}

tests/rtld/fail-load/real/meson.build

@@ -0,0 +1,5 @@
+libfoo = shared_library('foo', 'libfoo.c', install_dir: 'real')
+libfoo_native = shared_library('native-foo', 'libfoo.c', native: true, install_dir: 'real')
+
+test_link_with += [libfoo]
+test_native_link_with += [libfoo_native]

tests/rtld/fail-load/test.c

@@ -0,0 +1,9 @@
+#include <assert.h>
+
+int foo(void);
+
+int main() {
+	assert(foo() == 69);
+
+	return 0;
+}

tests/rtld/meson.build

@@ -17,6 +17,7 @@ rtld_test_cases = [
 	'tls_align',
 	'relr',
 	'symver',
+	'fail-load',
 ]
 
 if host_machine.cpu_family() != 'm68k'