gc: skip the processes own pid FIXES manually deleting paths

[Artturin]

fixes manually deleting paths

nix store delete /nix/store/...-zlib-1.2.11-static
error: Cannot delete path '/nix/store/...-zlib-1.2.11-static' since it is still alive. To find out why, use: nix-store --query --roots

it failed due to the function finding itself using that path

Closes #6141 #6135

tested with

(self: super: {
  nixUnstable = super.nixUnstable.overrideAttrs (old: {
    patches = [
      (builtins.toFile "gc.patch" ''
        diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
        index f65fb1b2e..e25f3ba1f 100644
        --- a/src/libstore/gc.cc
        +++ b/src/libstore/gc.cc
        @@ -357,7 +357,7 @@ void LocalStore::findRuntimeRoots(Roots & roots, bool censor)
                 auto storePathRegex = std::regex(quoteRegexChars(storeDir) + R"(/[0-9a-z]+[0-9a-zA-Z\+\-\._\?=]*)");
                 while (errno = 0, ent = readdir(procDir.get())) {
                     checkInterrupt();
        -            if (std::regex_match(ent->d_name, digitsRegex)) {
        +            if (std::regex_match(ent->d_name, digitsRegex) && ent->d_name != std::to_string(getpid())) {
                         readProcLink(fmt("/proc/%s/exe" ,ent->d_name), unchecked);
                         readProcLink(fmt("/proc/%s/cwd", ent->d_name), unchecked);
                                                                                                                                                
      '')
    ];
  });
})

[trofi]

My main worry is that the patch effectively skips everything that current process refers. My worry is that it could skip too much and garbage collect live nix-daemon paths. Would be nice to somehow limit the filter down to path that unintentionally leaks the path out.

Initially I though I understand path leak, but I don't. Do you have a simple reproducer perhaps? I'd like to explore it locally is it reproduces for me. Here is my failed attempt:

nixpkgs-staging-next $ nix-build -A mc --no-link
this path will be fetched (1.74 MiB download, 7.25 MiB unpacked):
  /nix/store/1jxvr0ahs06kfkncbvapgmgkmygacca6-mc-4.8.27
copying path '/nix/store/1jxvr0ahs06kfkncbvapgmgkmygacca6-mc-4.8.27' from 'https://cache.nixos.org'...
/nix/store/1jxvr0ahs06kfkncbvapgmgkmygacca6-mc-4.8.27

$ nix store delete /nix/store/1jxvr0ahs06kfkncbvapgmgkmygacca6-mc-4.8.27
1 store paths deleted, 3.18 MiB freed

Deletion Just Works. Here are my thoughts why it might not work for you:

There are 2 relevant processes nix gc and nix-daemon. Your patch handles nix-daemon, but nix gc might have a similar problem (maybe?).

LocalStore::findRuntimeRoots explores the following paths:

• [likely] /proc/$pid/fd/$fd - symlinks to open files (probable? what keeps it open?)
• [possible] /proc/$pid/environ - environment variables of a process
• [unlikely] /proc/$pid/exe - full path to executable (unlikely leaks reference)
• [unlikely] /proc/$pid/cwd - current directory (unlikely leaks reference)
• [unlikely] /proc/$pid/maps - mapped files into address space (unlikely leaks reference)

I think nix gc passes path as a string over file descriptor. It should only retain the path in /prod/$pid/cmdline (nix does not read it, maybe it should :).

nix-daemon is more complicated (LocalStore::collectGarbage):

• it has a server thread to get GC connections
• it has a bunch of clients pulling in paths to delete. Those get stored intempRoots by stating the file first to check if it's a link and polling for shared->pending state as scan completion hint.

I think /proc/$pid/fd/$fd is our most probable suspect. I wonder if client/server threads sometimes catch one another by accident. Or maybe it's a complex graph that has a chance to open scanned root multiple times.

[Artturin]

i can't reproduce it right now. seems like it only happens sometimes

[edolstra]

My main worry is that the patch effectively skips everything that current process refers. My worry is that it could skip too much and garbage collect live nix-daemon paths.

Yes, this would especially be a problem if auto-GC is enabled, since then a process that builds something can also do a GC.

[trofi]

I suspect it's an UI thing rather than unexpected GC root. Here is an example:

$ nix store delete /nix/store/c5bdgx7whrq3bvgfz2gsyyz989yn0ixz-mc-4.8.27.tar.xz
0 store paths deleted, 0.00 MiB freed
error: Cannot delete path '/nix/store/c5bdgx7whrq3bvgfz2gsyyz989yn0ixz-mc-4.8.27.tar.xz' since it is still alive. To find out why, use: nix-store --query --roots
$ nix-store --query --roots /nix/store/c5bdgx7whrq3bvgfz2gsyyz989yn0ixz-mc-4.8.27.tar.xz | cat

You would think it's an unreferenced tarball, but it's referenced from .drv file and is kept by keep-derivations = true:

$ nix-store --query --deriver /nix/store/c5bdgx7whrq3bvgfz2gsyyz989yn0ixz-mc-4.8.27.tar.xz | cat
/nix/store/jaqzay2r0rm3yidscxw14xd6djd319ai-mc-4.8.27.tar.xz.drv

Ideally nix-store --query should also take into account keep-derivations = true setting and show .drv as an indirect referrer.

[ghost]

Is there a workaround for this bug in the meantime while we wait for this PR to merge?

I have a store path with no references that nix-store persistently refuses to delete. I'm hunting down impurities related to switching sandbox on/off, so I need to be able to force a drv to rebuild since the sandbox parameter does not affect the drv-hash (maybe that should be reconsidered? it certainly affects the build result in many cases...)

"Just use the sandbox" is not such an easy argument anymore with all of the userns-caused LPE exploits we've had in the past month or two. Counterintuitively, allowing sandboxing makes a system less secure.

[Artturin]

Is there a workaround for this bug in the meantime while we wait for this PR to merge?

I have a store path with no references that nix-store persistently refuses to delete. I'm hunting down impurities related to switching sandbox on/off, so I need to be able to force a drv to rebuild since the sandbox parameter does not affect the drv-hash (maybe that should be reconsidered? it certainly affects the build result in many cases...)

"Just use the sandbox" is not such an easy argument anymore with all of the userns-caused LPE exploits we've had in the past month or two. Counterintuitively, allowing sandboxing makes a system less secure.

there's a --rebuild flag for nix build and --check for nix-build

[ghost]

there's a --rebuild flag for nix build and --check for nix-build

Yeah --check hardly ever works for me because most installPhases will fail if they try to copy something to the installation directory and there is already a file there.

Regarding --rebuild:

$ nix-build --rebuild /nix/store/2n7frkqmkavkaj1va3wphr4w61dr8vj3-bzip2-1.0.6.0.2
error: unrecognised flag '--rebuild'
Try 'nix-build --help' for more information.

[ghost]

Yeah --check hardly ever works for me because most installPhases will fail if they try to copy something to the installation directory and there is already a file there.

Ok, I guess the workaround is to:

1. make really sure the store path is unreferenced
2. rm -rf the store path
3. mkdir the store path
4. run nix-build --check

[ghost]

I think #6354 might be related.

[asymmetric]

Regarding --rebuild:

@a-m-joseph Try nix build —rebuild