Replace pid with actual bitpattern

Author: NicolasFussberger

score/launch_manager/src/daemon/src/osal/ipc_comms.hpp

@@ -15,6 +15,7 @@
 #define OSAL_IPC_COMMS_HPP_INCLUDED
 
 #include <sys/mman.h>
+#include <cstdint>
 #include <memory>
 #include <utility>
 
@@ -29,21 +30,30 @@ using IpcCommsP = std::shared_ptr<IpcCommsSync>;
 
 /// @brief Structure for managing inter-process communication synchronization.
 /// The `IpcCommsSync` structure is designed to handle synchronization mechanisms required
-/// for inter-process communication. It uses a semaphore to signal kRunning and
-/// a process ID to identify the communicating process.
+/// for inter-process communication. It uses a semaphore to signal kRunning, preceded by a magic number and a
+/// layout version that allow the client to verify that the shared memory really was set up by Launch Manager.
 /// It is only created for processes that are expected to report kRunning.
 struct IpcCommsSync final
 {
+    /// @brief Magic number used as a safeguard by application processes
+    static constexpr std::uint32_t kMagic{0x4C4D5352U};
+
+    /// @brief Expected value of `version_`.
+    static constexpr std::uint32_t kLayoutVersion{1U};
+
+    /// @brief Marks the shared memory as a comms object created by Launch Manager.
+    /// It is the first member, so that checking it does not depend on the layout of the remaining members.
+    /// Launch Manager writes it last, after all other members have been initialized.
+    std::uint32_t magic_;
+
+    /// @brief Layout version of this structure, to detect a Launch Manager and client built from different versions.
+    std::uint32_t version_;
+
     /// @brief Semaphore used by the process to report kRunning.
     /// The communicating process posts this semaphore exactly once to signal that it has finished initialization.
     /// Launch Manager waits on it (and also posts it to wake the waiter up early if the process terminates).
     Semaphore send_sync_;
 
-    /// @brief Process ID of the communicating process.
-    /// The `pid_` is written by the child process right after fork. The client checks it against its own pid.
-    /// The `pid_` is used as an expected bit pattern on the client side.
-    ProcessID pid_;
-
     /// @brief Constant for the synchronization file descriptor.
     /// The `sync_fd` is a constant representing the file descriptor used for synchronization
     /// during communication. It is set to a value of 111 by default.
@@ -109,6 +119,9 @@ struct IpcCommsSync final
             return nullptr;  // Releasing mapping unmaps the memory
         }
 
+        mapping->version_ = kLayoutVersion;
+        mapping->magic_ = kMagic;
+
         IpcCommsSync* const block = mapping.get();
         return IpcCommsP(block, IpcCommsOwnerDeletor{std::move(mapping)});
     }

score/launch_manager/src/daemon/src/process_group_manager/details/process_launcher.cpp

@@ -89,7 +89,6 @@ void handleComms(score::mw::lifecycle::internal::osal::ChildProcessConfig& param
     }
 
     param.fd = dup2(param.fd, param.shared_block->sync_fd);
-    param.shared_block->pid_ = getpid();  // Store pid for check at client end
 
     // Flag FD_CLOEXEC is cleared to ensure that sync_fd remains open after the execve call.
     if (-1 == fcntl(IpcCommsSync::sync_fd, F_SETFD, 0))

score/launch_manager/src/daemon/src/process_group_manager/details/process_launcher_UT.cpp

@@ -71,7 +71,6 @@ class ProcessLauncherTest : public ::testing::Test
         std::memset(test_buffer.data(), 0, test_buffer.size());
         auto* sync = reinterpret_cast<IpcCommsSync*>(test_buffer.data());
 
-        sync->pid_ = 0;
         EXPECT_EQ(sync->send_sync_.init(0, false), OsalReturnType::kSuccess);
         // Real msync calls would treat this fake shared memory as invalid because it isn't page aligned.
         ON_CALL(*g_syscall_mock, msync(sync, _, _)).WillByDefault(Return(0));
