LibELF: Call mmap() before constructing the DynamicLoader object

Refactor DynamicLoader construction with a try_create() helper so that
we can call mmap() before making a loader. This way the loader doesn't
need to have an "mmap failed" state.

This patch also takes care of determining the ELF file size in
try_create() instead of expecting callers to provide it.

4 files changed, 48 insertions, 65 deletions

diff --git a/Userland/Libraries/LibC/dlfcn.cpp b/Userland/Libraries/LibC/dlfcn.cpp
index bcf6de70ae..2a52042804 100644
--- a/Userland/Libraries/LibC/dlfcn.cpp
+++ b/Userland/Libraries/LibC/dlfcn.cpp
@@ -24,14 +24,6 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include <assert.h>
-#include <dlfcn.h>
-#include <fcntl.h>
-#include <mman.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <sys/stat.h>
-
 #include <AK/HashMap.h>
 #include <AK/LexicalPath.h>
 #include <AK/RefPtr.h>
@@ -39,6 +31,11 @@
 #include <AK/String.h>
 #include <AK/StringBuilder.h>
 #include <LibELF/DynamicLoader.h>
+#include <assert.h>
+#include <dlfcn.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
 
 // NOTE: The string here should never include a trailing newline (according to POSIX)
 String g_dlerror_msg;
@@ -85,18 +82,8 @@ void* dlopen(const char* filename, int flags)
 
     ScopeGuard close_fd_guard([fd]() { close(fd); });
 
