Fix no comms reporting crash

src/control_client_lib/src/control_client_impl.cpp

@@ -18,6 +18,7 @@
 #include <thread>
 #include <map>
 #include <iostream>
+#include <sys/stat.h>
 
 #include "score/concurrency/future/interruptible_future.h"
 #include "score/concurrency/future/interruptible_promise.h"
@@ -73,12 +74,22 @@ ControlClientImpl::ControlClientImpl(std::function<void(const score::lcm::Execut
 
         std::unique_lock<std::mutex> lock(instance_creation_mutex_);
         if (instance_created_) {
-            std::cerr << "[Control Client] Only one instance of ControlClient is allowed per process." << std::endl;
+            LM_LOG_ERROR() << "[Control Client] Only one instance of ControlClient is allowed per process.";
             std::abort();
         } else {
             instance_created_ = true;
         }
 
+    struct stat stats;
+    const auto fstat_ret = fstat(internal::osal::IpcCommsSync::sync_fd, &stats);
+    // Check size we have access of to avoid a crash if fd is not pointing to correct data
+    const auto needed_size = sizeof(internal::osal::IpcCommsSync) + sizeof(internal::ControlClientChannel);
+    if (fstat_ret == -1 || stats.st_size != static_cast<off_t>(needed_size)) {
+        LM_LOG_ERROR() << "Control client channel at sync_fd is not valid!";
+        instance_created_ = false;
+        std::abort();
+    }
+
     // initialization of control_client_requests_ is a bit more complicated...
     // std::atomic_bool is not copyable so we can't use fill method,
     // we will need to do this by hand

src/launch_manager_daemon/common/BUILD

@@ -80,7 +80,10 @@ cc_library(
         allow_empty = False,
     ),
     includes = ["include"],
-    visibility = ["//src:__subpackages__"],
+    visibility = [
+        "//src:__subpackages__",
+        "//tests:__subpackages__",
+    ],
     deps = [
         ":common",
         ":log",

src/launch_manager_daemon/lifecycle_client_lib/src/lifecycleclientimpl.cpp

@@ -12,6 +12,7 @@
  ********************************************************************************/
 
 #include <sys/mman.h>
+#include <sys/stat.h>
 #include <unistd.h>
 #include <atomic>
 #include <variant>
@@ -62,90 +63,63 @@ namespace score
 
         score::Result<std::monostate> LifecycleClient::LifecycleClientImpl::reportKRunningtoDaemon() const noexcept
         {
-            score::Result<std::monostate> retVal{score::MakeUnexpected(score::lcm::ExecErrc::kCommunicationError)};
+            score::Result<std::monostate> comms_error {score::MakeUnexpected(score::lcm::ExecErrc::kCommunicationError)};
 
             // Define necessary constants
             const int sync_fd = IpcCommsSync::sync_fd;
 
-            // coverity[autosar_cpp14_a18_5_8_violation:FALSE] sync is a shared memory object and so has to be allocated.
-            IpcCommsP sync = IpcCommsSync::getCommsObject(sync_fd);
-
+            struct stat stats;  // Check accessible size to avoid a crash when we do other checks
+            const bool correct_fd = fstat(sync_fd, &stats) != -1 && stats.st_size >= static_cast<off_t>(sizeof(IpcCommsSync));
 
-            // The Lambda function is used to check if the file descriptor is closed successfully
-            auto checkClose = [&]()
-            {
-                if ((sync->comms_type_ == CommsType::kReporting) && close(sync_fd) < 0)
-                {
-                    LM_LOG_ERROR() << "[Lifecycle Client] Closing file descriptor failed.";
+            if (!correct_fd) {
+                LM_LOG_ERROR() << "[Lifecycle client] FD " << sync_fd << " is invalid for kRunning report";
+                return comms_error;
+            }
 
-                    return false;
-                }
+            // coverity[autosar_cpp14_a18_5_8_violation:FALSE] sync is a shared memory object and so has to be allocated.
+            const IpcCommsP sync = IpcCommsSync::getCommsObject(sync_fd);
 
-                return true;
-            };
+            if (!sync) {
+                LM_LOG_ERROR() << "[Lifecycle Client] Failed to access communication channel with Launch Manager.";
 
-            // The Lambda function is used to check if the PID is matched
-            auto checkPid = [&]()
-            {
-                if (sync->pid_ != getpid())
-                {
-                    LM_LOG_ERROR() << "[Lifecycle Client] PID mismatch.";
+                return comms_error;
+            }
 
-                    return false;
-                }
+            const bool correct_type = sync->comms_type_ == CommsType::kReporting || sync->comms_type_ == CommsType::kControlClient;
 
-                return true;
-            };
+            // This is our best safeguard against incorrect data treated as an IPCCommsSync
+            if (!correct_type || sync->pid_ != getpid()) {
+                LM_LOG_ERROR() << "[Lifecycle client] Cannot report kRunning from a non-reporting process or a process not started by Launch Manager";
+                return comms_error;
+            }
 
-            // The Lambda function is used to report kRunning to LM.
-            // Reporting is implemented as posting on send_sync_ semaphore.
-            auto checkSendSync = [&]()
+            if ((sync->comms_type_ == CommsType::kReporting) && close(sync_fd) < 0)
             {
-                if (sync->send_sync_.post() == OsalReturnType::kFail)
-                {
-                    LM_LOG_ERROR() << "[Lifecycle Client] Sending kRunning to Launch Manager failed.";
-
-                    return false;
-                }
+                LM_LOG_ERROR() << "[Lifecycle Client] Closing file descriptor failed.";
 
-                return true;
-            };
+                return comms_error;
+            }
 
-            // The Lambda function is used to wait for a replay from LM.
-            // By posting on the reply_sync_ semaphore, LM confirms that it read and processed our kRunning report.
-            auto checkReplySync = [&]()
+            if (sync->send_sync_.post() == OsalReturnType::kFail)
             {
-                if (sync->reply_sync_.timedWait(score::lcm::internal::kMaxKRunningDelay) == OsalReturnType::kFail)
-                {
-                    LM_LOG_ERROR() << "[Lifecycle Client] Launch Manager failed to acknowledge kRunning report.";
-
-                    return false;
-                }
-
-                return true;
-            };
+                LM_LOG_ERROR() << "[Lifecycle Client] Sending kRunning to Launch Manager failed.";
 
-            /* RULECHECKER_comment(1, 1, check_c_style_cast, "This is the definition provided by the system header and does a C-style cast.", true) */
-            if (!sync)
-            {
-                LM_LOG_ERROR() << "[Lifecycle Client] Failed to access communication channel with Launch Manager.";
+                return comms_error;
             }
-            else
+
+            if (sync->reply_sync_.timedWait(score::lcm::internal::kMaxKRunningDelay) == OsalReturnType::kFail)
             {
-                // Check all the conditions
-                if (!(checkClose() && checkPid() && checkSendSync() && checkReplySync()))
-                {
-                    return retVal;
-                }
-                // Final post to semaphore, so LM know that communication channel can be closed now
-                sync->send_sync_.post();
-                // Mark as reported if successful
-                reported = true;
-                // Set return value to success
-                retVal = score::Result<std::monostate>{};
-            }
+                LM_LOG_ERROR() << "[Lifecycle Client] Launch Manager failed to acknowledge kRunning report.";
 
-            return retVal;
+                return comms_error;
+            }
+
+            // Final post to semaphore, so LM know that communication channel can be closed now
+            sync->send_sync_.post();
+            // Mark as reported if successful
+            reported = true;
+            // Set return value to success
+            return score::Result<std::monostate>{};
         }
 
     } // namespace lcm

