about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEelco Dolstra <edolstra@gmail.com>2019-02-05T09·49+0100
committerEelco Dolstra <edolstra@gmail.com>2019-02-05T09·49+0100
commit01d07b1e92c298f729a73705907b2987da9a4d0c (patch)
treea58d0e6ad9d7b1586346cb1f155d9b79aa15bb4d
parent92d08c02c84be34ec0df56ed718526c382845d1a (diff)
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.
-rw-r--r--src/libstore/build.cc2
-rw-r--r--src/libstore/local-store.cc2
-rw-r--r--src/libstore/ssh.cc3
-rw-r--r--src/libutil/util.cc24
-rw-r--r--src/libutil/util.hh10
-rwxr-xr-xsrc/nix-build/nix-build.cc4
-rw-r--r--src/nix/edit.cc5
-rw-r--r--src/nix/repl.cc2
-rw-r--r--src/nix/run.cc4
9 files changed, 4 insertions, 52 deletions
diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 59abae9b90db..47ee8b48f4b4 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 5b4e7ca4ca99..485fdd691932 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 cf133b57cb20..5e0e44935cca 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::Connection> SSHMaster::startCommand(const std::string
 
     auto conn = std::make_unique<Connection>();
     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 ce50334e1e62..7eca35577b01 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -936,8 +936,6 @@ pid_t startProcess(std::function<void()> 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<InterruptCallback> createInterruptCallback(std::function<void()>
     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
-}
-
 }
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index d67bddc138c8..bda87bee433e 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<void()> fun, const ProcessOptions & options = ProcessOptions());
@@ -515,13 +514,4 @@ typedef std::function<bool(const Path & path)> 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 11ea3b1f7ae1..618895d387d4 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 d8d5895bd867..c9671f76d0fa 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 <unistd.h>
 
@@ -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 dd3d9ed97495..227affc60e20 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 1297072989b9..35b763345872 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