Reload SCS binary when updating security-containers

[Feature]       Reload SCS without turning containers off when binary is updated.
[Cause]         When updating SCS we don't want to restart containers, only SCS itself.
[Solution]      Add SIGUSR1 handling which will tell SCS to keep containers alive when exiting.
                Add check in ContainerConnectionTransport if containers are running to skip remount
                of tmpfs when it is not needed.
[Verification]  Build, install, reboot target. Test the following when SCS is running together with
                containers active:
                  * Call "systemctl stop security-containers". SCS should turn off and containers
                    should turn off as well. Call "systemctl start security-containers", SCS and
                    containers should start up.
                  * Simulate update by calling "kill -USR1 `pidof security-containers-server`". SCS
                    should properly reload, however containers should stay on. (note - the best way
                    to check it would be by verifying logs in journalctl).

Change-Id: I3a6d0fb25a4579208ad0f6d0de00e2755548230e
Signed-off-by: Lukasz Kostyra <l.kostyra@samsung.com>

diff --git a/common/utils/fs.cpp b/common/utils/fs.cpp
index b3e15cf..aac496f 100644 (file)
--- a/common/utils/fs.cpp
+++ b/common/utils/fs.cpp
@@ -117,6 +117,25 @@ bool umount(const std::string& path)
     return true;
 }
 
+bool isMountPoint(const std::string& path, bool& result)
+{
+    struct stat stat, parentStat;
+    std::string parentPath = dirName(path);
+
+    if (::stat(path.c_str(), &stat)) {
+        LOGD("Failed to get stat of " << path << ": " << strerror(errno));
+        return false;
+    }
+
+    if (::stat(parentPath.c_str(), &parentStat)) {
+        LOGD("Failed to get stat of " << parentPath << ": " << strerror(errno));
+        return false;
+    }
+
+    result = (stat.st_dev != parentStat.st_dev);
+    return true;
+}
+
 
 } // namespace utils
 } // namespace security_containers

diff --git a/common/utils/fs.hpp b/common/utils/fs.hpp
index 52273dd..95ce200 100644 (file)
--- a/common/utils/fs.hpp
+++ b/common/utils/fs.hpp
@@ -67,6 +67,11 @@ bool mountTmpfs(const std::string& path);
  */
 bool umount(const std::string& path);
 
+/**
+ * Check if given path is a mount point
+ */
+bool isMountPoint(const std::string& path, bool& result);
+
 
 } // namespace utils
 } // namespace security_containers

diff --git a/server/container-admin.cpp b/server/container-admin.cpp
index 808bd76..657ab4f 100644 (file)
--- a/server/container-admin.cpp
+++ b/server/container-admin.cpp
@@ -69,6 +69,7 @@ ContainerAdmin::ContainerAdmin(ContainerConfig& config)
     : mConfig(config),
       mDom(utils::readFileContent(mConfig.config)),
       mId(getDomainName(mDom.get())),
+      mDetachOnExit(false),
       mLifecycleCallbackId(-1),
       mRebootCallbackId(-1),
       mNextIdForListener(0)
