3 files changed, 72 insertions, 39 deletions
diff --git a/src/libutil/types.hh b/src/libutil/types.hh
index 030996a060e2..160884ee1ad7 100644
--- a/src/libutil/types.hh
+++ b/src/libutil/types.hh
@@ -73,7 +73,6 @@ class SysError : public Error
 public:
     int errNo;
     SysError(const FormatOrString & fs);
-    SysError(int errNo, const FormatOrString & fs);
 };
 
 
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index 60be02cd4647..dcdb438e03b2 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -45,14 +45,8 @@ BaseError & BaseError::addPrefix(const FormatOrString & fs)
 
 
 SysError::SysError(const FormatOrString & fs)
-    : SysError(errno, fs)
-{
-}
-
-
-SysError::SysError(int errNo, const FormatOrString & fs)
-    : Error(format("%1%: %2%") % fs.s % strerror(errNo))
-    , errNo(errNo)
+    : Error(format("%1%: %2%") % fs.s % strerror(errno))
+    , errNo(errno)
 {
 }
 
@@ -271,7 +265,7 @@ void writeFile(const Path & path, const string & s)
     AutoCloseFD fd = open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT, 0666);
     if (fd == -1)
         throw SysError(format("opening file ‘%1%’") % path);
-    writeFull(fd, (unsigned char *) s.data(), s.size());
+    writeFull(fd, s);
 }
 
 
@@ -298,7 +292,7 @@ string readLine(int fd)
 void writeLine(int fd, string s)
 {
     s += '\n';
-    writeFull(fd, (const unsigned char *) s.data(), s.size());
+    writeFull(fd, s);
 }
 
 
@@ -489,18 +483,13 @@ void warnOnce(bool & haveWarned, const FormatOrString & fs)
 }
 
 
-static void defaultWriteToStderr(const unsigned char * buf, size_t count)
-{
-    writeFull(STDERR_FILENO, buf, count);
-}
-
-
 void writeToStderr(const string & s)
 {
     try {
-        auto p = _writeToStderr;
-        if (!p) p = defaultWriteToStderr;
-        p((const unsigned char *) s.data(), s.size());
+        if (_writeToStderr)
+            _writeToStderr((const unsigned char *) s.data(), s.size());
+        else
+            writeFull(STDERR_FILENO, s);
     } catch (SysError & e) {
         /* Ignore failing writes to stderr if we're in an exception
            handler, otherwise throw an exception.  We need to ignore
@@ -512,7 +501,7 @@ void writeToStderr(const string & s)
 }
 
 
-void (*_writeToStderr) (const unsigned char * buf, size_t count) = defaultWriteToStderr;
+void (*_writeToStderr) (const unsigned char * buf, size_t count) = 0;
 
 
 void readFull(int fd, unsigned char * buf, size_t count)
@@ -546,6 +535,12 @@ void writeFull(int fd, const unsigned char * buf, size_t count)
 }
 
 
+void writeFull(int fd, const string & s)
+{
+    writeFull(fd, (const unsigned char *) s.data(), s.size());
+}
+
+
 string drainFD(int fd)
 {
     string result;
@@ -831,6 +826,9 @@ void killUser(uid_t uid)
        users to which the current process can send signals.  So we
        fork a process, switch to uid, and send a mass kill. */
 
+    ProcessOptions options;
+    options.allowVfork = false;
+
     Pid pid = startProcess([&]() {
 
         if (setuid(uid) == -1)
@@ -853,7 +851,7 @@ void killUser(uid_t uid)
         }
 
         _exit(0);
-    });
+    }, options);
 
     int status = pid.wait(true);
     if (status != 0)
@@ -869,46 +867,64 @@ void killUser(uid_t uid)
 //////////////////////////////////////////////////////////////////////
 
 
-pid_t startProcess(std::function<void()> fun,
-    bool dieWithParent, const string & errorPrefix, bool runExitHandlers)
+/* Wrapper around vfork to prevent the child process from clobbering
+   the caller's stack frame in the parent. */
+static pid_t doFork(bool allowVfork, std::function<void()> fun) __attribute__((noinline));
+static pid_t doFork(bool allowVfork, std::function<void()> fun)
 {
+#ifdef __linux__
+    pid_t pid = allowVfork ? vfork() : fork();
+#else
     pid_t pid = fork();
-    if (pid == -1) throw SysError("unable to fork");
+#endif
+    if (pid != 0) return pid;
+    fun();
+    abort();
+}
 
