Revert "Restore parent mount namespace before executing a child process"

This reverts commit a0ef21262f. This
doesn't work in 'nix run' and nix-shell because setns() fails in
multithreaded programs, and Boehm GC mark threads are uncancellable.

Fixes #2646.
This commit is contained in:
Eelco Dolstra 2019-02-05 10:49:19 +01:00
parent 92d08c02c8
commit 01d07b1e92
No known key found for this signature in database
GPG key ID: 8170B4726D7198DE
9 changed files with 4 additions and 52 deletions

View file

@ -2193,7 +2193,6 @@ void DerivationGoal::startBuilder()
userNamespaceSync.create(); userNamespaceSync.create();
options.allowVfork = false; options.allowVfork = false;
options.restoreMountNamespace = false;
Pid helper = startProcess([&]() { Pid helper = startProcess([&]() {
@ -2260,7 +2259,6 @@ void DerivationGoal::startBuilder()
#endif #endif
{ {
options.allowVfork = !buildUser && !drv->isBuiltin(); options.allowVfork = !buildUser && !drv->isBuiltin();
options.restoreMountNamespace = false;
pid = startProcess([&]() { pid = startProcess([&]() {
runChild(); runChild();
}, options); }, options);

View file

@ -366,8 +366,6 @@ void LocalStore::makeStoreWritable()
throw SysError("getting info about the Nix store mount point"); throw SysError("getting info about the Nix store mount point");
if (stat.f_flag & ST_RDONLY) { if (stat.f_flag & ST_RDONLY) {
saveMountNamespace();
if (unshare(CLONE_NEWNS) == -1) if (unshare(CLONE_NEWNS) == -1)
throw SysError("setting up a private mount namespace"); throw SysError("setting up a private mount namespace");

View file

@ -1,5 +1,4 @@
#include "ssh.hh" #include "ssh.hh"
#include "affinity.hh"
namespace nix { namespace nix {
@ -35,9 +34,7 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(const std::string
auto conn = std::make_unique<Connection>(); auto conn = std::make_unique<Connection>();
conn->sshPid = startProcess([&]() { conn->sshPid = startProcess([&]() {
restoreAffinity();
restoreSignals(); restoreSignals();
restoreMountNamespace();
close(in.writeSide.get()); close(in.writeSide.get());
close(out.readSide.get()); close(out.readSide.get());

View file

@ -936,8 +936,6 @@ pid_t startProcess(std::function<void()> fun, const ProcessOptions & options)
throw SysError("setting death signal"); throw SysError("setting death signal");
#endif #endif
restoreAffinity(); restoreAffinity();
if (options.restoreMountNamespace)
restoreMountNamespace();
fun(); fun();
} catch (std::exception & e) { } catch (std::exception & e) {
try { try {
@ -1506,26 +1504,4 @@ std::unique_ptr<InterruptCallback> createInterruptCallback(std::function<void()>
return std::unique_ptr<InterruptCallback>(res.release()); return std::unique_ptr<InterruptCallback>(res.release());
} }
static AutoCloseFD fdSavedMountNamespace;
void saveMountNamespace()
{
#if __linux__
std::once_flag done;
std::call_once(done, []() {
fdSavedMountNamespace = open("/proc/self/ns/mnt", O_RDONLY);
if (!fdSavedMountNamespace)
throw SysError("saving parent mount namespace");
});
#endif
}
void restoreMountNamespace()
{
#if __linux__
if (fdSavedMountNamespace && setns(fdSavedMountNamespace.get(), CLONE_NEWNS) == -1)
throw SysError("restoring parent mount namespace");
#endif
}
} }

View file

@ -250,7 +250,6 @@ struct ProcessOptions
bool dieWithParent = true; bool dieWithParent = true;
bool runExitHandlers = false; bool runExitHandlers = false;
bool allowVfork = true; bool allowVfork = true;
bool restoreMountNamespace = true;
}; };
pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = ProcessOptions()); pid_t startProcess(std::function<void()> fun, const ProcessOptions & options = ProcessOptions());
@ -515,13 +514,4 @@ typedef std::function<bool(const Path & path)> PathFilter;
extern PathFilter defaultPathFilter; extern PathFilter defaultPathFilter;
/* Save the current mount namespace. Ignored if called more than
once. */
void saveMountNamespace();
/* Restore the mount namespace saved by saveMountNamespace(). Ignored
if saveMountNamespace() was never called. */
void restoreMountNamespace();
} }

View file

@ -401,6 +401,8 @@ static void _main(int argc, char * * argv)
} else } else
env[var.first] = var.second; env[var.first] = var.second;
restoreAffinity();
/* Run a shell using the derivation's environment. For /* Run a shell using the derivation's environment. For
convenience, source $stdenv/setup to setup additional convenience, source $stdenv/setup to setup additional
environment variables and shell functions. Also don't environment variables and shell functions. Also don't
@ -444,9 +446,7 @@ static void _main(int argc, char * * argv)
auto argPtrs = stringsToCharPtrs(args); auto argPtrs = stringsToCharPtrs(args);
restoreAffinity();
restoreSignals(); restoreSignals();
restoreMountNamespace();
execvp(shell.c_str(), argPtrs.data()); execvp(shell.c_str(), argPtrs.data());

View file

@ -3,7 +3,6 @@
#include "eval.hh" #include "eval.hh"
#include "attr-path.hh" #include "attr-path.hh"
#include "progress-bar.hh" #include "progress-bar.hh"
#include "affinity.hh"
#include <unistd.h> #include <unistd.h>
@ -73,10 +72,6 @@ struct CmdEdit : InstallableCommand
stopProgressBar(); stopProgressBar();
restoreAffinity();
restoreSignals();
restoreMountNamespace();
execvp(args.front().c_str(), stringsToCharPtrs(args).data()); execvp(args.front().c_str(), stringsToCharPtrs(args).data());
throw SysError("cannot run editor '%s'", editor); throw SysError("cannot run editor '%s'", editor);

View file

@ -337,8 +337,6 @@ static int runProgram(const string & program, const Strings & args)
if (pid == -1) throw SysError("forking"); if (pid == -1) throw SysError("forking");
if (pid == 0) { if (pid == 0) {
restoreAffinity(); restoreAffinity();
restoreSignals();
restoreMountNamespace();
execvp(program.c_str(), stringsToCharPtrs(args2).data()); execvp(program.c_str(), stringsToCharPtrs(args2).data());
_exit(1); _exit(1);
} }

View file

@ -153,9 +153,9 @@ struct CmdRun : InstallablesCommand
stopProgressBar(); stopProgressBar();
restoreAffinity();
restoreSignals(); restoreSignals();
restoreMountNamespace();
restoreAffinity();
/* If this is a diverted store (i.e. its "logical" location /* If this is a diverted store (i.e. its "logical" location
(typically /nix/store) differs from its "physical" location (typically /nix/store) differs from its "physical" location