Makefile.am               |   32 +++++------
 common/JailUtil.cpp       |  131 +++++++++++++++++++++++++---------------------
 common/JailUtil.hpp       |   35 ++++++------
 kit/ForKit.cpp            |   15 ++---
 kit/Kit.cpp               |  114 +++++++++++++++++++++++++---------------
 loolwsd-systemplate-setup |   34 ++++++++++-
 6 files changed, 219 insertions(+), 142 deletions(-)

New commits:
commit 29a5a1f1e97e10aa6f9bdbed0b1062e2ab4e128b
Author:     Ashod Nakashian <ashod.nakash...@collabora.co.uk>
AuthorDate: Sun Aug 23 12:11:23 2020 -0400
Commit:     Andras Timar <andras.ti...@collabora.com>
CommitDate: Tue Aug 25 07:58:30 2020 +0200

    wsd: move jail setup to the script to support readonly systemplate
    
    We now gracefully fallback to copying when/if systemplate
    is readonly.
    
    The bulk of the change is to support proper cleanup in
    both cases.
    
    First, we had to move as much of the jail bootstrapping
    into the loolwsd-systemplate-setup script, so systemplate
    will be as complete as possible before it is locked down.
    Next, we needed to update the jail with graceful fallback
    to linking/copying upon failure. For that, the jail setup
    logic in Kit.cpp has been reworked to support not just
    update failures, but also more comprehensive mounting
    failures as well.
    
    Finally, jail cleanup now is seamless. To support proper
    cleanup when we had mounting enabled but had to fallback,
    we mark jails that aren't mounted so we can 'rm -rf' the
    contents safely and without fear or causing undue damage
    (as unlikely as that is, technically we wouldn't want to
    rm systemplate files, if mounting read-only had failed).
    
    There are a few minor refactorings of JailUtil to make
    it cleaner and more robust.
    
    Change-Id: Iac34869cb84f45acf64fbbc46d46898367b496d2
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/101260
    Tested-by: Jenkins
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
    Reviewed-by: Andras Timar <andras.ti...@collabora.com>

diff --git a/Makefile.am b/Makefile.am
index e2c4b57e6..e50b68022 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -335,27 +335,26 @@ SYSTEM_STAMP = @SYSTEMPLATE_PATH@/system_stamp
 CAPABILITIES = $(if @ENABLE_SETCAP@,true,false)
 RUN_GDB = $(if $(GDB_FRONTEND),$(GDB_FRONTEND),gdb --tui --args)
 
-# Add caps to the binaries that need them.
-caps_bins: loolforkit loolmount
 if ENABLE_SETCAP
-           sudo @SETCAP@ cap_fowner,cap_mknod,cap_sys_chroot=ep loolforkit
-           sudo @SETCAP@ cap_sys_admin=ep loolmount
+SET_CAPS_COMMAND=sudo @SETCAP@ cap_fowner,cap_mknod,cap_sys_chroot=ep 
loolforkit && sudo @SETCAP@ cap_sys_admin=ep loolmount
 else
-           echo "Skipping capability setting"
+SET_CAPS_COMMAND=
+echo "Skipping capability setting"
 endif
 
 if ENABLE_LIBFUZZER
 CLEANUP_DEPS=
 else
-CLEANUP_DEPS=loolwsd
+CLEANUP_DEPS=loolwsd loolmount loolforkit
 endif
 
 # Build loolwsd and loolmount first, so we can cleanup before updating
 # the systemplate directory, which we can't rm if it's mounted.
-$(SYSTEM_STAMP): ${top_srcdir}/loolwsd-systemplate-setup $(CLEANUP_DEPS) 
caps_bins
+$(SYSTEM_STAMP): ${top_srcdir}/loolwsd-systemplate-setup $(CLEANUP_DEPS)
+       $(SET_CAPS_COMMAND)
        $(CLEANUP_COMMAND)