src/launch_manager_daemon/src/process_group_manager/processlauncher.cpp

@@ -50,67 +50,50 @@ void handleComms(score::lcm::internal::osal::ChildProcessConfig& param)
     // kReporting  fd3 & !fd4
     // kControlClient  fd3 & fd4
     // kLaunchManager  does not matter
-    if (param.shared_block)
+    if (!param.shared_block)
     {
-        param.fd = dup2(param.fd, param.shared_block->sync_fd);  // always make sure we are using fd=3
-        param.shared_block->pid_ = getpid();                     // Store pid for check at client end
-
-        // It must be ensured that sync_fd (f3) and control_client_handler_nudge_fd (fd4) remain open depending on
-        // the communication type. Flag FD_CLOEXEC is cleared conditionally to ensure that the
-        // respective file descriptor remains open after the execve call.
-        switch (param.shared_block->comms_type_)
-        {
-            case CommsType::kNoComms:
-                // in the current implementation this case means param.shared_block == nullptr and is handled in below
-                // else
-                break;
-            case CommsType::kReporting:
-                if (-1 == fcntl(IpcCommsSync::sync_fd, F_SETFD, 0))
-                {
-                    LM_LOG_ERROR() << "[New process] fcntl() at line" << __LINE__ << "failed:" << std::strerror(errno);
-                    sysexit(EXIT_FAILURE);
-                }
-                close(IpcCommsSync::control_client_handler_nudge_fd);
-                break;
-            case CommsType::kControlClient:
-                if (-1 == fcntl(IpcCommsSync::sync_fd, F_SETFD, 0))
-                {
-                    LM_LOG_ERROR() << "[New process] fcntl() at line" << __LINE__ << "failed:" << std::strerror(errno);
-                    sysexit(EXIT_FAILURE);
-                }
-                if (-1 == fcntl(IpcCommsSync::control_client_handler_nudge_fd, F_SETFD, 0))
-                {
-                    LM_LOG_ERROR() << "[New process] fcntl() at line" << __LINE__ << "failed:" << std::strerror(errno);
-                }
-                break;
-            case CommsType::kLaunchManager:
-                // nothing to do here
-                break;
-            default:
-                LM_LOG_ERROR() << "[New process] at line" << __LINE__ << "unknown CommsType"
-                               << static_cast<std::int32_t>(param.shared_block->comms_type_);
-                sysexit(EXIT_FAILURE);
-                break;
-        }
+        // kNoComms, fds are CLOEXEC
+        return;
     }
