From 01d07b1e92c298f729a73705907b2987da9a4d0c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 5 Feb 2019 10:49:19 +0100 Subject: [PATCH] Revert "Restore parent mount namespace before executing a child process" This reverts commit a0ef21262f4d5652bfb65cfacaec01d89c475a93. 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. --- src/libstore/build.cc | 2 -- src/libstore/local-store.cc | 2 -- src/libstore/ssh.cc | 3 --- src/libutil/util.cc | 24 ------------------------ src/libutil/util.hh | 10 ---------- src/nix-build/nix-build.cc | 4 ++-- src/nix/edit.cc | 5 ----- src/nix/repl.cc | 2 -- src/nix/run.cc | 4 ++-- 9 files changed, 4 insertions(+), 52 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 59abae9b9..47ee8b48f 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2193,7 +2193,6 @@ void DerivationGoal::startBuilder() userNamespaceSync.create(); options.allowVfork = false; - options.restoreMountNamespace = false; Pid helper = startProcess([&]() { @@ -2260,7 +2259,6 @@ void DerivationGoal::startBuilder() #endif { options.allowVfork = !buildUser && !drv->isBuiltin(); - options.restoreMountNamespace = false; pid = startProcess([&]() { runChild(); }, options); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 5b4e7ca4c..485fdd691 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -366,8 +366,6 @@ void LocalStore::makeStoreWritable() throw SysError("getting info about the Nix store mount point"); if (stat.f_flag & ST_RDONLY) { - saveMountNamespace(); - if (unshare(CLONE_NEWNS) == -1) throw SysError("setting up a private mount namespace"); diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index cf133b57c..5e0e44935 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -1,5 +1,4 @@ #include "ssh.hh" -#include "affinity.hh" namespace nix { @@ -35,9 +34,7 @@ std::unique_ptr SSHMaster::startCommand(const std::string auto conn = std::make_unique(); conn->sshPid = startProcess([&]() { - restoreAffinity(); restoreSignals(); - restoreMountNamespace(); close(in.writeSide.get()); close(out.readSide.get()); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index ce50334e1..7eca35577 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -936,8 +936,6 @@ pid_t startProcess(std::function fun, const ProcessOptions & options) throw SysError("setting death signal"); #endif restoreAffinity(); - if (options.restoreMountNamespace) - restoreMountNamespace(); fun(); } catch (std::exception & e) { try { @@ -1506,26 +1504,4 @@ std::unique_ptr createInterruptCallback(std::function return std::unique_ptr(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 -} - } diff --git a/src/libutil/util.hh b/src/libutil/util.hh index d67bddc13..bda87bee4 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -250,7 +250,6 @@ struct ProcessOptions bool dieWithParent = true; bool runExitHandlers = false; bool allowVfork = true; - bool restoreMountNamespace = true; }; pid_t startProcess(std::function fun, const ProcessOptions & options = ProcessOptions()); @@ -515,13 +514,4 @@ typedef std::function PathFilter; 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(); - - } diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 11ea3b1f7..618895d38 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -401,6 +401,8 @@ static void _main(int argc, char * * argv) } else env[var.first] = var.second; + restoreAffinity(); + /* Run a shell using the derivation's environment. For convenience, source $stdenv/setup to setup additional environment variables and shell functions. Also don't @@ -444,9 +446,7 @@ static void _main(int argc, char * * argv) auto argPtrs = stringsToCharPtrs(args); - restoreAffinity(); restoreSignals(); - restoreMountNamespace(); execvp(shell.c_str(), argPtrs.data()); diff --git a/src/nix/edit.cc b/src/nix/edit.cc index d8d5895bd..c9671f76d 100644 --- a/src/nix/edit.cc +++ b/src/nix/edit.cc @@ -3,7 +3,6 @@ #include "eval.hh" #include "attr-path.hh" #include "progress-bar.hh" -#include "affinity.hh" #include @@ -73,10 +72,6 @@ struct CmdEdit : InstallableCommand stopProgressBar(); - restoreAffinity(); - restoreSignals(); - restoreMountNamespace(); - execvp(args.front().c_str(), stringsToCharPtrs(args).data()); throw SysError("cannot run editor '%s'", editor); diff --git a/src/nix/repl.cc b/src/nix/repl.cc index dd3d9ed97..227affc60 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -337,8 +337,6 @@ static int runProgram(const string & program, const Strings & args) if (pid == -1) throw SysError("forking"); if (pid == 0) { restoreAffinity(); - restoreSignals(); - restoreMountNamespace(); execvp(program.c_str(), stringsToCharPtrs(args2).data()); _exit(1); } diff --git a/src/nix/run.cc b/src/nix/run.cc index 129707298..35b763345 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -153,9 +153,9 @@ struct CmdRun : InstallablesCommand stopProgressBar(); - restoreAffinity(); restoreSignals(); - restoreMountNamespace(); + + restoreAffinity(); /* If this is a diverted store (i.e. its "logical" location (typically /nix/store) differs from its "physical" location