@@ -106,6 +107,8 @@ ContainerAdmin::ContainerAdmin(ContainerConfig& config)
 
 ContainerAdmin::~ContainerAdmin()
 {
+    LOGD(mId << ": Destroying ContainerAdmin object...");
+
     // Deregister callbacks
     if (mLifecycleCallbackId >= 0) {
         virConnectDomainEventDeregisterAny(virDomainGetConnect(mDom.get()),
@@ -117,11 +120,12 @@ ContainerAdmin::~ContainerAdmin()
     }
 
     // Try to forcefully stop
-    LOGD(mId << ": Destroying ContainerAdmin object...");
-    try {
-        destroy();
-    } catch (ServerException&) {
-        LOGE(mId << ": Failed to destroy the container");
+    if (!mDetachOnExit) {
+        try {
+            destroy();
+        } catch (ServerException&) {
+            LOGE(mId << ": Failed to destroy the container");
+        }
     }
 
     LOGD(mId << ": ContainerAdmin object destroyed");
@@ -144,10 +148,10 @@ void ContainerAdmin::start()
         return;
     }
 
-    // Autodestroyed when connection pointer released
-    // Any managed save file for this domain is discarded,
-    // and the domain boots from scratch
-    u_int flags = VIR_DOMAIN_START_AUTODESTROY;
+    // In order to update daemon without shutting down the containers
+    // autodestroy option must NOT be set. It's best to create domain
+    // without any flags.
+    u_int flags = VIR_DOMAIN_NONE;
 
     if (virDomainCreateWithFlags(mDom.get(), flags) < 0) {
         LOGE(mId << ": Failed to start the container\n"
@@ -362,6 +366,10 @@ void ContainerAdmin::setSchedulerParams(std::uint64_t cpuShares, std::uint64_t v
     }
 }
 
+void ContainerAdmin::setDetachOnExit()
+{
+    mDetachOnExit = true;
+}
 
 std::int64_t ContainerAdmin::getSchedulerQuota()
 {

diff --git a/server/container-admin.hpp b/server/container-admin.hpp
index add8f9a..ed1e00e 100644 (file)
--- a/server/container-admin.hpp
+++ b/server/container-admin.hpp
@@ -134,6 +134,11 @@ public:
     void setSchedulerLevel(SchedulerLevel sched);
 
     /**
+     * Set whether container should be detached on exit.
+     */
+    void setDetachOnExit();
+
+    /**
      * @return Scheduler CFS quota,
      * TODO: this function is only for UNIT TESTS
      */
@@ -160,6 +165,7 @@ private:
     ContainerConfig& mConfig;
     libvirt::LibvirtDomain mDom;
     const std::string mId;
+    bool mDetachOnExit;
 
     int getState();   // get the libvirt's domain state
     void setSchedulerParams(std::uint64_t cpuShares, std::uint64_t vcpuPeriod, std::int64_t vcpuQuota);

diff --git a/server/container-connection-transport.cpp b/server/container-connection-transport.cpp
index 7f4fb5e..3c14a67 100644 (file)
--- a/server/container-connection-transport.cpp
+++ b/server/container-connection-transport.cpp
@@ -43,7 +43,7 @@ const unsigned int TRANSPORT_READY_TIMEOUT = 2 * 60 * 1000;
 
 
 ContainerConnectionTransport::ContainerConnectionTransport(const std::string& runMountPoint)
-    : mRunMountPoint(runMountPoint)
+    : mRunMountPoint(runMountPoint), mDetachOnExit(false)
 {
     if (runMountPoint.empty()) {
         return;
@@ -53,12 +53,20 @@ ContainerConnectionTransport::ContainerConnectionTransport(const std::string& ru
         throw ContainerConnectionException("Could not create: " + runMountPoint);
     }
 
-    // try to umount if already mounted
-    utils::umount(runMountPoint);
+    bool isMount = false;
+    if (!utils::isMountPoint(runMountPoint, isMount)) {
+        LOGE("Failed to check if " << runMountPoint << " is a mount point.");
+        throw ContainerConnectionException("Could not check if " + runMountPoint +
+                                           " is a mount point.");
+    }
+
+    if (!isMount) {
+        LOGD(runMountPoint << " not mounted - mounting.");
 
-    if (!utils::mountTmpfs(runMountPoint)) {
-        LOGE("Initialization failed: could not mount " << runMountPoint);
-        throw ContainerConnectionException("Could not mount: " + runMountPoint);
+        if (!utils::mountTmpfs(runMountPoint)) {
+            LOGE("Initialization failed: could not mount " << runMountPoint);
+            throw ContainerConnectionException("Could not mount: " + runMountPoint);
+        }
     }
 
     // if there is no systemd in the container this dir won't be created automatically
@@ -73,9 +81,11 @@ ContainerConnectionTransport::ContainerConnectionTransport(const std::string& ru
 
 ContainerConnectionTransport::~ContainerConnectionTransport()
 {
-    if (!mRunMountPoint.empty()) {
-        if (!utils::umount(mRunMountPoint)) {
-            LOGE("Deinitialization failed: could not umount " << mRunMountPoint);
+    if (!mDetachOnExit) {
+        if (!mRunMountPoint.empty()) {
+            if (!utils::umount(mRunMountPoint)) {
+                LOGE("Deinitialization failed: could not umount " << mRunMountPoint);
+            }
         }
     }
 }
@@ -96,5 +106,9 @@ std::string ContainerConnectionTransport::acquireAddress()
     return "unix:path=" + dbusPath;
 }
 
+void ContainerConnectionTransport::setDetachOnExit()
+{
+    mDetachOnExit = true;
+}
 
 } // namespace security_containers

diff --git a/server/container-connection-transport.hpp b/server/container-connection-transport.hpp
index 989ba96..0be1035 100644 (file)
--- a/server/container-connection-transport.hpp
+++ b/server/container-connection-transport.hpp
@@ -46,8 +46,14 @@ public:
      */
     std::string acquireAddress();
 
+    /**
+     * Set whether object should detach from transport filesystem on exit
+     */
+    void setDetachOnExit();
+
 private:
     std::string mRunMountPoint;
+    bool mDetachOnExit;
 };
 
 

diff --git a/server/container.cpp b/server/container.cpp
index 4717429..a4ad5f1 100644 (file)
--- a/server/container.cpp
+++ b/server/container.cpp
@@ -103,6 +103,12 @@ void Container::goBackground()
     mAdmin->setSchedulerLevel(SchedulerLevel::BACKGROUND);
 }
 
+void Container::setDetachOnExit()
+{
+    mAdmin->setDetachOnExit();
+    mConnectionTransport->setDetachOnExit();
+}
+
 bool Container::isRunning()
 {
     return mAdmin->isRunning();

diff --git a/server/container.hpp b/server/container.hpp
index 5607715..26a94c5 100644 (file)
--- a/server/container.hpp
+++ b/server/container.hpp
@@ -79,6 +79,14 @@ public:
     void goBackground();
 
     /**
+     * Set if container should be detached on exit.
+     *
+     * This sends detach flag to ContainerAdmin object and disables unmounting tmpfs
+     * in ContainerConnectionTransport.
+     */
+    void setDetachOnExit();
+
+    /**
      * @return Is the container running?
      */
     bool isRunning();

diff --git a/server/containers-manager.cpp b/server/containers-manager.cpp
index 288bdef..0aaebc7 100644 (file)
--- a/server/containers-manager.cpp
+++ b/server/containers-manager.cpp
@@ -36,7 +36,7 @@
 namespace security_containers {
 
 
-ContainersManager::ContainersManager(const std::string& managerConfigPath)
+ContainersManager::ContainersManager(const std::string& managerConfigPath): mDetachOnExit(false)
 {
     LOGD("Instantiating ContainersManager object...");
     mConfig.parseFile(managerConfigPath);
@@ -64,11 +64,15 @@ ContainersManager::ContainersManager(const std::string& managerConfigPath)
 ContainersManager::~ContainersManager()
 {
     LOGD("Destroying ContainersManager object...");
-    try {
-        stopAll();
-    } catch (ServerException&) {
-        LOGE("Failed to stop all of the containers");
+
+    if (!mDetachOnExit) {
+        try {
+            stopAll();
+        } catch (ServerException&) {
+            LOGE("Failed to stop all of the containers");
+        }
     }
+
     LOGD("ContainersManager object destroyed");
 }
 
@@ -139,4 +143,13 @@ std::string ContainersManager::getRunningForegroundContainerId()
 }
 
 
+void ContainersManager::setContainersDetachOnExit()
+{
+    mDetachOnExit = true;
+
+    for (auto& container : mContainers) {
+        container.second->setDetachOnExit();
+    }
+}
+
 } // namespace security_containers

diff --git a/server/containers-manager.hpp b/server/containers-manager.hpp
index e3a428c..920d5fd 100644 (file)
--- a/server/containers-manager.hpp
+++ b/server/containers-manager.hpp
@@ -66,10 +66,16 @@ public:
      */
     std::string getRunningForegroundContainerId();
 
+    /**
+     * Set whether ContainersManager should detach containers on exit
+     */
+    void setContainersDetachOnExit();
+
 private:
     ContainersManagerConfig mConfig;
     typedef std::unordered_map<std::string, std::unique_ptr<Container>> ContainerMap;
     ContainerMap mContainers; // map of containers, id is the key
+    bool mDetachOnExit;
 };
 
 

diff --git a/server/main.cpp b/server/main.cpp
index d911071..395d936 100644 (file)
--- a/server/main.cpp
+++ b/server/main.cpp
@@ -114,6 +114,7 @@ int main(int argc, char* argv[])
     try {
         Server server(configPath);
         server.run();
+        server.reloadIfRequired(argv);
 
     } catch (std::exception& e) {
         LOGE("Unexpected: " << utils::getTypeName(e) << ": " << e.what());

diff --git a/server/server.cpp b/server/server.cpp
index 01b0ab3..82d0a6d 100644 (file)
--- a/server/server.cpp
+++ b/server/server.cpp
@@ -30,7 +30,13 @@
 #include "utils/glib-loop.hpp"
 
 #include <csignal>
+#include <cerrno>
 #include <string>
+#include <cstring>
+#include <atomic>
+#include <unistd.h>
+
+extern char** environ;
 
 namespace security_containers {
 
@@ -47,18 +53,29 @@ Server::~Server()
 
 
 namespace {
-utils::Latch signalLatch;
+
+std::atomic_bool gUpdateTriggered(false);
+utils::Latch gSignalLatch;
+
 void signalHandler(const int sig)
 {
     LOGI("Got signal " << sig);
-    signalLatch.set();
-}
+
+    if (sig == SIGUSR1) {
+        LOGD("Received SIGUSR1 - triggering update.");
+        gUpdateTriggered = true;
+    }
+
+    gSignalLatch.set();
 }
 
+} // namespace
+
 void Server::run()
 {
     signal(SIGINT,  signalHandler);
     signal(SIGTERM, signalHandler);
+    signal(SIGUSR1, signalHandler);
 
     LOGI("Starting daemon...");
     {
@@ -68,7 +85,12 @@ void Server::run()
         manager.startAll();
         LOGI("Daemon started");
 
-        signalLatch.wait();
+        gSignalLatch.wait();
+
+        // Detach containers if we triggered an update
+        if (gUpdateTriggered) {
+            manager.setContainersDetachOnExit();
+        }
 
         LOGI("Stopping daemon...");
         // manager.stopAll() will be called in destructor
@@ -76,10 +98,19 @@ void Server::run()
     LOGI("Daemon stopped");
 }
 
+void Server::reloadIfRequired(char* argv[])
+{
+    if (gUpdateTriggered) {
+        execve(argv[0], argv, environ);
+
+        LOGE("Failed to reload " << argv[0] << ": " << strerror(errno));
+    }
+}
+
 void Server::terminate()
 {
     LOGI("Terminating server");
-    signalLatch.set();
+    gSignalLatch.set();
 }
 
 } // namespace security_containers

diff --git a/server/server.hpp b/server/server.hpp
index 59a1a35..abb8775 100644 (file)
--- a/server/server.hpp
+++ b/server/server.hpp
@@ -38,11 +38,16 @@ public:
     virtual ~Server();
 
     /**
-     * Starts all the containers and blocks until SIGINT or SIGTERM
+     * Starts all the containers and blocks until SIGINT, SIGTERM or SIGUSR1
      */
     void run();
 
     /**
+     * Reload the server by launching execve on itself if SIGUSR1 was sent to server.
+     */
+    void reloadIfRequired(char* argv[]);
+
+    /**
      * Terminates the server.
      * Equivalent of sending SIGINT or SIGTERM signal
      */