-    else
-    {  // No communications channel was requested, but still make sure fd=3 is in use
-        close(IpcCommsSync::sync_fd);
-        close(IpcCommsSync::control_client_handler_nudge_fd);
-        const char* shmem_name = "/ipc_secondary_shared_mem";
-        param.fd = shm_open(shmem_name, O_CREAT, 0U);
 
-        if (-1 == param.fd)
-        {
-            LM_LOG_ERROR() << "[New process] shm_open() failed:" << std::strerror(errno);
-            sysexit(EXIT_FAILURE);
-        }
+    param.fd = dup2(param.fd, param.shared_block->sync_fd);  // always make sure we are using fd=3
+    param.shared_block->pid_ = getpid();                     // Store pid for check at client end
 
-        if (-1 == shm_unlink(shmem_name))
-        {
-            LM_LOG_ERROR() << "[New process] shm_unlink() failed:" << std::strerror(errno);
+    // It must be ensured that sync_fd (f3) and control_client_handler_nudge_fd (fd4) remain open depending on
+    // the communication type. Flag FD_CLOEXEC is cleared conditionally to ensure that the
+    // respective file descriptor remains open after the execve call.
+    switch (param.shared_block->comms_type_)
+    {
+        case CommsType::kNoComms:
+            // in the current implementation this case means param.shared_block == nullptr and is handled above
+            break;
+        case CommsType::kReporting:
+            if (-1 == fcntl(IpcCommsSync::sync_fd, F_SETFD, 0))
+            {
+                LM_LOG_ERROR() << "[New process] fcntl() at line" << __LINE__ << "failed:" << std::strerror(errno);
+                sysexit(EXIT_FAILURE);
+            }
+            close(IpcCommsSync::control_client_handler_nudge_fd);
+            break;
+        case CommsType::kControlClient:
+            if (-1 == fcntl(IpcCommsSync::sync_fd, F_SETFD, 0))
+            {
+                LM_LOG_ERROR() << "[New process] fcntl() at line" << __LINE__ << "failed:" << std::strerror(errno);
+                sysexit(EXIT_FAILURE);
+            }
+            if (-1 == fcntl(IpcCommsSync::control_client_handler_nudge_fd, F_SETFD, 0))
+            {
+                LM_LOG_ERROR() << "[New process] fcntl() at line" << __LINE__ << "failed:" << std::strerror(errno);
+            }
+            break;
+        case CommsType::kLaunchManager:
+            // nothing to do here
+            break;
+        default:
+            LM_LOG_ERROR() << "[New process] at line" << __LINE__ << "unknown CommsType"
+                            << static_cast<std::int32_t>(param.shared_block->comms_type_);
             sysexit(EXIT_FAILURE);
-        }
+            break;
     }
 }
 