-       if test "z@SYSTEMPLATE_PATH@" != "z"; then rm -rf "@SYSTEMPLATE_PATH@"; 
fi
-       ${top_srcdir}/loolwsd-systemplate-setup "@SYSTEMPLATE_PATH@" 
"@LO_PATH@" && touch $@
+       if test "z@SYSTEMPLATE_PATH@" != "z"; then rm -rf "@SYSTEMPLATE_PATH@" 
&& \
+       ${top_srcdir}/loolwsd-systemplate-setup "@SYSTEMPLATE_PATH@" 
"@LO_PATH@" && touch $@; fi
 
 @JAILS_PATH@:
        $(CLEANUP_COMMAND)
@@ -486,12 +485,7 @@ SYSTEM_STAMP =
 
 endif
 
-# After building loolforkit, set its capabilities as required. Do it
-# already after a plain 'make' to allow for testing without
-# installing. When building for packaging, no need for this, as the
-# capabilities won't survive packaging anyway. Instead, handle it when
-# installing the RPM or Debian package.
-.PHONY: caps_bins cleanup
+.PHONY: cleanup
 
 if ENABLE_LIBFUZZER
 ALL_LOCAL_DEPS=
@@ -499,7 +493,13 @@ else
 ALL_LOCAL_DEPS=loolwsd
 endif
 
-all-local: $(ALL_LOCAL_DEPS) caps_bins @JAILS_PATH@ $(SYSTEM_STAMP)
+# After building loolforkit, set its capabilities as required. Do it
+# already after a plain 'make' to allow for testing without
+# installing. When building for packaging, no need for this, as the
+# capabilities won't survive packaging anyway. Instead, handle it when
+# installing the RPM or Debian package.
+all-local: $(ALL_LOCAL_DEPS) @JAILS_PATH@ $(SYSTEM_STAMP)
+       $(SET_CAPS_COMMAND)
 
 # just run the build without any tests
 build-nocheck: all-am
diff --git a/common/JailUtil.cpp b/common/JailUtil.cpp
index c7ce612df..7b313d470 100644
--- a/common/JailUtil.cpp
+++ b/common/JailUtil.cpp
@@ -54,9 +54,9 @@ bool remountReadonly(const std::string& source, const 
std::string& target)
     Poco::File(target).createDirectory();
     const bool res = loolmount("-r", source, target);
     if (res)
-        LOG_TRC("Mounted [" << source << "] -> [" << target << "].");
+        LOG_TRC("Mounted [" << source << "] -> [" << target << "] readonly.");
     else
-        LOG_ERR("Failed to mount [" << source << "] -> [" << target << "].");
+        LOG_ERR("Failed to mount [" << source << "] -> [" << target << "] 
readonly.");
     return res;
 }
 
@@ -71,38 +71,69 @@ bool unmount(const std::string& target)
     return res;
 }
 
+// This file signifies that we copied instead of mounted.
+// NOTE: jail cleanup helpers are called from forkit and
+// loolwsd, and they may have bind-mounting enabled, but the
+// kit could have had it removed when falling back to copying.
+// In such cases, we cannot safely know whether the jail was
+// copied or not, since the bind envar will be present and
+// assuming it was mounted would leak them.
+// Alternatively, we if remove the files when mounted
+// we could destroy systemplate if remounting read-only had
+// failed (and it wasn't owned by root).
+constexpr const char* COPIED_JAIL_MARKER_FILE = "delete.me";
+
+void markJailCopied(const std::string& root)
+{
+    // The reason we should be able to create this file
+    // is because the jail must be writable.
+    // Failing this will cause an exception, signaling an error.
+    Poco::File(root + '/' + COPIED_JAIL_MARKER_FILE).createFile();
+}
+
+bool isJailCopied(const std::string& root)
+{
+    // If the marker file exists, the jail was copied.
+    FileUtil::Stat delFileStat(root + '/' + COPIED_JAIL_MARKER_FILE);
+    return delFileStat.exists();
+}
+
 bool safeRemoveDir(const std::string& path)
 {
+    // Always unmount, just in case.
     unmount(path);
 
-    static const bool bind = std::getenv("LOOL_BIND_MOUNT");
+    // Regardless of the bind flag, check if the jail is marked as copied.
+    const bool copied = isJailCopied(path);
 
     // We must be empty if we had mounted.
-    if (bind && !FileUtil::isEmptyDirectory(path))
+    if (!copied && JailUtil::isBindMountingEnabled() && 
!FileUtil::isEmptyDirectory(path))
     {
         LOG_WRN("Path [" << path << "] is not empty. Will not remove it.");
         return false;
     }
 
     // Recursively remove if link/copied.
-    FileUtil::removeFile(path, !bind);
+    const bool recursive = copied;
+    FileUtil::removeFile(path, recursive);
     return true;
 }
 
