Skip to content

Commit

Permalink
Restore parent mount namespace before executing a child process
Browse files Browse the repository at this point in the history
This ensures that they can't write to /nix/store. Fixes #2535.
  • Loading branch information
edolstra committed Nov 13, 2018
1 parent 56f6e38 commit a0ef212
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 4 deletions.
2 changes: 2 additions & 0 deletions src/libstore/local-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,8 @@ 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");

Expand Down
3 changes: 3 additions & 0 deletions src/libstore/ssh.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "ssh.hh"
#include "affinity.hh"

namespace nix {

Expand Down Expand Up @@ -34,7 +35,9 @@ 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());
Expand Down
23 changes: 23 additions & 0 deletions src/libutil/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,7 @@ pid_t startProcess(std::function<void()> fun, const ProcessOptions & options)
throw SysError("setting death signal");
#endif
restoreAffinity();
restoreMountNamespace();
fun();
} catch (std::exception & e) {
try {
Expand Down Expand Up @@ -1504,4 +1505,26 @@ 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
}

}
9 changes: 9 additions & 0 deletions src/libutil/util.hh
Original file line number Diff line number Diff line change
Expand Up @@ -514,4 +514,13 @@ 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();


}
4 changes: 2 additions & 2 deletions src/nix-build/nix-build.cc
Original file line number Diff line number Diff line change
Expand Up @@ -401,8 +401,6 @@ 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
Expand Down Expand Up @@ -446,7 +444,9 @@ static void _main(int argc, char * * argv)

auto argPtrs = stringsToCharPtrs(args);

restoreAffinity();
restoreSignals();
restoreMountNamespace();

execvp(shell.c_str(), argPtrs.data());

Expand Down
5 changes: 5 additions & 0 deletions src/nix/edit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "eval.hh"
#include "attr-path.hh"
#include "progress-bar.hh"
#include "affinity.hh"

#include <unistd.h>

Expand Down Expand Up @@ -72,6 +73,10 @@ struct CmdEdit : InstallableCommand

stopProgressBar();

restoreAffinity();
restoreSignals();
restoreMountNamespace();

execvp(args.front().c_str(), stringsToCharPtrs(args).data());

throw SysError("cannot run editor '%s'", editor);
Expand Down
2 changes: 2 additions & 0 deletions src/nix/repl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,8 @@ 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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/nix/run.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ struct CmdRun : InstallablesCommand

stopProgressBar();

restoreSignals();

restoreAffinity();
restoreSignals();
restoreMountNamespace();

/* If this is a diverted store (i.e. its "logical" location
(typically /nix/store) differs from its "physical" location
Expand Down

4 comments on commit a0ef212

@dtzWill
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't had a chance to properly chase this down (including isolating to be sure re:blame), but ended up needing to revert this in order for builds to work at all on various NixOS 18.09 machines.

I'll try to dig up the exact error but as I recall the problem was an error putting together the chroot directory -- my initial guess would be because /nix/store is now read-only while we're trying to create the chroot skeleton.
Sorry for not having a better report, but wanted to mention it so it's not entirely blocked on me and possibly be enough to save others some pain. Assuming it's not just me O:).

@edolstra
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm yeah, I see a build failure in https://hydra.nixos.org/build/84073518.

@edolstra
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

@dtzWill
Copy link
Member

@dtzWill dtzWill commented on a0ef212 Nov 15, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.