Merge pull request #326 from dciabrin/tracefd-cloexec

Issue #325: bash-engine: new option to not leak trace fd to children

src/CMakeLists.txt

@@ -301,6 +301,10 @@ add_library (bash_execve_redirector SHARED engines/bash-execve-redirector.c)
 set_target_properties(bash_execve_redirector PROPERTIES SUFFIX ".so")
 target_link_libraries(bash_execve_redirector ${DL_LIBRARY})
 
+add_library (bash_tracefd_cloexec SHARED engines/bash-tracefd-cloexec.c)
+set_target_properties(bash_tracefd_cloexec PROPERTIES SUFFIX ".so")
+target_link_libraries(bash_tracefd_cloexec ${DL_LIBRARY})
+
 add_library (kcov_system_lib SHARED engines/system-mode-binary-lib.cc utils.cc system-mode/registration.cc)
 set_target_properties(kcov_system_lib PROPERTIES SUFFIX ".so")
 target_link_libraries(kcov_system_lib
@@ -330,6 +334,17 @@ add_custom_command(
  "${CMAKE_CURRENT_SOURCE_DIR}/bin-to-c-source.py"
 )
 
+add_custom_command(
+ OUTPUT bash-cloexec-library.cc
+ COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/bin-to-c-source.py"
+ $<TARGET_FILE:bash_tracefd_cloexec>
+ bash_cloexec_library
+ > bash-cloexec-library.cc
+ DEPENDS
+ bash_tracefd_cloexec
+ "${CMAKE_CURRENT_SOURCE_DIR}/bin-to-c-source.py"
+)
+
 add_custom_command(
  OUTPUT kcov-system-library.cc
  COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/bin-to-c-source.py"
@@ -409,7 +424,7 @@ if (SPECIFY_RPATH)
 endif (SPECIFY_RPATH)
 
 if (LIBELF_FOUND)
- add_executable (${KCOV} ${${KCOV}_SRCS} ${SOLIB_generated} bash-redirector-library.cc python-helper.cc bash-helper.cc kcov-system-library.cc html-data-files.cc version.c)
+ add_executable (${KCOV} ${${KCOV}_SRCS} ${SOLIB_generated} bash-redirector-library.cc bash-cloexec-library.cc python-helper.cc bash-helper.cc kcov-system-library.cc html-data-files.cc version.c)
 
  target_link_libraries(${KCOV}
  stdc++

src/configuration.cc

@@ -137,6 +137,7 @@ class Configuration : public IConfiguration
  { "debug", required_argument, 0, 'D' },
  { "debug-force-bash-stderr", no_argument, 0, 'd' },
  { "bash-handle-sh-invocation", no_argument, 0, 's' },
+ { "bash-tracefd-cloexec", no_argument, 0, 't' },
  { "bash-dont-parse-binary-dir", no_argument, 0, 'j' },
  { "bash-parse-files-in-dirs", required_argument, 0, 'J' },
  { "replace-src-path", required_argument, 0, 'R' },
@@ -376,6 +377,9 @@ class Configuration : public IConfiguration
  case 's':
  setKey("bash-handle-sh-invocation", 1);
  break;
+ case 't':
+ setKey("bash-tracefd-cloexec", 1);
+ break;
  case '4':
  {
  std::string s(optarg);
@@ -598,6 +602,7 @@ class Configuration : public IConfiguration
  setKey("include-path", StrVecMap_t());
  setKey("bash-force-stderr-input", 0);
  setKey("bash-handle-sh-invocation", 0);
+ setKey("bash-tracefd-cloexec", 0);
  setKey("bash-use-basic-parser", 0);
  setKey("bash-use-ps4", 1);
  setKey("bash-parse-file-dir", StrVecMap_t());
@@ -726,6 +731,8 @@ class Configuration : public IConfiguration
  " --bash-method=method Bash coverage collection method, PS4 (default) or DEBUG\n"
  " --bash-handle-sh-invocation Try to handle #!/bin/sh scripts by a LD_PRELOAD\n"
  " execve replacement. Buggy on some systems\n"
+ " --bash-tracefd-cloexec Force children to not be traced by configuring the trace\n"
+ " fd as non-cloneable with LD_PRELOAD. Buggy on some systems\n"
  " --bash-dont-parse-binary-dir Don't parse the binary directory for other scripts\n"
  " --bash-parse-files-in-dir=dir[,...] Parse bash scripts in dir(s)\n",
  keyAsInt("path-strip-level"), keyAsInt("output-interval"),

src/engines/bash-engine.cc

@@ -32,6 +32,7 @@ using namespace kcov;
 extern GeneratedData bash_helper_data;
 extern GeneratedData bash_helper_debug_trap_data;
 extern GeneratedData bash_redirector_library_data;
+extern GeneratedData bash_cloexec_library_data;
 
 enum InputType
 {
@@ -82,6 +83,7 @@ class BashEngine : public ScriptEngineBase
  std::string helperPath = IOutputHandler::getInstance().getBaseDirectory() + "bash-helper.sh";
  std::string helperDebugTrapPath = IOutputHandler::getInstance().getBaseDirectory() + "bash-helper-debug-trap.sh";
  std::string redirectorPath = IOutputHandler::getInstance().getBaseDirectory() + "libbash_execve_redirector.so";
+ std::string cloexecPath = IOutputHandler::getInstance().getBaseDirectory() + "libbash_tracefd_cloexec.so";
 
  if (write_file(bash_helper_data.data(), bash_helper_data.size(), "%s", helperPath.c_str()) < 0)
  {
@@ -104,6 +106,14 @@ class BashEngine : public ScriptEngineBase
  return false;
  }
 
+ if (write_file(bash_cloexec_library_data.data(), bash_cloexec_library_data.size(), "%s",
+ cloexecPath.c_str()) < 0)
+ {
+ error("Can't write tracefd-cloexec library at %s", cloexecPath.c_str());
+
+ return false;
+ }
+
  m_listener = &listener;
 
  /* Launch child */
@@ -197,8 +207,17 @@ class BashEngine : public ScriptEngineBase
  }
 
  // And preload it!
- if (conf.keyAsInt("bash-handle-sh-invocation"))
- doSetenv(std::string("LD_PRELOAD=" + redirectorPath));
+ if (conf.keyAsInt("bash-handle-sh-invocation") ||
+ conf.keyAsInt("bash-tracefd-cloexec"))
+ {
+ std::string preloadStr;
+ if (conf.keyAsInt("bash-handle-sh-invocation"))
+ preloadStr += redirectorPath;
+ if (conf.keyAsInt("bash-tracefd-cloexec"))
+ preloadStr += (preloadStr.size() ? ":" : "") + cloexecPath;
+
+ doSetenv(std::string("LD_PRELOAD=" + preloadStr));
+ }
 
  // Make a copy of the vector, now with "bash -x" first
  char **vec;

src/engines/bash-tracefd-cloexec.c

@@ -0,0 +1,28 @@
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+
+void __attribute__((constructor)) tracefd_cloexec(void)
+{
+ int xtraceFd;
+ const char* fdStr;
+ char* endStr;
+
+ if (getenv("KCOV_BASH_USE_DEBUG_TRAP"))
+ fdStr = getenv("KCOV_BASH_XTRACEFD");
+ else
+ fdStr = getenv("BASH_XTRACEFD");
+
+ if (fdStr)
+ {
+ xtraceFd = (int)strtol(fdStr, &endStr, 10);
+ if (endStr != NULL && *endStr == '\0')
+ {
+ int flags = fcntl(xtraceFd, F_GETFD);
+ if (flags != -1)
+ fcntl(xtraceFd, F_SETFD, flags | FD_CLOEXEC);
+ }
+ }
+}

tests/bash/background-child.sh

@@ -0,0 +1,4 @@
+#! /usr/bin/env bash
+
+nohup sleep 6 &>/dev/null &
+echo leave child running

tests/tools/bash_linux_only.py

@@ -10,3 +10,26 @@ def runTest(self):
 
  dom = parse_cobertura.parseFile(testbase.outbase + "/kcov/shell-main/cobertura.xml")
  assert parse_cobertura.hitsPerLine(dom, "sh-shebang.sh", 4) == 1
+
+class bash_exit_before_child(testbase.KcovTestCase):
+ def runTest(self):
+ # kcovKcov shouldn't wait for the background process, so call it with kcovKcov = False
+ rv,o = self.do(testbase.kcov + " --bash-tracefd-cloexec " + testbase.outbase + "/kcov " +
+ testbase.sources + "/tests/bash/background-child.sh",
+ kcovKcov = False,
+ timeout = 3.0,
+ kill = True)
+ self.assertEqual(0, rv, "kcov exited unsuccessfully")
+ dom = parse_cobertura.parseFile(testbase.outbase + "/kcov/background-child.sh/cobertura.xml")
+ self.assertIsNone(parse_cobertura.hitsPerLine(dom, "background-child.sh", 1))
+ self.assertEqual(1, parse_cobertura.hitsPerLine(dom, "background-child.sh", 3))
+ self.assertEqual(1, parse_cobertura.hitsPerLine(dom, "background-child.sh", 4))
+
+class bash_ldpreload_multilib(testbase.KcovTestCase):
+ def runTest(self):
+ rv,o = self.do(testbase.kcov + " --bash-handle-sh-invocation --bash-tracefd-cloexec " + testbase.outbase + "/kcov " +
+ testbase.sources + "/tests/bash/sh-shebang.sh")
+ self.assertEqual(0, rv, "kcov exited unsuccessfully")
+ dom = parse_cobertura.parseFile(testbase.outbase + "/kcov/sh-shebang.sh/cobertura.xml")
+ self.assertIsNone(parse_cobertura.hitsPerLine(dom, "sh-shebang.sh", 1))
+ self.assertEqual(1, parse_cobertura.hitsPerLine(dom, "sh-shebang.sh", 4))

tests/tools/testbase.py

@@ -35,7 +35,7 @@ def doShell(self, cmdline):
 
  return rv, output
 
- def do(self, cmdline, kcovKcov = True, timeout = None):
+ def do(self, cmdline, kcovKcov = True, timeout = None, kill = False):
  output = ""
  rv = 0
 
@@ -51,7 +51,10 @@ def do(self, cmdline, kcovKcov = True, timeout = None):
  if timeout is not None:
  def stopChild():
  print("\n didn't finish within %s seconds; killing ..." % timeout)
- child.terminate()
+ if kill:
+ child.kill()
+ else:
+ child.terminate()
 
  timer = threading.Timer(timeout, stopChild)
  timer.start()