@@ -397,12 +396,18 @@ TEST_F(SetupCommsTest, getCommsFails)
 
 TEST_F(SetupCommsTest, initSemaphoresFails)
 {
-    RecordProperty("Description", "Verify that if setting up the semaphores fails, startProcess fails");
+    RecordProperty(
+        "Description",
+        "Verify that if setting up the semaphores fails, startProcess fails, the shared memory is not marked as a "
+        "valid comms object and no semaphore is destroyed");
 
-    StubShmObject();
+    IpcCommsSync* block = StubShmObject();
+    block->magic_ = 0U;
     EXPECT_CALL(*semaphore_mock_, sem_init).WillRepeatedly(Return(SemError(EINVAL)));
+    EXPECT_CALL(*g_syscall_mock, sem_destroy).Times(0);
 
     EXPECT_EQ(process_launcher->startProcess(pid_, sync_, config_), OsalReturnType::kFail);
+    EXPECT_EQ(block->magic_, 0U);
 }
 
 TEST_F(ProcessLauncherTest, releasingCommsObjectDestroysSemaphore)
@@ -419,6 +424,8 @@ TEST_F(ProcessLauncherTest, releasingCommsObjectDestroysSemaphore)
 
     IpcCommsP block = IpcCommsSync::createCommsObject(fd);
     ASSERT_NE(block, nullptr);
+    EXPECT_EQ(block->magic_, IpcCommsSync::kMagic);
+    EXPECT_EQ(block->version_, IpcCommsSync::kLayoutVersion);
 
     EXPECT_CALL(*g_syscall_mock, msync(data, _, _)).WillOnce(Return(0));
     Expectation destroyed = EXPECT_CALL(*g_syscall_mock, sem_destroy).WillOnce(Return(0));
@@ -797,7 +804,8 @@ TEST_F(StartProcessTest, startProcessChildSuccess)
 
     static_cast<void>(process_launcher->startProcess(pid_, sync_, config_));  // No return from forked process
 
-    EXPECT_EQ(block->pid_, forked_pid);
+    EXPECT_EQ(block->magic_, IpcCommsSync::kMagic);
+    EXPECT_EQ(block->version_, IpcCommsSync::kLayoutVersion);
 }
 
 class ClientMethodsTest : public ProcessLauncherTest

score/launch_manager/src/lifecycle_client/src/details/report_running_impl.cpp

@@ -78,9 +78,18 @@ score::Result<std::monostate> ReportRunningImpl::reportKRunningtoDaemon() const
     }
 
     // This is our best safeguard against incorrect data treated as an IPCCommsSync
-    if (sync->pid_ != getpid())
+    if (sync->magic_ != IpcCommsSync::kMagic)
     {
-        LM_LOG_ERROR() << "[Lifecycle client] Cannot report kRunning from a process not started by Launch Manager";
+        LM_LOG_ERROR() << "[Lifecycle client] FD " << sync_fd
+                       << " is not a Launch Manager comms object, cannot report kRunning";
+        return comms_error;
+    }
+
+    if (sync->version_ != IpcCommsSync::kLayoutVersion)
+    {
+        LM_LOG_ERROR() << "[Lifecycle client] Comms object version " << sync->version_
+                       << " does not match expected version " << IpcCommsSync::kLayoutVersion
+                       << ", Launch Manager and client are incompatible";
         return comms_error;
     }
 

score/launch_manager/src/lifecycle_client/src/details/report_running_impl_UT.cpp

@@ -38,11 +38,14 @@ using namespace score::mw::lifecycle::internal::osal;
 alignas(IpcCommsSync) static char g_fake_ipc_storage[sizeof(IpcCommsSync)];
 
 // Helper to create a fake IPC structure with specified properties
-void* CreateFakeIpcComms(pid_t pid)
+void* CreateFakeIpcComms(
+    std::uint32_t magic = IpcCommsSync::kMagic,
+    std::uint32_t version = IpcCommsSync::kLayoutVersion)
 {
     std::memset(g_fake_ipc_storage, 0, sizeof(IpcCommsSync));
     IpcCommsSync* fake_comms = reinterpret_cast<IpcCommsSync*>(g_fake_ipc_storage);
-    fake_comms->pid_ = pid;
+    fake_comms->magic_ = magic;
+    fake_comms->version_ = version;
 
     return g_fake_ipc_storage;
 }
