Glibc hook: fixed detection of the container's glibc version

CHANGELOG.md

@@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
+## [Unreleased]
+
+### Fixed
+
+- Glibc hook: fixed detection of the container's glibc version, which was causing a shell-init error on some systems
+
+
 ## [1.6.3]
 
 ### Changed

src/common/Utility.cpp

@@ -213,6 +213,18 @@ void logProcessUserAndGroupIdentifiers() {
                % rgid % egid % sgid % setfsgid(-1), common::LogLevel::DEBUG);
 }
 
+static void readCStream(FILE* const in, std::iostream* const out) {
+    char buffer[1024];
+    while(!feof(in)) {
+        if(fgets(buffer, sizeof(buffer), in)) {
+            *out << buffer;
+        }
+        else if(!feof(in)) {
+            SARUS_THROW_ERROR("Failed to read C stream: call to fgets() failed.");
+        }
+    }
+}
+
 std::string executeCommand(const std::string& command) {
     auto commandWithRedirection = command + " 2>&1"; // stderr-to-stdout redirection necessary because popen only reads stdout
     logMessage(boost::format("Executing command '%s'") % commandWithRedirection, common::LogLevel::DEBUG);
@@ -224,17 +236,12 @@ std::string executeCommand(const std::string& command) {
         SARUS_THROW_ERROR(message.str());
     }
 
-    char buffer[1024];
-    std::string commandOutput;
-    while(!feof(pipe)) {
-        if(fgets(buffer, sizeof(buffer), pipe)) {
-            commandOutput += buffer;
-        }
-        else if(!feof(pipe)) {
-            auto message = boost::format("Failed to execute command \"%s\". Call to fgets() failed.")
-                % commandWithRedirection;
-            SARUS_THROW_ERROR(message.str());
-        }
+    std::stringstream commandOutput;
+    try {
+        readCStream(pipe, &commandOutput);
+    } catch(const common::Error& e) {
+        auto message = boost::format("Failed to read stdout from command \"%s\"") % commandWithRedirection;
+        SARUS_RETHROW_ERROR(e, message.str());
     }
 
     auto status = pclose(pipe);
@@ -246,24 +253,35 @@ std::string executeCommand(const std::string& command) {
     else if(!WIFEXITED(status)) {
         auto message = boost::format(   "Failed to execute command \"%s\"."
                                         " Process terminated abnormally. Process' output:\n\n%s")
-                                        % commandWithRedirection % commandOutput;
+                                        % commandWithRedirection % commandOutput.str();
         SARUS_THROW_ERROR(message.str());
     }
     else if(WEXITSTATUS(status) != 0) {
         auto message = boost::format(   "Failed to execute command \"%s\"."
                                         " Process terminated with status %d. Process' output:\n\n%s")
-                                        % commandWithRedirection % WEXITSTATUS(status) % commandOutput;
+                                        % commandWithRedirection % WEXITSTATUS(status) % commandOutput.str();
         SARUS_THROW_ERROR(message.str());
     }
 
-    return commandOutput;
+    return commandOutput.str();
 }
 
 int forkExecWait(const common::CLIArguments& args,
                  const boost::optional<std::function<void()>>& preExecChildActions,
-                 const boost::optional<std::function<void(int)>>& postForkParentActions) {
+                 const boost::optional<std::function<void(int)>>& postForkParentActions,
+                 std::iostream* const childStdoutStream) {
     logMessage(boost::format("Forking and executing '%s'") % args, common::LogLevel::DEBUG);
 
+
+    int pipefd[2];
+    if(childStdoutStream) {
+        if(pipe(pipefd) == -1) {
+            auto message = boost::format("Failed to open pipe to execute subprocess %s: %s")
+                % args % strerror(errno);
+            SARUS_THROW_ERROR(message.str());
+        }
+    }
+
     // fork and execute
     auto pid = fork();
     if(pid == -1) {
@@ -274,6 +292,12 @@ int forkExecWait(const common::CLIArguments& args,
 
     bool isChild = pid == 0;
     if(isChild) {
+        if(childStdoutStream) {
+            // Redirect stdout to write to the pipe, then we don't need the pipe ends anymore
+            dup2(pipefd[1], STDOUT_FILENO);
+            close(pipefd[0]);
+            close(pipefd[1]);
+        }
         if(preExecChildActions) {
             (*preExecChildActions)();
         }
@@ -285,6 +309,18 @@ int forkExecWait(const common::CLIArguments& args,
         if(postForkParentActions) {
             (*postForkParentActions)(pid);
         }
+        if(childStdoutStream) {
+            // Close the write end of the pipe, as it won't be used
+            close(pipefd[1]);
+
+            FILE *childStdoutPipe = fdopen(pipefd[0], "r");
+            try {
+                readCStream(childStdoutPipe, childStdoutStream);
+            } catch(const common::Error& e) {
+                auto message = boost::format("Failed to read stdout from subprocess %s") % args;
+                SARUS_RETHROW_ERROR(e, message.str());
+            }
+        }
         int status;
         do {
             if(waitpid(pid, &status, 0) == -1) {
@@ -309,7 +345,7 @@ int forkExecWait(const common::CLIArguments& args,
 
 void redirectStdoutToFile(const boost::filesystem::path& path) {
     int fd = open(path.c_str(), O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
-    dup2(fd, 1);
+    dup2(fd, STDOUT_FILENO);
     close(fd);
 }
 

src/common/Utility.hpp

@@ -50,7 +50,8 @@ void logProcessUserAndGroupIdentifiers();
 std::string executeCommand(const std::string& command);
 int forkExecWait(const common::CLIArguments& args,
                  const boost::optional<std::function<void()>>& preExecChildActions = {},
-                 const boost::optional<std::function<void(int)>>& postForkParentActions = {});
+                 const boost::optional<std::function<void(int)>>& postForkParentActions = {},
+                 std::iostream* const childStdoutStream= nullptr);
 void redirectStdoutToFile(const boost::filesystem::path& file);
 void SetStdinEcho(bool);
 std::string getHostname();

src/hooks/glibc/GlibcHook.cpp

@@ -157,7 +157,7 @@ bool GlibcHook::containerGlibcHasToBeReplaced() const {
 }
 
 /*
- * Use the output of "ldd --version" to obtain information about the glibc version from the host.
+ * Use the output of "ldd --version" to obtain information about the glibc version.
  * Obtaining the glibc version through the glibc.so filename is not always viable since some Linux distributions
  * (e.g. Ubuntu 21.10, Fedora 35) package the library without the version in the filename.
  * Likewise, obtaining the version from executing the glibc shared object is not reliable because some distributions
@@ -166,53 +166,42 @@ bool GlibcHook::containerGlibcHasToBeReplaced() const {
  * that would require glibc headers to be available in the container, which cannot be guaranteed, e.g. in the case
  * of a slim image.
  */
-std::tuple<unsigned int, unsigned int> GlibcHook::detectHostLibcVersion() const {
-    auto lddOutput = std::string();
-    try {
-        lddOutput = sarus::common::executeCommand(lddPath.string() + " --version");
-    }
-    catch (sarus::common::Error& e) {
-        SARUS_RETHROW_ERROR(e, "Failed to detect host glibc version.");
+static std::tuple<unsigned int, unsigned int> detectLibcVersion(
+    const boost::filesystem::path& lddPath,
+    const boost::optional<std::function<void()>> preExecActions,
+    const std::string& context) {
+    auto lddOutput = std::stringstream();
+    auto lddCommand = sarus::common::CLIArguments{lddPath.string(), "--version"};
+    auto status = sarus::common::forkExecWait(lddCommand, preExecActions, {}, &lddOutput);
+    if(status != 0) {
+        auto message = boost::format("Failed to detect %s glibc version. Command %s exited with status %d")
+            % context % lddCommand % status;
+        SARUS_THROW_ERROR(message.str());
     }
+    return hooks::common::utility::parseLibcVersionFromLddOutput(lddOutput.str());
+}
 
-    return hooks::common::utility::parseLibcVersionFromLddOutput(lddOutput);
+std::tuple<unsigned int, unsigned int> GlibcHook::detectHostLibcVersion() const {
+    return detectLibcVersion(lddPath, {}, "host");
 }
 
 /*
- * Use the output of "ldd --version" to obtain information about the glibc version from the container.
- * The same considerations made for detectHostLibcVersion() apply here.
- * Because the Glibc hook runs with root privileges, this function uses the forkExecWait() utility function to
- * drop all privileges and switch to the user identity before executing a binary from the container.
- * A file is used to store information about the ldd output, since forkExecWait() does not capture stdout.
+ * Obtain information about the glibc version from the container.
+ * Because the Glibc hook runs with root privileges, this function uses the forkExecWait() utility function
+ * to change its root directory, drop all privileges, and switch to the user identity before executing
+ * the ldd binary from the container.
  */
 std::tuple<unsigned int, unsigned int> GlibcHook::detectContainerLibcVersion() const {
-    auto glibcOutput = std::string();
-    auto lddOutputPath = sarus::common::makeUniquePathWithRandomSuffix(boost::filesystem::path{"/tmp/glibc-hook-ldd-out"});
-
-    std::function<void()> preExecActions = [this, &lddOutputPath]() {
+    std::function<void()> preExecActions = [this]() {
         if(chroot(rootfsDir.c_str()) != 0) {
             auto message = boost::format("Failed to chroot to %s: %s")
                 % rootfsDir % strerror(errno);
             SARUS_THROW_ERROR(message.str());
         }
-
         hooks::common::utility::switchToUnprivilegedProcess(userIdentity.uid, userIdentity.gid);
-        sarus::common::createFileIfNecessary(lddOutputPath);
-        sarus::common::redirectStdoutToFile(lddOutputPath);
+        sarus::common::changeDirectory("/");
     };
-
-    auto lddCommand = sarus::common::CLIArguments{"/usr/bin/ldd", "--version"};
-    auto status = sarus::common::forkExecWait(lddCommand, preExecActions);
-    if(status != 0) {
-        auto message = boost::format("Failed to detect container glibc version. /usr/bin/ldd exited with status %d")
-            % status;
-        SARUS_THROW_ERROR(message.str());
-    }
-
-    auto lddOutput = sarus::common::readFile(rootfsDir / lddOutputPath);
-    boost::filesystem::remove(rootfsDir / lddOutputPath);
-
-    return hooks::common::utility::parseLibcVersionFromLddOutput(lddOutput);
+    return detectLibcVersion("/usr/bin/ldd", preExecActions, "container");
 }
 
 void GlibcHook::verifyThatHostAndContainerGlibcAreABICompatible(