RDKEMW-25196: Harden force-stop to SIGKILL the whole container cgroup

daemon/lib/source/DobbyManager.cpp

@@ -1310,22 +1310,68 @@ bool DobbyManager::restartContainer(const ContainerId &id,
     return true;
 }
 
+// -----------------------------------------------------------------------------
+/**
+ *  @brief Sends SIGKILL to every process in the container's cgroup and
+ *  confirms it actually died.
+ *
+ *  A crashed or hung process inside the container can leave orphaned
+ *  descendants that the container's init hasn't (or, if itself stuck, can't)
+ *  reaped, which would otherwise leave the container running forever from
+ *  runc/Dobby's point of view. Killing the whole cgroup (rather than just the
+ *  tracked init process) means the container is torn down even if init isn't
+ *  able to clean up after its children itself.
+ *
+ *  SIGKILL can't be masked or ignored, but a process stuck in an
+ *  uninterruptible sleep (D state) won't die until it exits that syscall, so
+ *  this waits for the container to actually leave the Running state instead
+ *  of just trusting that sending the signal was enough.
+ *
+ *  @param[in]  id  The id of the container to kill.
+ *
+ *  @return true if the container was confirmed stopped, false if it is still
+ *  running after all retries (almost always a process wedged in an
+ *  uninterruptible sleep, which no amount of signalling can fix).
+ */
+bool DobbyManager::forceKillContainerAndVerify(const ContainerId &id)
+{
+    mRunc->killCont(id, SIGKILL, /*all=*/true);
+
+    const int maxRetry = 10;
+    for (int attempt = 1; attempt <= maxRetry; attempt++)
+    {
+        /* coverity[sleep : FALSE] */
+        std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+        if (mRunc->state(id) != DobbyRunC::ContainerStatus::Running)
+            return true;
+    }
+
+    AI_LOG_ERROR("SIGKILL did not stop container '%s' - a process is likely "
+                 "stuck in an uninterruptible sleep", id.c_str());
+    return false;
+}
+
 // -----------------------------------------------------------------------------
 /**
  *  @brief Stops a running container
  *
  *  If withPrejudice is not specified (the default) then we send the init
  *  process within the container a SIGTERM.
  *
- *  If the withPrejudice is true then we use the SIGKILL signal.
+ *  If the withPrejudice is true then we SIGKILL every process in the
+ *  container's cgroup (not just the tracked init process) and block for up
+ *  to ~500ms confirming the container actually stopped, so a hung/crashed
+ *  process with orphaned descendants can't leave the container running
+ *  forever.
  *
- *  The kill signal itself is sent asynchronously (the actual container teardown
- *  happens in the background and @a mContainerStoppedCb is called when it
- *  completes). However, if the container is in the Hibernating state when this
- *  is called, the function will block for up to DobbyHibernate::DFL_TIMEOUTE_MS
- *  while aborting the in-progress hibernation via WakeupProcess() before
- *  sending the kill signal. This is necessary to ensure memcr_worker has
- *  unseized the in-flight PID before it is killed.
+ *  A plain (non-prejudice) SIGTERM is sent asynchronously (the actual
+ *  container teardown happens in the background and @a mContainerStoppedCb
+ *  is called when it completes). However, if the container is in the
+ *  Hibernating state when this is called, the function will block for up to
+ *  DobbyHibernate::DFL_TIMEOUTE_MS while aborting the in-progress hibernation
+ *  via WakeupProcess() before sending the kill signal. This is necessary to
+ *  ensure memcr_worker has unseized the in-flight PID before it is killed.
  *
  *  The @a mContainerStoppedCb callback will be called when the container
  *  has actually been torn down.
@@ -1334,8 +1380,9 @@ bool DobbyManager::restartContainer(const ContainerId &id,
  *  @param[in]  withPrejudice   If true the container process is killed with
  *                              SIGKILL, otherwise SIGTERM is used.
  *
- *  @return true if a container with a matching id was found and a signal
- *  sent successfully to it.
+ *  @return true if a container with a matching id was found and, for a
+ *  SIGTERM, the signal was sent successfully, or for a SIGKILL, the
+ *  container was confirmed stopped.
  */
 bool DobbyManager::stopContainer(int32_t cd, bool withPrejudice)
 {
@@ -1403,7 +1450,15 @@ bool DobbyManager::stopContainer(int32_t cd, bool withPrejudice)
             }
         }
 
-        if (!mRunc->killCont(id, withPrejudice ? SIGKILL : SIGTERM))
+        if (withPrejudice)
+        {
+            if (!forceKillContainerAndVerify(id))
+            {
+                AI_LOG_FN_EXIT();
+                return false;
+            }
+        }
+        else if (!mRunc->killCont(id, SIGTERM))
         {
             AI_LOG_WARN("failed to send signal to '%s'", id.c_str());
             AI_LOG_FN_EXIT();
@@ -1436,10 +1491,8 @@ bool DobbyManager::stopContainer(int32_t cd, bool withPrejudice)
             }
 
             // Container has been resumed, so kill it now