-    if (pid == 0) {
-        _writeToStderr = 0;
+
+pid_t startProcess(std::function<void()> fun, const ProcessOptions & options)
+{
+    auto wrapper = [&]() {
+        if (!options.allowVfork) _writeToStderr = 0;
         try {
 #if __linux__
-            if (dieWithParent && prctl(PR_SET_PDEATHSIG, SIGKILL) == -1)
+            if (options.dieWithParent && prctl(PR_SET_PDEATHSIG, SIGKILL) == -1)
                 throw SysError("setting death signal");
 #endif
             restoreAffinity();
             fun();
         } catch (std::exception & e) {
             try {
-                std::cerr << errorPrefix << e.what() << "\n";
+                std::cerr << options.errorPrefix << e.what() << "\n";
             } catch (...) { }
         } catch (...) { }
-        if (runExitHandlers)
+        if (options.runExitHandlers)
             exit(1);
         else
             _exit(1);
-    }
+    };
+
+    pid_t pid = doFork(options.allowVfork, wrapper);
+    if (pid == -1) throw SysError("unable to fork");
 
     return pid;
 }
 
 
+std::vector<const char *> stringsToCharPtrs(const Strings & ss)
+{
+    std::vector<const char *> res;
+    for (auto & s : ss) res.push_back(s.c_str());
+    res.push_back(0);
+    return res;
+}
+
+
 string runProgram(Path program, bool searchPath, const Strings & args)
 {
     checkInterrupt();
 
-    std::vector<const char *> cargs; /* careful with c_str()! */
-    cargs.push_back(program.c_str());
-    for (Strings::const_iterator i = args.begin(); i != args.end(); ++i)
-        cargs.push_back(i->c_str());
-    cargs.push_back(0);
-
     /* Create a pipe. */
     Pipe pipe;
     pipe.create();
@@ -918,6 +934,10 @@ string runProgram(Path program, bool searchPath, const Strings & args)
         if (dup2(pipe.writeSide, STDOUT_FILENO) == -1)
             throw SysError("dupping stdout");
 
+        Strings args_(args);
+        args_.push_front(program);
+        auto cargs = stringsToCharPtrs(args_);
+
         if (searchPath)
             execvp(program.c_str(), (char * *) &cargs[0]);
         else
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 628b8a0e1f09..186ee71f45d0 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -171,6 +171,7 @@ extern void (*_writeToStderr) (const unsigned char * buf, size_t count);
    requested number of bytes. */
 void readFull(int fd, unsigned char * buf, size_t count);
 void writeFull(int fd, const unsigned char * buf, size_t count);
+void writeFull(int fd, const string & s);
 
 MakeError(EndOfFile, Error)
 
@@ -269,8 +270,16 @@ void killUser(uid_t uid);
 
 /* Fork a process that runs the given function, and return the child
    pid to the caller. */
-pid_t startProcess(std::function<void()> fun, bool dieWithParent = true,
-    const string & errorPrefix = "error: ", bool runExitHandlers = false);
+struct ProcessOptions
+{
+    string errorPrefix;
+    bool dieWithParent;
+    bool runExitHandlers;
+    bool allowVfork;
+    ProcessOptions() : errorPrefix("error: "), dieWithParent(true), runExitHandlers(false), allowVfork(true) { };
+};
+
+pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = ProcessOptions());
 
 
 /* Run a program and return its stdout in a string (i.e., like the
@@ -280,6 +289,11 @@ string runProgram(Path program, bool searchPath = false,
 
 MakeError(ExecError, Error)
 
+/* Convert a list of strings to a null-terminated vector of char
+   *'s. The result must not be accessed beyond the lifetime of the
+   list of strings. */
+std::vector<const char *> stringsToCharPtrs(const Strings & ss);
+
 /* Close all file descriptors except stdin, stdout, stderr, and those
    listed in the given set.  Good practice in child processes. */
 void closeMostFDs(const set<int> & exceptions);