From d16781d16a441e98972eca679224b4d78da2a3d8 Mon Sep 17 00:00:00 2001
From: lukemartinlogan <lukemartinlogan@gmail.com>
Date: Sat, 9 Dec 2023 18:47:57 -0600
Subject: [PATCH] Add more checks to backends

---
 .../hermes_shm/memory/backend/memory_backend.h   |  2 +-
 .../memory/backend/memory_backend_factory.h      | 16 ++++++++++++----
 .../hermes_shm/memory/backend/posix_shm_mmap.h   |  3 +++
 include/hermes_shm/memory/memory_registry.h      |  7 ++++++-
 include/hermes_shm/util/errors.h                 |  1 +
 5 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/include/hermes_shm/memory/backend/memory_backend.h b/include/hermes_shm/memory/backend/memory_backend.h
index 01039fbd..829fe182 100644
--- a/include/hermes_shm/memory/backend/memory_backend.h
+++ b/include/hermes_shm/memory/backend/memory_backend.h
@@ -44,7 +44,7 @@ class MemoryBackend {
   bitfield32_t flags_;
 
  public:
-  MemoryBackend() = default;
+  MemoryBackend() : header_(nullptr), data_(nullptr) {}
 
   virtual ~MemoryBackend() = default;
 
diff --git a/include/hermes_shm/memory/backend/memory_backend_factory.h b/include/hermes_shm/memory/backend/memory_backend_factory.h
index cb0f35ff..eefc5042 100644
--- a/include/hermes_shm/memory/backend/memory_backend_factory.h
+++ b/include/hermes_shm/memory/backend/memory_backend_factory.h
@@ -31,22 +31,30 @@ class MemoryBackendFactory {
     if constexpr(std::is_same_v<PosixShmMmap, BackendT>) {
       // PosixShmMmap
       auto backend = std::make_unique<PosixShmMmap>();
-      backend->shm_init(size, url, std::forward<args>(args)...);
+      if (!backend->shm_init(size, url, std::forward<args>(args)...)) {
+        throw MEMORY_BACKEND_CREATE_FAILED.format();
+      }
       return backend;
     } else if constexpr(std::is_same_v<PosixMmap, BackendT>) {
       // PosixMmap
       auto backend = std::make_unique<PosixMmap>();
-      backend->shm_init(size, std::forward<args>(args)...);
+      if (!backend->shm_init(size, std::forward<args>(args)...)) {
+        throw MEMORY_BACKEND_CREATE_FAILED.format();
+      }
       return backend;
     } else if constexpr(std::is_same_v<NullBackend, BackendT>) {
       // NullBackend
       auto backend = std::make_unique<NullBackend>();
-      backend->shm_init(size, url, std::forward<args>(args)...);
+      if (!backend->shm_init(size, url, std::forward<args>(args)...)) {
+        throw MEMORY_BACKEND_CREATE_FAILED.format();
+      }
       return backend;
     } else if constexpr(std::is_same_v<ArrayBackend, BackendT>) {
       // ArrayBackend
       auto backend = std::make_unique<ArrayBackend>();
-      backend->shm_init(size, url, std::forward<args>(args)...);
+      if (!backend->shm_init(size, url, std::forward<args>(args)...)) {
+        throw MEMORY_BACKEND_CREATE_FAILED.format();
+      }
       return backend;
     } else {
       throw MEMORY_BACKEND_NOT_FOUND.format();
diff --git a/include/hermes_shm/memory/backend/posix_shm_mmap.h b/include/hermes_shm/memory/backend/posix_shm_mmap.h
index acdebb61..c7f642ba 100644
--- a/include/hermes_shm/memory/backend/posix_shm_mmap.h
+++ b/include/hermes_shm/memory/backend/posix_shm_mmap.h
@@ -14,6 +14,7 @@
 #define HERMES_INCLUDE_MEMORY_BACKEND_POSIX_SHM_MMAP_H
 
 #include "memory_backend.h"
+#include "hermes_shm/util/logging.h"
 #include <string>
 
 #include <stdio.h>
@@ -57,6 +58,7 @@ class PosixShmMmap : public MemoryBackend {
     shm_unlink(url_.c_str());
     fd_ = shm_open(url_.c_str(), O_CREAT | O_RDWR, 0666);
     if (fd_ < 0) {
+      HILOG(kError, "shm_open failed: {}", strerror(errno));
       return false;
     }
     _Reserve(size + HERMES_SYSTEM_INFO->page_size_);
@@ -74,6 +76,7 @@ class PosixShmMmap : public MemoryBackend {
     url_ = std::move(url);
     fd_ = shm_open(url_.c_str(), O_RDWR, 0666);
     if (fd_ < 0) {
+      HILOG(kError, "shm_open failed: {}", strerror(errno));
       return false;
     }
     header_ = _Map<MemoryBackendHeader>(HERMES_SYSTEM_INFO->page_size_, 0);
diff --git a/include/hermes_shm/memory/memory_registry.h b/include/hermes_shm/memory/memory_registry.h
index 9ff144cb..a30e9cac 100644
--- a/include/hermes_shm/memory/memory_registry.h
+++ b/include/hermes_shm/memory/memory_registry.h
@@ -18,6 +18,7 @@
 #include "hermes_shm/memory/allocator/stack_allocator.h"
 #include "hermes_shm/memory/backend/posix_mmap.h"
 #include "hermes_shm/util/errors.h"
+#include "hermes_shm/util/logging.h"
 
 namespace hipc = hshm::ipc;
 
@@ -95,7 +96,11 @@ class MemoryRegistry {
 
   /** Registers an allocator. */
   HSHM_ALWAYS_INLINE void RegisterAllocator(Allocator *alloc) {
-    auto idx = alloc->GetId().ToIndex();
+    uint32_t idx = alloc->GetId().ToIndex();
+    if (idx > MAX_ALLOCATORS) {
+      HILOG(kError, "Allocator index out of range: {}", idx)
+      throw std::runtime_error("Too many allocators");
+    }
     allocators_[idx] = alloc;
   }
 
diff --git a/include/hermes_shm/util/errors.h b/include/hermes_shm/util/errors.h
index 3dedb5cf..d7a56373 100644
--- a/include/hermes_shm/util/errors.h
+++ b/include/hermes_shm/util/errors.h
@@ -48,6 +48,7 @@ namespace hshm {
   const Error SHMEM_CREATE_FAILED("Failed to allocate SHMEM");
   const Error SHMEM_RESERVE_FAILED("Failed to reserve SHMEM");
   const Error SHMEM_NOT_SUPPORTED("Attempting to deserialize a non-shm backend");
+  const Error MEMORY_BACKEND_CREATE_FAILED("Failed to load memory backend");
   const Error MEMORY_BACKEND_NOT_FOUND("Failed to find the memory backend");
   const Error NOT_ENOUGH_CONCURRENT_SPACE("{}: Failed to divide memory slot {} among {} devices");
   const Error ALIGNED_ALLOC_NOT_SUPPORTED("Allocator does not support aligned allocations");