-            if (!mRunc->killCont(id, SIGKILL))
-
+            if (!forceKillContainerAndVerify(id))
             {
-                AI_LOG_WARN("failed to send signal to '%s'", id.c_str());
                 AI_LOG_FN_EXIT();
                 return false;
             }

daemon/lib/source/include/DobbyManager.h

@@ -184,6 +184,8 @@ class DobbyManager
 
     bool abortContainerHibernationIfNeeded(int32_t cd);
 
+    bool forceKillContainerAndVerify(const ContainerId& id);
+
 private:
     ContainerStartedFunc mContainerStartedCb;
     ContainerStoppedFunc mContainerStoppedCb;

openspec/specs/daemon-core.md

@@ -21,6 +21,7 @@ The daemon is the heart of Dobby, orchestrating container creation, start, stop,
 - Invokes legacy plugin hooks (PostConstruction, PreStart, PostStart, PostStop, PreDestruction) and RDK plugin hooks (postInstallation, preCreation, postHalt)
 - Supports `restartOnCrash` for automatic container restart
 - Loads plugins from configurable `PLUGIN_PATH` (default: `/usr/lib/plugins/dobby`)
+- `stopContainer(cd, withPrejudice=true)` sends SIGKILL to every process in the container's cgroup (`killCont(..., all=true)`), not just the tracked init process, and blocks for up to ~500ms confirming via `DobbyRunC::state()` that the container actually stopped. This guarantees termination even if a crashed/hung process left orphaned descendants that init can't reap; it only fails if a process is wedged in an uninterruptible (D-state) sleep.
 
 ### DobbyContainer
 - Stores container state: bundle, config, rootfs, rdkPluginManager
@@ -161,3 +162,4 @@ _No open queries._
 
 ## Change History
 - 2025-05-18 - openspec-templater - Restructured to match spec template.
+- 2026-09-16 - Hardened `stopContainer(withPrejudice=true)` to SIGKILL the whole container cgroup and verify the container actually stopped, so a hung/crashed process with orphaned descendants can't leave the container running forever.

tests/L1_testing/tests/DobbyManagerTest/DaemonDobbyManagerTest.cpp

@@ -3115,6 +3115,73 @@ TEST_F(DaemonDobbyManagerTest, stopContainer_FailedToSendSignal)
     expect_cleanupContainersShutdown();
 }
 
+/**
+ * @brief Test stopContainer with withPrejudice=true.
+ * Check that the force-kill path sends SIGKILL to the whole cgroup
+ * (all=true), not just the tracked init process, and that stopContainer
+ * returns true once runc reports the container has actually stopped.
+ *
+ * @return true.
+ */
+TEST_F(DaemonDobbyManagerTest, stopContainer_ForceKill_SendsSigkillToWholeCgroup)
+{
+    int32_t cd = 1234;
+    ContainerId id = ContainerId::create("container1");
+    expect_invalidContainerCleanupTask();
+
+    expect_startContainerFromBundle(cd,id);
+
+    bool killedWithAllFlag = false;
+    EXPECT_CALL(*p_runcMock, killCont(::testing::_, SIGKILL, ::testing::_))
+        .Times(1)
+        .WillOnce(::testing::Invoke(
+            [&killedWithAllFlag](const ContainerId &id, int signal, bool all) {
+                killedWithAllFlag = all;
+                return true;
+            }));
+
+    EXPECT_CALL(*p_runcMock, state(::testing::_))
+        .Times(1)
+        .WillOnce(::testing::Return(DobbyRunC::ContainerStatus::Stopped));
+
+    int return_value = dobbyManager_test->stopContainer(cd, true);
+    EXPECT_EQ(return_value, true);
+    EXPECT_TRUE(killedWithAllFlag);
+
+    expect_cleanupContainersShutdown();
+}
+
+/**
+ * @brief Test stopContainer with withPrejudice=true.
+ * Check that if runc still reports the container as Running after SIGKILL
+ * has been sent (e.g. a process is stuck in an uninterruptible sleep),
+ * stopContainer gives up after retrying and returns false rather than
+ * claiming success.
+ *
+ * @return false.
+ */
+TEST_F(DaemonDobbyManagerTest, stopContainer_ForceKill_ContainerStaysRunning_ReturnsFalse)
+{
+    int32_t cd = 1234;
+    ContainerId id = ContainerId::create("container1");
+    expect_invalidContainerCleanupTask();
+
+    expect_startContainerFromBundle(cd,id);
+
+    EXPECT_CALL(*p_runcMock, killCont(::testing::_, SIGKILL, ::testing::_))
+        .Times(1)
+        .WillOnce(::testing::Return(true));
+
+    EXPECT_CALL(*p_runcMock, state(::testing::_))
+        .Times(::testing::AtLeast(1))
+        .WillRepeatedly(::testing::Return(DobbyRunC::ContainerStatus::Running));
+
+    int return_value = dobbyManager_test->stopContainer(cd, true);
+    EXPECT_EQ(return_value, false);
+
+    expect_cleanupContainersShutdown();
+}
+
 /* -----------------------------------------------------------------------------
  *  @brief Gets the stats for the container
  *