-    struct stat file_stats {
-    };
-
-    int ret = fstat(fd, &file_stats);
-    if (ret < 0) {
-        g_dlerror_msg = String::formatted("Unable to stat file {}", filename);
-        return nullptr;
-    }
-
-    auto loader = ELF::DynamicLoader::construct(filename, fd, file_stats.st_size);
-
-    if (!loader->is_valid()) {
+    auto loader = ELF::DynamicLoader::try_create(fd, filename);
+    if (!loader || !loader->is_valid()) {
         g_dlerror_msg = String::formatted("{} is not a valid ELF dynamic shared object!", filename);
         return nullptr;
     }
diff --git a/Userland/Libraries/LibELF/DynamicLinker.cpp b/Userland/Libraries/LibELF/DynamicLinker.cpp
index 81349606ab..8fa91231fb 100644
--- a/Userland/Libraries/LibELF/DynamicLinker.cpp
+++ b/Userland/Libraries/LibELF/DynamicLinker.cpp
@@ -35,16 +35,14 @@
 #include <LibC/stdio.h>
 #include <LibC/sys/internals.h>
 #include <LibC/unistd.h>
-#include <LibCore/File.h>
 #include <LibELF/AuxiliaryVector.h>
 #include <LibELF/DynamicLinker.h>
 #include <LibELF/DynamicLoader.h>
 #include <LibELF/DynamicObject.h>
 #include <LibELF/Image.h>
-#include <LibELF/exec_elf.h>
 #include <dlfcn.h>
+#include <fcntl.h>
 #include <string.h>
-#include <sys/stat.h>
 #include <sys/types.h>
 
 namespace ELF {
@@ -84,14 +82,14 @@ Optional<DynamicObject::SymbolLookupResult> DynamicLinker::lookup_global_symbol(
 
 static void map_library(const String& name, int fd)
 {
-    struct stat lib_stat;
-    int rc = fstat(fd, &lib_stat);
-    ASSERT(!rc);
-
-    auto loader = ELF::DynamicLoader::construct(name.characters(), fd, lib_stat.st_size);
+    auto loader = ELF::DynamicLoader::try_create(fd, name);
+    if (!loader) {
+        dbgln("Failed to create ELF::DynamicLoader for fd={}, name={}", fd, name);
+        ASSERT_NOT_REACHED();
+    }
     loader->set_tls_offset(g_current_tls_offset);
 
-    g_loaders.set(name, loader);
+    g_loaders.set(name, *loader);
 
     g_current_tls_offset += loader->tls_size();
 }
diff --git a/Userland/Libraries/LibELF/DynamicLoader.cpp b/Userland/Libraries/LibELF/DynamicLoader.cpp
index b7c98085c8..5429c99fad 100644
--- a/Userland/Libraries/LibELF/DynamicLoader.cpp
+++ b/Userland/Libraries/LibELF/DynamicLoader.cpp
@@ -30,13 +30,13 @@
 #include <AK/StringBuilder.h>
 #include <LibELF/DynamicLoader.h>
 #include <LibELF/Validation.h>
-
 #include <assert.h>
 #include <dlfcn.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/mman.h>
+#include <sys/stat.h>
 
 #ifndef __serenity__
 static void* mmap_with_name(void* addr, size_t length, int prot, int flags, int fd, off_t offset, const char*)
@@ -51,38 +51,45 @@ namespace ELF {
 
 static bool s_always_bind_now = false;
 
-NonnullRefPtr<DynamicLoader> DynamicLoader::construct(const char* filename, int fd, size_t size)
+RefPtr<DynamicLoader> DynamicLoader::try_create(int fd, String filename)
 {
-    return adopt(*new DynamicLoader(filename, fd, size));
-}
+    struct stat stat;
+    if (fstat(fd, &stat) < 0) {
+        perror("DynamicLoader::try_create fstat");
+        return {};
+    }
 
-void* DynamicLoader::do_mmap(int fd, size_t size, const String& name)
-{
+    ASSERT(stat.st_size >= 0);
+    size_t size = static_cast<size_t>(stat.st_size);
     if (size < sizeof(Elf32_Ehdr))
-        return MAP_FAILED;
+        return {};
 
-    String file_mmap_name = String::formatted("ELF_DYN: {}", name);
-    return mmap_with_name(nullptr, size, PROT_READ, MAP_PRIVATE, fd, 0, file_mmap_name.characters());
+    String file_mmap_name = String::formatted("ELF_DYN: {}", filename);
+    auto* data = mmap_with_name(nullptr, size, PROT_READ, MAP_PRIVATE, fd, 0, file_mmap_name.characters());
+    if (data == MAP_FAILED) {
+        perror("DynamicLoader::try_create mmap");
+        return {};
+    }
+
+    return adopt(*new DynamicLoader(fd, move(filename), data, size));
 }
 
-DynamicLoader::DynamicLoader(const char* filename, int fd, size_t size)
-    : m_filename(filename)
+DynamicLoader::DynamicLoader(int fd, String filename, void* data, size_t size)
+    : m_filename(move(filename))
     , m_file_size(size)
     , m_image_fd(fd)
-    , m_file_mapping(do_mmap(m_image_fd, m_file_size, m_filename))
-    , m_elf_image((u8*)m_file_mapping, m_file_size)
+    , m_file_data(data)
+    , m_elf_image((u8*)m_file_data, m_file_size)
 {
-
-    if (m_file_mapping == MAP_FAILED) {
-        m_valid = false;
-        return;
-    }
-
     m_tls_size = calculate_tls_size();
-
     m_valid = validate();
 }
 
+DynamicLoader::~DynamicLoader()
+{
+    munmap(m_file_data, m_file_size);
+}
+
 const DynamicObject& DynamicLoader::dynamic_object() const
 {
     if (!m_cached_dynamic_object) {
@@ -115,15 +122,8 @@ size_t DynamicLoader::calculate_tls_size() const
 
 bool DynamicLoader::validate()
 {
-    auto* elf_header = (Elf32_Ehdr*)m_file_mapping;
-    return validate_elf_header(*elf_header, m_file_size) && validate_program_headers(*elf_header, m_file_size, (u8*)m_file_mapping, m_file_size, &m_program_interpreter);
-}
-
-DynamicLoader::~DynamicLoader()
-{
-    if (MAP_FAILED != m_file_mapping)
-        munmap(m_file_mapping, m_file_size);
-    close(m_image_fd);
+    auto* elf_header = (Elf32_Ehdr*)m_file_data;
+    return validate_elf_header(*elf_header, m_file_size) && validate_program_headers(*elf_header, m_file_size, (u8*)m_file_data, m_file_size, &m_program_interpreter);
 }
 
 void* DynamicLoader::symbol_for_name(const char* name)
@@ -338,7 +338,7 @@ void DynamicLoader::load_program_headers()
     else
         data_segment_start = data_region.value().desired_load_address();
 
-    memcpy(data_segment_start.as_ptr(), (u8*)m_file_mapping + data_region.value().offset(), data_region.value().size_in_image());
+    memcpy(data_segment_start.as_ptr(), (u8*)m_file_data + data_region.value().offset(), data_region.value().size_in_image());
 
     // FIXME: Initialize the values in the TLS section. Currently, it is zeroed.
 }
diff --git a/Userland/Libraries/LibELF/DynamicLoader.h b/Userland/Libraries/LibELF/DynamicLoader.h
index 69f1144296..896a7e2158 100644
--- a/Userland/Libraries/LibELF/DynamicLoader.h
+++ b/Userland/Libraries/LibELF/DynamicLoader.h
@@ -42,8 +42,7 @@ namespace ELF {
 
 class DynamicLoader : public RefCounted<DynamicLoader> {
 public:
-    static NonnullRefPtr<DynamicLoader> construct(const char* filename, int fd, size_t file_size);
-
+    static RefPtr<DynamicLoader> try_create(int fd, String filename);
     ~DynamicLoader();
 
     bool is_valid() const { return m_valid; }
@@ -79,6 +78,8 @@ public:
     bool is_dynamic() const { return m_elf_image.is_dynamic(); }
 
 private:
+    DynamicLoader(int fd, String filename, void* file_data, size_t file_size);
+
     class ProgramHeaderRegion {
     public:
         void set_program_header(const Elf32_Phdr& header) { m_program_header = header; }
@@ -105,11 +106,8 @@ private:
         Elf32_Phdr m_program_header; // Explicitly a copy of the PHDR in the image
     };
 
-    static void* do_mmap(int fd, size_t size, const String& name);
     const DynamicObject& dynamic_object() const;
 
-    explicit DynamicLoader(const char* filename, int fd, size_t file_size);
-
     // Stage 1
     void load_program_headers();
 
@@ -137,7 +135,7 @@ private:
     String m_program_interpreter;
     size_t m_file_size { 0 };
     int m_image_fd { -1 };
-    void* m_file_mapping { MAP_FAILED };
+    void* m_file_data { nullptr };
     ELF::Image m_elf_image;
     bool m_valid { true };