@@ -90,15 +93,12 @@ class ReportRunningImplTest : public ::testing::Test
         // By default, close succeeds
         EXPECT_CALL(*g_syscall_mock, close(_)).WillRepeatedly(Return(0));
 
-        // By default, getpid returns current PID
-        EXPECT_CALL(*g_syscall_mock, getpid()).WillRepeatedly(Return(::getpid()));
-
         // By default, mmap succeeds and returns valid IPC structure
         EXPECT_CALL(*g_syscall_mock, mmap(_, _, _, _, _, _))
             .WillRepeatedly(Invoke([](void*, size_t length, int, int, int, off_t) -> void* {
                 if (length == sizeof(IpcCommsSync))
                 {
-                    return CreateFakeIpcComms(::getpid());
+                    return CreateFakeIpcComms();
                 }
                 return MAP_FAILED;
             }));
@@ -296,15 +296,47 @@ TEST_F(ReportRunningImplTest, GivenReportRunningImplInstance_ExceptGetCommsObjec
     EXPECT_EQ(result.error(), ExecErrc::kCommunicationError);
 }
 
-TEST_F(ReportRunningImplTest, GivenWrongProcessId_WhenPidDoesNotMatch_ThenReturnsCommunicationError)
+TEST_F(ReportRunningImplTest, GivenWrongMagic_WhenReportRunningStateCalled_ThenReturnsCommunicationError)
 {
     RecordProperty("DerivationTechnique", "explorative-testing");
     RecordProperty(
         "Description",
-        "Given a ReportRunningImpl instace. "
-        "Expect a pid mismatch. "
+        "Given a ReportRunningImpl instance. "
+        "Expect the shared memory not to contain the Launch Manager magic number. "
         "When ReportRunningState() is called. "
-        "Then ReportRunningState() returns a kCommunicationError error. ");
+        "Then ReportRunningState() returns a kCommunicationError error and does not post the semaphore. ");
+
+    // Given
+    ReportRunningImpl impl;
+
+    // Expect
+    EXPECT_CALL(*g_syscall_mock, mmap(_, _, _, _, _, _))
+        .WillOnce([](void*, size_t length, int, int, int, off_t) -> void* {
+            if (length == sizeof(IpcCommsSync))
+            {
+                return CreateFakeIpcComms(0U, IpcCommsSync::kLayoutVersion);
+            }
+            return MAP_FAILED;
+        });
+    EXPECT_CALL(*semaphore_mock_, sem_post(_)).Times(0);
+
+    // When
+    auto result = impl.ReportRunningState();
+
+    // Then
+    EXPECT_FALSE(result.has_value());
+    EXPECT_EQ(result.error(), ExecErrc::kCommunicationError);
+}
+
+TEST_F(ReportRunningImplTest, GivenWrongLayoutVersion_WhenReportRunningStateCalled_ThenReturnsCommunicationError)
+{
+    RecordProperty("DerivationTechnique", "explorative-testing");
+    RecordProperty(
+        "Description",
+        "Given a ReportRunningImpl instance. "
+        "Expect the shared memory to contain a different layout version. "
+        "When ReportRunningState() is called. "
+        "Then ReportRunningState() returns a kCommunicationError error and does not post the semaphore. ");
 
     // Given
     ReportRunningImpl impl;
@@ -314,10 +346,11 @@ TEST_F(ReportRunningImplTest, GivenWrongProcessId_WhenPidDoesNotMatch_ThenReturn
         .WillOnce([](void*, size_t length, int, int, int, off_t) -> void* {
             if (length == sizeof(IpcCommsSync))
             {
-                return CreateFakeIpcComms(99999);
+                return CreateFakeIpcComms(IpcCommsSync::kMagic, IpcCommsSync::kLayoutVersion + 1U);
             }
             return MAP_FAILED;
         });
+    EXPECT_CALL(*semaphore_mock_, sem_post(_)).Times(0);
 
     // When
     auto result = impl.ReportRunningState();