diff --git a/CHANGELOG.md b/CHANGELOG.md index 26543545..f38fa05a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/common/Utility.cpp b/src/common/Utility.cpp index f77caf12..35bc457c 100644 --- a/src/common/Utility.cpp +++ b/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>& preExecChildActions, - const boost::optional>& postForkParentActions) { + const boost::optional>& 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); } diff --git a/src/common/Utility.hpp b/src/common/Utility.hpp index 2b8000fb..ab2a947f 100644 --- a/src/common/Utility.hpp +++ b/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>& preExecChildActions = {}, - const boost::optional>& postForkParentActions = {}); + const boost::optional>& postForkParentActions = {}, + std::iostream* const childStdoutStream= nullptr); void redirectStdoutToFile(const boost::filesystem::path& file); void SetStdinEcho(bool); std::string getHostname(); diff --git a/src/hooks/glibc/GlibcHook.cpp b/src/hooks/glibc/GlibcHook.cpp index 9a038bcf..356ceb14 100644 --- a/src/hooks/glibc/GlibcHook.cpp +++ b/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 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 detectLibcVersion( + const boost::filesystem::path& lddPath, + const boost::optional> 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 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 GlibcHook::detectContainerLibcVersion() const { - auto glibcOutput = std::string(); - auto lddOutputPath = sarus::common::makeUniquePathWithRandomSuffix(boost::filesystem::path{"/tmp/glibc-hook-ldd-out"}); - - std::function preExecActions = [this, &lddOutputPath]() { + std::function 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(