Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: NixOS/nix
base: c9d0cf7e02d4
Choose a base ref
...
head repository: NixOS/nix
compare: ee754f0f41df
Choose a head ref
  • 2 commits
  • 1 file changed
  • 2 contributors

Commits on Apr 27, 2020

  1. Fix long paths permanently breaking GC

    Suppose I have a path /nix/store/[hash]-[name]/a/a/a/a/a/[...]/a,
    long enough that everything after "/nix/store/" is longer than 4096
    (MAX_PATH) bytes.
    
    Nix will happily allow such a path to be inserted into the store,
    because it doesn't look at all the nested structure.  It just cares
    about the /nix/store/[hash]-[name] part.  But, when the path is deleted,
    we encounter a problem.  Nix will move the path to /nix/store/trash, but
    then when it's trying to recursively delete the trash directory, it will
    at some point try to unlink
    /nix/store/trash/[hash]-[name]/a/a/a/a/a/[...]/a.  This will fail,
    because the path is too long.  After this has failed, any store deletion
    operation will never work again, because Nix needs to delete the trash
    directory before recreating it to move new things to it.  (I assume this
    is because otherwise a path being deleted could already exist in the
    trash, and then moving it would fail.)
    
    This means that if I can trick somebody into just fetching a tarball
    containing a path of the right length, they won't be able to delete
    store paths or garbage collect ever again, until the offending path is
    manually removed from /nix/store/trash.  (And even fixing this manually
    is quite difficult if you don't understand the issue, because the
    absolute path that Nix says it failed to remove is also too long for
    rm(1).)
    
    This patch fixes the issue by making Nix's recursive delete operation
    use unlinkat(2).  This function takes a relative path and a directory
    file descriptor.  We ensure that the relative path is always just the
    name of the directory entry, and therefore its length will never exceed
    255 bytes.  This means that it will never even come close to AX_PATH,
    and Nix will therefore be able to handle removing arbitrarily deep
    directory hierachies.
    
    Since the directory file descriptor is used for recursion after being
    used in readDirectory, I made a variant of readDirectory that takes an
    already open directory stream, to avoid the directory being opened
    multiple times.  As we have seen from this issue, the less we have to
    interact with paths, the better, and so it's good to reuse file
    descriptors where possible.
    
    I left _deletePath as succeeding even if the parent directory doesn't
    exist, even though that feels wrong to me, because without that early
    return, the linux-sandbox test failed.
    
    Reported-by: Alyssa Ross <hi@alyssa.is>
    Thanks-to: Puck Meerburg <puck@puckipedia.com>
    Tested-by: Puck Meerburg <puck@puckipedia.com>
    Reviewed-by: Puck Meerburg <puck@puckipedia.com>
    alyssais committed Apr 27, 2020
    Copy the full SHA
    c05e20d View commit details
    Browse the repository at this point in the history

Commits on Apr 28, 2020

  1. Merge pull request #3541 from alyssais/gcdos

    Fix long paths permanently breaking GC
    edolstra committed Apr 28, 2020
    Copy the full SHA
    ee754f0 View commit details
    Browse the repository at this point in the history