nix-shell as root cd's to /

[dermetfan]

Describe the bug

Running nix-shell or nix develop or nix shell as root drops you into / instead of staying in the current directory.

Steps To Reproduce

nix shell example

sudo nix shell nixpkgs#hello

nix-shell example

# shell.nix
with import <nixpkgs> {};
mkShell {}

sudo nix-shell

nix develop example

# flake.nix
{
  outputs = { nixpkgs, ... }: {
    devShell.x86_64-linux = nixpkgs.legacyPackages.x86_64-linux.mkShell {};
  };
}

sudo nix develop

Expected behavior

Drops you into a shell in the current directory.

Additional context

nix (Nix) 2.7.0pre20220210_5b809f9

First noticed on 52f5231.

We worked around it for now using env -C "$PWD".

[disassembler]

nix run also executes in context of / so relative paths passed as arguments no longer works.

[cole-h]

I did a small bit of testing, and found that it was introduced between 2.4 and 2.5.0. I then bisected it to fcb8af5 (by testing nix build && doas result/bin/nix-shell -p):

git bisect log

git bisect start
# good: [9d5c985aedcf7d11a5310eae6f27dcde8eb7f608] Set 2.4 release date
git bisect good 9d5c985aedcf7d11a5310eae6f27dcde8eb7f608
# bad: [a646dfcdd642df404ef1e172abdcddb3aa179d48] Set version
git bisect bad a646dfcdd642df404ef1e172abdcddb3aa179d48
# good: [5a0c8c6712ee31d85ac77b94707d3510f9383efa] Merge pull request #5348 from edolstra/chroot-addpath
git bisect good 5a0c8c6712ee31d85ac77b94707d3510f9383efa
# bad: [51ffc19f02e78d7bea31c2916bc18798183f9ca1] Merge branch 'add-docker-image-to-hydra-jobs' of https://github.com/garbas/nix
git bisect bad 51ffc19f02e78d7bea31c2916bc18798183f9ca1
# good: [e6795c43504b763a03b21dccd6814b8703af357e] Style
git bisect good e6795c43504b763a03b21dccd6814b8703af357e
# bad: [7a71621b7c43d7d2f264cc495fb7ceb66455fd3c] Merge branch 'fix-writable-shell' of https://github.com/yorickvP/nix
git bisect bad 7a71621b7c43d7d2f264cc495fb7ceb66455fd3c
# good: [7d6017b7a911dd5b8a7ccb5bc1bf9282140de20c] Merge pull request #5493 from jtojnar/patch-1
git bisect good 7d6017b7a911dd5b8a7ccb5bc1bf9282140de20c
# good: [b5cb31e032d91ceda9fbbfb7a5b3e68a05a0ec14] Merge pull request #5514 from andir/let-body-unused
git bisect good b5cb31e032d91ceda9fbbfb7a5b3e68a05a0ec14
# good: [736d6ab7214d12e4837424b301a803acae02c273] Merge pull request #5504 from NixOS/flake-options-and-daemon
git bisect good 736d6ab7214d12e4837424b301a803acae02c273
# good: [d9c9d0e0eba9313eb0361223605fdacdf8e0689a] Merge pull request #5500 from abathur/fix_darwin_existing_mounted_volume
git bisect good d9c9d0e0eba9313eb0361223605fdacdf8e0689a
# good: [6c2af1f201a925c2aa632737765685c72b642847] Merge pull request #5434 from timothyklim/git-url-submodules
git bisect good 6c2af1f201a925c2aa632737765685c72b642847
# bad: [fcb8af550f5fca37458da0d9042a2b59523eb304] Restore parent mount namespace in restoreProcessContext
git bisect bad fcb8af550f5fca37458da0d9042a2b59523eb304
# first bad commit: [fcb8af550f5fca37458da0d9042a2b59523eb304] Restore parent mount namespace in restoreProcessContext

cc @yorickvP -- do you have any idea how to fix this?

[aszlig]

Since I'm not going to have time to continue on this during the next few days: The fix is to chdir back to the directory before saveMountNamespace after setns in restoreMountNamespace.

Here is my WIP commit message for anyone who wants to work on this:

libutil: Fix restoring mount namespace

I regularly pass around simple scripts by using nix-shell as the script
interpreter, eg. like this:

  #!/usr/bin/env nix-shell
  #!nix-shell -p dd_rescue coreutils bash -i bash

While this works most of the time, I recently had one occasion where it
would not and the above would result in the following:

  $ sudo ./myscript.sh
  bash: ./myscript.sh: No such file or directory

Note the "sudo" here, because this error only occurs if we're root.

The reason for the latter is because running Nix as root means that we
can directly access the store, which makes sure we use a filesystem
namespace to make the store writable. XXX - REWORD!

So when stracing the process, I stumbled on the following sequence:

  openat(AT_FDCWD, "/proc/self/ns/mnt", O_RDONLY) = 3
  unshare(CLONE_NEWNS)                            = 0
  ... later ...
  getcwd("/the/real/cwd", 4096)                   = 14
  setns(3, CLONE_NEWNS)                           = 0
  getcwd("/", 4096)                               = 2

In the whole strace output there are no calls to chdir() whatsoever, so
I decided to look into the kernel source to see what else could change
directories and found this[1]:

  /* Update the pwd and root */
  set_fs_pwd(fs, &root);
  set_fs_root(fs, &root);

The set_fs_pwd() call is roughly equivalent to a chdir() syscall and
this is called when the setns() syscall is invoked[2].

[1]: https://github.com/torvalds/linux/blob/b14ffae378aa1db993e62b01392e70d1e585fb23/fs/namespace.c#L4659
[2]: https://github.com/torvalds/linux/blob/b14ffae378aa1db993e62b01392e70d1e585fb23/kernel/nsproxy.c#L346

edit: Note the XXX - REWORD, this is because I wasn't sure at the time of writing this whether this is really true.

[aszlig]

Here's the diff with the preliminary fix, use it with a grain of salt because I wasn't sure whether we can actually use std::filesystem already:

diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index 70eaf4f9c..654c16e4a 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -10,6 +10,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <filesystem>
 #include <future>
 #include <iostream>
 #include <mutex>
@@ -1690,13 +1691,17 @@ void setStackSize(size_t stackSize)
     #endif
 }
 
+#if __linux__
 static AutoCloseFD fdSavedMountNamespace;
+std::optional<std::filesystem::path> savedCwd;
+#endif
 
 void saveMountNamespace()
 {
 #if __linux__
     static std::once_flag done;
     std::call_once(done, []() {
+        savedCwd.emplace(std::filesystem::current_path());
         AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY);
         if (!fd)
             throw SysError("saving parent mount namespace");
@@ -1714,6 +1719,12 @@ void restoreMountNamespace()
     } catch (Error & e) {
         debug(e.msg());
     }
+    try {
+        if (savedCwd)
+            std::filesystem::current_path(*savedCwd);
+    } catch (std::filesystem::filesystem_error const &e) {
+        debug(e.what());
+    }
 #endif
 }
 

[cole-h]

I've opened #6348 with your patch (and also my follow-up patch which removes the std::filesystem stuff, in case we can't actually use it for any reason) and will take the helm on dealing with comments and criticisms. Thanks for your hard work!