forked from lix-project/lix
Alyssa Ross
c05e20daa1
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> |
||
|---|---|---|
| .. | ||
| affinity.cc | ||
| affinity.hh | ||
| ansicolor.hh | ||
| archive.cc | ||
| archive.hh | ||
| args.cc | ||
| args.hh | ||
| compression.cc | ||
| compression.hh | ||
| config.cc | ||
| config.hh | ||
| error.cc | ||
| error.hh | ||
| finally.hh | ||
| hash.cc | ||
| hash.hh | ||
| istringstream_nocopy.hh | ||
| json.cc | ||
| json.hh | ||
| lazy.hh | ||
| local.mk | ||
| logging.cc | ||
| logging.hh | ||
| lru-cache.hh | ||
| monitor-fd.hh | ||
| pool.hh | ||
| ref.hh | ||
| rust-ffi.cc | ||
| rust-ffi.hh | ||
| serialise.cc | ||
| serialise.hh | ||
| sync.hh | ||
| tarfile.cc | ||
| tarfile.hh | ||
| thread-pool.cc | ||
| thread-pool.hh | ||
| types.hh | ||
| url.cc | ||
| url.hh | ||
| util.cc | ||
| util.hh | ||
| xml-writer.cc | ||
| xml-writer.hh | ||