tests/integration/incorrect_config_non_reporting/BUILD

@@ -0,0 +1,69 @@
+# *******************************************************************************
+# Copyright (c) 2026 Contributors to the Eclipse Foundation
+#
+# See the NOTICE file(s) distributed with this work for additional
+# information regarding copyright ownership.
+#
+# This program and the accompanying materials are made available under the
+# terms of the Apache License Version 2.0 which is available at
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# SPDX-License-Identifier: Apache-2.0
+# *******************************************************************************
+load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_files")
+load("@rules_pkg//pkg:tar.bzl", "pkg_tar")
+load("@score_itf//:defs.bzl", "py_itf_test")
+load("@score_lifecycle_pip//:requirements.bzl", "all_requirements")
+load("@score_tooling//:defs.bzl", "score_py_pytest")
+load("//:defs.bzl", "launch_manager_config")
+load("//config:flatbuffers_rules.bzl", "flatbuffer_json_to_bin")
+load("//tests/utils/bazel:integration.bzl", "integration_test")
+
+launch_manager_config(
+    name = "lm_non_reporting_config",
+    config = ":non_reporting_config.json",
+    flatbuffer_out_dir = "etc",
+)
+
+cc_binary(
+    name = "non_reporting_process",
+    srcs = ["non_reporting_process.cpp"],
+    deps = [
+        "//src/launch_manager_daemon/common:osal",
+        "//src/launch_manager_daemon/lifecycle_client_lib:lifecycle_client",
+        "//tests/utils/test_helper",
+        "@googletest//:gtest_main",
+    ],
+)
+
+pkg_files(
+    name = "non_reporting_main_files",
+    srcs = [
+        ":non_reporting_process",
+        "//src/launch_manager_daemon:launch_manager",
+    ],
+    attributes = pkg_attributes(mode = "0755"),
+    prefix = "tests/incorrect_config_non_reporting",
+)
+
+pkg_files(
+    name = "non_reporting_etc_files",
+    srcs = [":lm_non_reporting_config"],
+    prefix = "tests/incorrect_config_non_reporting",
+)
+
+pkg_tar(
+    name = "non_reporting_binaries",
+    srcs = [
+        ":non_reporting_etc_files",
+        ":non_reporting_main_files",
+    ],
+)
+
+integration_test(
+    name = "incorrect_config_non_reporting",
+    srcs = ["test_incorrect_config_non_reporting.py"],
+    tags = ["integration"],
+    test_binaries = ":non_reporting_binaries",
+    deps = ["//tests/utils/testing_utils"],
+)