-void removeJail(const std::string& path)
+void removeJail(const std::string& root)
 {
-    LOG_INF("Removing jail [" << path << "].");
+    LOG_INF("Removing jail [" << root << "].");
 
     // Unmount the tmp directory. Don't care if we fail.
-    const std::string tmpPath = Poco::Path(path, "tmp").toString();
-    FileUtil::removeFile(tmpPath, true); // Delete tmp contents with prejeduce.
+    const std::string tmpPath = Poco::Path(root, "tmp").toString();
+    FileUtil::removeFile(tmpPath, true); // Delete tmp contents with prejudice.
     unmount(tmpPath);
 
     // Unmount the loTemplate directory.
-    unmount(Poco::Path(path, "lo").toString());
+    //FIXME: technically, the loTemplate directory may have any name.
+    unmount(Poco::Path(root, "lo").toString());
 
-    // Unmount the jail (sysTemplate).
-    safeRemoveDir(path);
+    // Unmount/delete the jail (sysTemplate).
+    safeRemoveDir(root);
 }
 
 /// This cleans up the jails directories.
@@ -121,6 +152,7 @@ void cleanupJails(const std::string& root)
         return;
     }
 
+    //FIXME: technically, the loTemplate directory may have any name.
     if (FileUtil::Stat(root + "/lo").exists())
     {
         // This is a jail.
@@ -156,7 +188,7 @@ void setupJails(bool bindMount, const std::string& 
jailRoot, const std::string&
     cleanupJails(jailRoot);
     Poco::File(jailRoot).createDirectories();
 
-    unsetenv("LOOL_BIND_MOUNT"); // Clear to avoid surprises.
+    disableBindMounting(); // Clear to avoid surprises.
     if (bindMount)
     {
         // Test mounting to verify it actually works,
@@ -164,8 +196,8 @@ void setupJails(bool bindMount, const std::string& 
jailRoot, const std::string&
         const std::string target = Poco::Path(jailRoot, 
"lool_test_mount").toString();
         if (bind(sysTemplate, target))
         {
+            enableBindMounting();
             safeRemoveDir(target);
-            setenv("LOOL_BIND_MOUNT", "1", 1);
             LOG_INF("Enabling Bind-Mounting of jail contents for better 
performance per "
                     "mount_jail_tree config in loolwsd.xml.");
         }
@@ -178,35 +210,6 @@ void setupJails(bool bindMount, const std::string& 
jailRoot, const std::string&
                 "mount_jail_tree config in loolwsd.xml.");
 }
 
-void symlinkPathToJail(const std::string& sysTemplate, const std::string& 
loTemplate,
-                       const std::string& loSubPath)
-{
-    std::string symlinkTarget;
-    for (int i = 0; i < Poco::Path(loTemplate).depth(); i++)
-        symlinkTarget += "../";
-    symlinkTarget += loSubPath;
-
-    const Poco::Path symlinkSourcePath(sysTemplate + '/' + loTemplate);
-    const std::string symlinkSource = symlinkSourcePath.toString();
-    Poco::File(symlinkSourcePath.parent()).createDirectories();
-
-    LOG_DBG("Linking symbolically [" << symlinkSource << "] to [" << 
symlinkTarget << "].");
-
-    const FileUtil::Stat stLink(symlinkSource, true); // The file is a link.
-    if (stLink.exists())
-    {
-        if (!stLink.isLink())
-            LOG_WRN("Link [" << symlinkSource << "] already exists but isn't a 
link.");
-        else
-            LOG_TRC("Link [" << symlinkSource << "] already exists, skipping 
linking.");
-
-        return;
-    }
-
-    if (symlink(symlinkTarget.c_str(), symlinkSource.c_str()) == -1)
-        LOG_SYS("Failed to symlink(\"" << symlinkTarget << "\", \"" << 
symlinkSource << "\")");
-}
-
 // This is the second stage of setting up /dev/[u]random
 // in the jails. Here we create the random devices in
 // /tmp/dev/ in the jail chroot. See setupRandomDeviceLinks().
@@ -239,6 +242,27 @@ void setupJailDevNodes(const std::string& root)
     }
 }
 
+/// The envar name used to control bind-mounting of systemplate/jails.
+constexpr const char* BIND_MOUNTING_ENVAR_NAME = "LOOL_BIND_MOUNT";
+
+void enableBindMounting()
+{
+    // Set the envar to enable.
+    setenv(BIND_MOUNTING_ENVAR_NAME, "1", 1);
+}
+
+void disableBindMounting()
+{
+    // Remove the envar to disable.
+    unsetenv(BIND_MOUNTING_ENVAR_NAME);
+}
+
+bool isBindMountingEnabled()
+{
+    // Check if we have a valid envar set.
+    return std::getenv(BIND_MOUNTING_ENVAR_NAME) != nullptr;
+}
+
 namespace SysTemplate
 {
 /// The network and other system files we need to keep up-to-date in jails.
@@ -268,12 +292,14 @@ void setupDynamicFiles(const std::string& sysTemplate)
                 << sysTemplate
                 << "]. Will disable bind-mounting in this run and clone 
systemplate into the "
                    "jails, which is more resource intensive.");
-        unsetenv("LOOL_BIND_MOUNT"); // We can't mount from incomplete 
systemplate.
+        disableBindMounting(); // We can't mount from incomplete systemplate.
+        LinkDynamicFiles = false;
     }
 
-    if (LinkDynamicFiles)
-        LOG_INF("Systemplate dynamic files in ["
-                << sysTemplate << "] are linked and will remain up-to-date.");
+    LOG_INF("Systemplate dynamic files in ["
+            << sysTemplate << "] "
+            << (LinkDynamicFiles ? "are linked and will remain" : "will be 
copied to keep them")
+            << " up-to-date.");
 }
 
 bool updateDynamicFilesImpl(const std::string& sysTemplate)
@@ -350,17 +376,6 @@ bool updateDynamicFiles(const std::string& sysTemplate)
     return LinkDynamicFiles ? true : updateDynamicFilesImpl(sysTemplate);
 }
 
-void setupLoSymlink(const std::string& sysTemplate, const std::string& 
loTemplate,
-                    const std::string& loSubPath)
-{
-    symlinkPathToJail(sysTemplate, loTemplate, loSubPath);
-
-    // Font paths can end up as realpaths so match that too.
-    const std::string resolved = FileUtil::realpath(loTemplate);
-    if (loTemplate != resolved)
-        symlinkPathToJail(sysTemplate, resolved, loSubPath);
-}
-
 void setupRandomDeviceLink(const std::string& sysTemplate, const std::string& 
name)
 {
     const std::string path = sysTemplate + "/dev/";
diff --git a/common/JailUtil.hpp b/common/JailUtil.hpp
index 949fdca50..0fad9950a 100644
--- a/common/JailUtil.hpp
+++ b/common/JailUtil.hpp
@@ -25,17 +25,14 @@ bool remountReadonly(const std::string& source, const 
std::string& target);
 /// Unmount a bind-mounted jail directory.
 bool unmount(const std::string& target);
 
-/// Remove the jail directory and all its contents.
-void removeJail(const std::string& path);
+/// Marks a jail as having been copied instead of mounted.
+void markJailCopied(const std::string& root);
 
-/// Remove all the jails given their paths.
-inline void removeJails(const std::vector<std::string>& jails)
-{
-    for (const auto& path : jails)
-    {
-        removeJail(path);
-    }
-}
+/// Returns true iff the jail in question was copied and not mounted.
+bool isJailCopied(const std::string& root);
+
+/// Remove the jail directory and all its contents.
+void removeJail(const std::string& root);
 
 /// Remove all jails.
 void cleanupJails(const std::string& jailRoot);
@@ -46,18 +43,24 @@ void setupJails(bool bindMount, const std::string& 
jailRoot, const std::string&
 /// Setup /dev/random and /dev/urandom in the given jail path.
 void setupJailDevNodes(const std::string& root);
 
+/// Enable bind-mounting in this process.
+void enableBindMounting();
+
+/// Disable bind-mounting in this process.
+void disableBindMounting();
+
+/// Returns true iff bind-mounting is enabled in this process.
+bool isBindMountingEnabled();
+
 namespace SysTemplate
 {
-/// Create a symlink inside the jailPath so that the absolute pathname 
loTemplate, when
-/// interpreted inside a chroot at jailPath, points to loSubPath (relative to 
the chroot).
-void setupLoSymlink(const std::string& sysTemplate, const std::string& 
loTemplate,
-                    const std::string& loSubPath);
-
 /// Setup links for /dev/random and /dev/urandom in systemplate.
 void setupRandomDeviceLinks(const std::string& root);
 
-/// Setup of the dynamic files within the sysTemplate by either
+/// Setup the dynamic files within the sysTemplate by either
 /// copying or linking. See updateJail_DynamicFilesInSysTemplate.
+/// If the dynamic files need updating and systemplate is read-only,
+/// this will fail and mark files for copying.
 void setupDynamicFiles(const std::string& sysTemplate);
 
 /// Update the dynamic files within the sysTemplate before each child fork.
diff --git a/kit/ForKit.cpp b/kit/ForKit.cpp
index 9afa62b4e..492dc300c 100644
--- a/kit/ForKit.cpp
+++ b/kit/ForKit.cpp
@@ -253,7 +253,8 @@ static void cleanupChildren()
 {
     std::vector<std::string> jails;
     pid_t exitedChildPid;
-    int status, segFaultCount = 0;
+    int status = 0;
+    int segFaultCount = 0;
 
     // Reap quickly without doing slow cleanup so WSD can spawn more rapidly.
     while ((exitedChildPid = waitpid(-1, &status, WUNTRACED | WNOHANG)) > 0)
@@ -272,7 +273,7 @@ static void cleanupChildren()
 
             if (WIFSIGNALED(status) && (WTERMSIG(status) == SIGSEGV || 
WTERMSIG(status) == SIGBUS))
             {
-                segFaultCount ++;
+                ++segFaultCount;
             }
         }
         else
@@ -306,7 +307,10 @@ static void cleanupChildren()
     }
 
     // Now delete the jails.
-    JailUtil::removeJails(jails);
+    for (const std::string& path : jails)
+    {
+        JailUtil::removeJail(path);
+    }
 }
 
 static int createLibreOfficeKit(const std::string& childRoot,
@@ -613,12 +617,9 @@ int main(int argc, char** argv)
     if (Util::getProcessThreadCount() != 1)
         LOG_ERR("Error: forkit has more than a single thread after pre-init");
 
-    // Link the network and system files in sysTemplate.
+    // Link the network and system files in sysTemplate, if possible.
     JailUtil::SysTemplate::setupDynamicFiles(sysTemplate);
 
-    // Make the real lo path in the chroot point to the chroot lo/.
-    JailUtil::SysTemplate::setupLoSymlink(sysTemplate, loTemplate, loSubPath);
-
     // Make dev/[u]random point to the writable devices in tmp/dev/.
     JailUtil::SysTemplate::setupRandomDeviceLinks(sysTemplate);
 
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 2a312b986..30cd4e5f8 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -11,6 +11,7 @@
  * a document editing session.
  */
 
+#include <Poco/File.h>
 #include <config.h>
 
 #include <dlfcn.h>
@@ -2119,9 +2120,10 @@ void lokit_main(
     {
 #if !MOBILEAPP
         const Path jailPath = Path::forDirectory(childRoot + '/' + jailId);
-        LOG_INF("Jail path: " << jailPath.toString());
+        const std::string jailPathStr = jailPath.toString();
+        LOG_INF("Jail path: " << jailPathStr);
         File(jailPath).createDirectories();
-        chmod(jailPath.toString().c_str(), S_IXUSR | S_IWUSR | S_IRUSR);
+        chmod(jailPathStr.c_str(), S_IXUSR | S_IWUSR | S_IRUSR);
 
         if (!ChildSession::NoCapsForKit)
         {
@@ -2131,66 +2133,96 @@ void lokit_main(
             userdir_url = "file:///tmp/user";
             instdir_path = '/' + loSubPath + "/program";
 
-            // Copy (link) LO installation and other necessary files into it 
from the template.
-            if (std::getenv("LOOL_BIND_MOUNT"))
-            {
-                const std::string destPath = jailPath.toString();
-                LOG_INF("Mounting " << sysTemplate << " -> " << destPath);
-                if (!JailUtil::bind(sysTemplate, destPath))
+            Poco::Path jailLOInstallation(jailPath, loSubPath);
+            jailLOInstallation.makeDirectory();
+            const std::string loJailDestPath = jailLOInstallation.toString();
+
+            // The bind-mount implementation: inlined here to mirror
+            // the fallback link/copy version bellow.
+            const auto mountJail = [&]() -> bool {
+                // Mount sysTemplate for the jail directory.
+                LOG_INF("Mounting " << sysTemplate << " -> " << jailPathStr);
+                if (!JailUtil::bind(sysTemplate, jailPathStr)
+                    || !JailUtil::remountReadonly(sysTemplate, jailPathStr))
                 {
-                    LOG_WRN("Failed to mount [" << sysTemplate << "] -> [" << 
destPath
+                    LOG_WRN("Failed to mount [" << sysTemplate << "] -> [" << 
jailPathStr
                                                 << "], will link/copy 
contents.");
-                    linkOrCopy(sysTemplate, destPath, LinkOrCopyType::All);
+                    return false;
                 }
-                else
-                    JailUtil::remountReadonly(sysTemplate, 
jailPath.toString());
 
-                // Mount lotemplate inside it.
-                const std::string loDestPath = Poco::Path(jailPath, 
"lo").toString();
-                LOG_INF("Mounting " << loTemplate << " -> " << loDestPath);
-                Poco::File(loDestPath).createDirectories();
-                if (!JailUtil::bind(loTemplate, loDestPath))
+                // Mount loTemplate inside it.
+                LOG_INF("Mounting " << loTemplate << " -> " << loJailDestPath);
+                Poco::File(loJailDestPath).createDirectories();
+                if (!JailUtil::bind(loTemplate, loJailDestPath)
+                    || !JailUtil::remountReadonly(loTemplate, loJailDestPath))
                 {
-                    LOG_WRN("Failed to mount [" << loTemplate << "] -> [" << 
loDestPath
+                    LOG_WRN("Failed to mount [" << loTemplate << "] -> [" << 
loJailDestPath
                                                 << "], will link/copy 
contents.");
-                    linkOrCopy(sysTemplate, loDestPath, LinkOrCopyType::LO);
+                    return false;
                 }
-                else
-                    JailUtil::remountReadonly(loTemplate, loDestPath);
 
-                // hard-random tmpdir inside the jail / root
+                // Hard-random tmpdir inside the jail for added sercurity.
                 const std::string tempRoot = Poco::Path(childRoot, 
"tmp").toString();
                 Poco::File(tempRoot).createDirectories();
                 const std::string tmpSubDir = 
Util::createRandomTmpDir(tempRoot);
                 const std::string jailTmpDir = Poco::Path(jailPath, 
"tmp").toString();
+                LOG_INF("Mounting random temp dir " << tmpSubDir << " -> " << 
jailTmpDir);
                 if (!JailUtil::bind(tmpSubDir, jailTmpDir))
                 {
-                    LOG_ERR("Failed to bind tmp dir in jail.");
+                    LOG_WRN("Failed to mount [" << tmpSubDir << "] -> [" << 
jailTmpDir
+                                                << "], will link/copy 
contents.");
+                    return false;
                 }
 
-                JailUtil::setupJailDevNodes(destPath + "/tmp");
+                return true;
+            };
+
+            // Copy (link) LO installation and other necessary files into it 
from the template.
+            bool bindMount = JailUtil::isBindMountingEnabled();
+            if (bindMount)
+            {
+                if (!mountJail())
+                {
+                    LOG_INF("Cleaning up jail before linking/copying.");
+                    JailUtil::removeJail(jailPathStr);
+                    bindMount = false;
+                    JailUtil::disableBindMounting();
+                }
             }
-            else
+
+            if (!bindMount)
             {
-                linkOrCopy(sysTemplate, jailPath, LinkOrCopyType::All);
+                LOG_INF("Mounting is disabled, will link/copy " << sysTemplate 
<< " -> "
+                                                                << 
jailPathStr);
 
-                Poco::Path jailLOInstallation(jailPath, loSubPath);
-                jailLOInstallation.makeDirectory();
-                Poco::File(jailLOInstallation).createDirectory();
-                linkOrCopy(loTemplate, jailLOInstallation, LinkOrCopyType::LO);
+                linkOrCopy(sysTemplate, jailPath, LinkOrCopyType::All);
 
-                JailUtil::setupJailDevNodes(Poco::Path(jailPath, 
"/tmp").toString());
+                linkOrCopy(loTemplate, loJailDestPath, LinkOrCopyType::LO);
 
                 // Update the dynamic files inside the jail.
-                if 
(!JailUtil::SysTemplate::updateDynamicFiles(jailPath.toString()))
+                if (!JailUtil::SysTemplate::updateDynamicFiles(jailPathStr))
                 {
-                    LOG_WRN("Failed to update the dynamic files in the jail ["
-                            << jailPath.toString() << "]. Some functionality 
may be missing.");
+                    LOG_WRN(
+                        "Failed to update the dynamic files in the jail ["
+                        << jailPathStr
+                        << "]. If the systemplate directory is owned by a 
superuser or is "
+                           "read-only, running the installation scripts with 
the owner's account "
+                           "should update these files. Some functionality may 
be missing.");
                 }
+
+                // Create a file to mark this a copied jail.
+                JailUtil::markJailCopied(jailPathStr);
             }
 
+            // Setup the devices inside /tmp and set TMPDIR.
+            JailUtil::setupJailDevNodes(Poco::Path(jailPath, 
"/tmp").toString());
             ::setenv("TMPDIR", "/tmp", 1);
 
+            // HOME must be writable, so create it in /tmp.
+            constexpr const char* HomePathInJail = "/tmp/home";
+            Poco::File(Poco::Path(jailPath, 
HomePathInJail)).createDirectories();
+            ::setenv("HOME", HomePathInJail, 1);
+
             const auto ms = 
std::chrono::duration_cast<std::chrono::milliseconds>(
                                 std::chrono::steady_clock::now() - 
jailSetupStartTime)
                                 .count();
@@ -2200,10 +2232,10 @@ void lokit_main(
             if (ProcSMapsFile < 0)
                 LOG_SYS("Failed to open /proc/self/smaps. Memory stats will be 
missing.");
 
-            LOG_INF("chroot(\"" << jailPath.toString() << "\")");
-            if (chroot(jailPath.toString().c_str()) == -1)
+            LOG_INF("chroot(\"" << jailPathStr << "\")");
+            if (chroot(jailPathStr.c_str()) == -1)
             {
-                LOG_SFL("chroot(\"" << jailPath.toString() << "\") failed.");
+                LOG_SFL("chroot(\"" << jailPathStr << "\") failed.");
                 Log::shutdown();
                 std::_Exit(EX_SOFTWARE);
             }
@@ -2223,8 +2255,9 @@ void lokit_main(
         }
         else // noCapabilities set
         {
-            LOG_ERR("Security warning - using template " << loTemplate << " as 
install subpath - skipping chroot jail setup");
-            userdir_url = "file:///" + jailPath.toString() + "/tmp/user";
+            LOG_ERR("Security warning - using template "
+                    << loTemplate << " as install subpath - skipping chroot 
jail setup");
+            userdir_url = "file:///" + jailPathStr + "/tmp/user";
             instdir_path = '/' + loTemplate + "/program";
         }
 
@@ -2243,7 +2276,8 @@ void lokit_main(
 #endif
             if (!kit)
             {
-                kit = (initFunction ? initFunction(instdir, userdir) : 
lok_init_2(instdir, userdir));
+                kit = (initFunction ? initFunction(instdir, userdir)
+                                    : lok_init_2(instdir, userdir));
             }
 
             loKit = std::make_shared<lok::Office>(kit);
diff --git a/loolwsd-systemplate-setup b/loolwsd-systemplate-setup
index b50d10cc3..ea04a03de 100755
--- a/loolwsd-systemplate-setup
+++ b/loolwsd-systemplate-setup
@@ -1,4 +1,5 @@
 #!/bin/bash
+#set -x
 
 test $# -eq 2 || { echo "Usage: $0 <chroot template directory for system libs 
to create> <LO installation directory>"; exit 1; }
 
@@ -11,8 +12,14 @@ test -d "$INSTDIR" || { echo "No such directory: $INSTDIR"; 
exit 1; }
 
 mkdir -p $CHROOT || exit 1
 
+# Resolve the real paths, in case they are relative and/or symlinked.
+# INSTDIR_LOGICAL will contain the logical path, if there are symlinks,
+# while INSTDIR is the physical one. Both will most likely be the same,
+# except on systems that have symlinks in the path. We must create
+# both paths (if they are different) inside the jail, hence we need both.
 CHROOT=`cd $CHROOT && /bin/pwd`
-INSTDIR=`cd $INSTDIR && /bin/pwd`
+INSTDIR_LOGICAL=`cd $INSTDIR && /bin/pwd -L`
+INSTDIR=`cd $INSTDIR && /bin/pwd -P`
 
 cd / || exit 1
 
@@ -58,10 +65,6 @@ grep -v dynamic | cut -d " " -f 3 | grep -E '^(/lib|/usr)' | 
sort -u | sed -e 's
 # This will now copy the file a symlink points to, but whatever.
 cpio -p -d -L $CHROOT
 
-mkdir -p $CHROOT/lo
-mkdir -p $CHROOT/dev
-mkdir -p $CHROOT/tmp/dev
-
 # Link the dynamic files, replacing any existing.
 for file in hosts nsswitch.conf resolv.conf passwd group host.conf timezone 
localtime
 do
@@ -70,6 +73,27 @@ do
     ln -f /etc/$file $CHROOT/etc/ 2> /dev/null || ln -f -s /etc/$file 
$CHROOT/etc/ || cp /etc/$file $CHROOT/etc/ || echo "Failed to link or copy 
$file"
 done
 
+# Link dev/random and dev/urandom to ../tmp/dev/.
+# The jail then creates the random device nodes in its /tmp/dev/.
+mkdir -p $CHROOT/dev
+mkdir -p $CHROOT/tmp/dev
+for file in random urandom
+do
+    ln -f ../tmp/dev/$file $CHROOT/dev/ 2> /dev/null || ln -f -s 
../tmp/dev/$file $CHROOT/dev/ || echo "Failed to link dev/$file"
+done
+
+# Create a relative symbolic link within systemplate that points from
+# the path of $INSTDIR (as seen from the jail as an absolute path)
+# to the /lo path, where the instdir of LO will really reside.
+mkdir -p $CHROOT/lo
+# In case the original path is different from
+for path in $INSTDIR $INSTDIR_LOGICAL
+do
+    INSTDIR_PARENT="$(dirname "$CHROOT/$path")"
+    mkdir -p $INSTDIR_PARENT
+    ln -f -s `realpath --relative-to=$INSTDIR_PARENT $CHROOT/lo` $CHROOT/$path
+done
+
 # /usr/share/fonts needs to be taken care of separately because the
 # directory time stamps must be preserved for fontconfig to trust
 # its cache.
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to