aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLudovic Courtès <ludo@gnu.org>2015-05-19 08:02:52 +0200
committerLudovic Courtès <ludo@gnu.org>2015-05-19 16:09:58 +0200
commita88b8c5c985a87586159c0621974a1dfe5b9b92d (patch)
treede07afac97f3f3061bd75a13e83576f48284c225
parente53fc0c8a33b1ea4f8503aca899da34ff9ebaa3c (diff)
downloadguix-a88b8c5c985a87586159c0621974a1dfe5b9b92d.tar
guix-a88b8c5c985a87586159c0621974a1dfe5b9b92d.tar.gz
Revert "daemon: Fix possible use-after-free."
This reverts commit 1303a4a4517260def862ce7fe97e6b28dd8005e1.
-rw-r--r--nix/libstore/build.cc29
-rw-r--r--nix/libutil/util.cc20
-rw-r--r--nix/libutil/util.hh5
3 files changed, 28 insertions, 26 deletions
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index b3c994d6de..f38cd29940 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -401,6 +401,18 @@ static void commonChildInit(Pipe & logPipe)
}
+/* Convert a string list to an array of char pointers. Careful: the
+ string list should outlive the array. */
+const char * * strings2CharPtrs(const Strings & ss)
+{
+ const char * * arr = new const char * [ss.size() + 1];
+ const char * * p = arr;
+ foreach (Strings::const_iterator, i, ss) *p++ = i->c_str();
+ *p = 0;
+ return arr;
+}
+
+
/* Restore default handling of SIGPIPE, otherwise some programs will
randomly say "Broken pipe". */
static void restoreSIGPIPE()
@@ -2123,7 +2135,11 @@ void DerivationGoal::initChild()
Strings envStrs;
foreach (Environment::const_iterator, i, env)
envStrs.push_back(rewriteHashes(i->first + "=" + i->second, rewritesToTmp));
- std::vector<const char *> envArr = stringsToCharPtrs(envStrs);
+ const char * * envArr = strings2CharPtrs(envStrs);
+
+ Path program = drv.builder.c_str();
+ std::vector<const char *> args; /* careful with c_str()! */
+ string user; /* must be here for its c_str()! */
/* If we are running in `build-users' mode, then switch to the
user we allocated above. Make sure that we drop all root
@@ -2149,18 +2165,17 @@ void DerivationGoal::initChild()
}
/* Fill in the arguments. */
- Strings args;
string builderBasename = baseNameOf(drv.builder);
args.push_back(builderBasename.c_str());
foreach (Strings::iterator, i, drv.args)
- args.push_back(rewriteHashes(*i, rewritesToTmp));
- std::vector<const char *> argArr = stringsToCharPtrs(args);
+ args.push_back(rewriteHashes(*i, rewritesToTmp).c_str());
+ args.push_back(0);
restoreSIGPIPE();
/* Execute the program. This should not return. */
inSetup = false;
- execve(drv.builder.c_str(), (char * *) &argArr[0], (char * *) &envArr[0]);
+ execve(program.c_str(), (char * *) &args[0], (char * *) envArr);
throw SysError(format("executing `%1%'") % drv.builder);
@@ -2763,7 +2778,7 @@ void SubstitutionGoal::tryToRun()
args.push_back("--substitute");
args.push_back(storePath);
args.push_back(destPath);
- std::vector<const char *> argArr = stringsToCharPtrs(args);
+ const char * * argArr = strings2CharPtrs(args);
/* Fork the substitute program. */
pid = maybeVfork();
@@ -2781,7 +2796,7 @@ void SubstitutionGoal::tryToRun()
if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1)
throw SysError("cannot dup output pipe into stdout");
- execv(sub.c_str(), (char * *) &argArr[0]);
+ execv(sub.c_str(), (char * *) argArr);
throw SysError(format("executing `%1%'") % sub);
diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc
index 024cea83d1..846674a29d 100644
--- a/nix/libutil/util.cc
+++ b/nix/libutil/util.cc
@@ -852,20 +852,16 @@ void killUser(uid_t uid)
//////////////////////////////////////////////////////////////////////
-std::vector<const char *> stringsToCharPtrs(const Strings & ss)
-{
- std::vector<const char *> res;
- foreach (Strings::const_iterator, i, ss)
- res.push_back(i->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();
@@ -884,10 +880,6 @@ 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/nix/libutil/util.hh b/nix/libutil/util.hh
index a70981877b..ce2d77c19a 100644
--- a/nix/libutil/util.hh
+++ b/nix/libutil/util.hh
@@ -257,11 +257,6 @@ void killUser(uid_t uid);
string runProgram(Path program, bool searchPath = false,
const Strings & args = Strings());
-/* 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);