diff options
| author | Reepca Russelstein <reepca@russelstein.xyz> | 2026-02-15 12:39:34 -0600 |
|---|---|---|
| committer | Ludovic Courtès <ludo@gnu.org> | 2026-02-27 23:54:00 +0100 |
| commit | 865cb0188c282726008c56319a853c7dd82c4057 (patch) | |
| tree | a6d6fa69b00786b734d7a2a704e19794c1984594 /nix/libutil/util.cc | |
| parent | a1611ced6de6e58ec77641281a49ed368d7d8cef (diff) | |
daemon: Actually remove unreadable directories.
Fixes a regression introduced in 7173c2c0ca. Additional discussion at
https://codeberg.org/guix/guix/pulls/5977.
* nix/libutil/util.cc (_deletePathAt): chmod directory and retry open when it
fails with EACCES. Do this using an O_PATH file descriptor referenced via
/proc/self/fd whenever possible to avoid it being replaced by a
non-directory immediately before being chmod'ed.
* nix/libutil/util.hh (deletePath): document TOCTTOU race on non-linux systems
where hardlinks aren't protected.
* tests/derivations.scm ("unreadable directories in build tree can be
removed"): new test.
Fixes: guix/guix#5891
Reported-by: Liliana Marie Prikler <liliana.prikler@gmail.com>
Change-Id: I749127fe5254ebabc8387a2f0ef47e3c116bfcc5
Signed-off-by: Ludovic Courtès <ludo@gnu.org>
Merges: #6460
Diffstat (limited to 'nix/libutil/util.cc')
| -rw-r--r-- | nix/libutil/util.cc | 51 |
1 files changed, 49 insertions, 2 deletions
diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc index ed1a371dfe1..95f293ff10f 100644 --- a/nix/libutil/util.cc +++ b/nix/libutil/util.cc | |||
| @@ -380,8 +380,55 @@ static void _deletePathAt(int fd, const Path & path, const Path & fullPath, unsi | |||
| 380 | O_DIRECTORY | | 380 | O_DIRECTORY | |
| 381 | O_NOFOLLOW | | 381 | O_NOFOLLOW | |
| 382 | O_CLOEXEC); | 382 | O_CLOEXEC); |
| 383 | if(!dirfd.isOpen()) | 383 | if(!dirfd.isOpen()) { |
| 384 | throw SysError(std::format("opening `{}'", fullPath)); | 384 | if (errno != EACCES) |
| 385 | throw SysError(std::format("opening `{}'", fullPath)); | ||
| 386 | /* Target directory must not have the right permissions for us to | ||
| 387 | * access it. Try changing them. We only do this after the | ||
| 388 | * initial attempt fails because some of the ways we might try | ||
| 389 | * changing the permissions have race conditions, and we'd rather | ||
| 390 | * avoid them if we can (e.g. because we're root). */ | ||
| 391 | #ifdef O_PATH | ||
| 392 | { | ||
| 393 | AutoCloseFD pathfd = openat(fd, path.c_str(), | ||
| 394 | O_PATH | | ||
| 395 | O_DIRECTORY | | ||
| 396 | O_NOFOLLOW | | ||
| 397 | O_CLOEXEC); | ||
| 398 | if (!pathfd.isOpen()) | ||
| 399 | throw SysError(std::format("opening `{}'", fullPath)); | ||
| 400 | |||
| 401 | /* fchmod doesn't work with O_PATH file descriptors. fchmodat | ||
| 402 | * does, but only on very recent kernels (linux 6.6). Despite | ||
| 403 | * this, regular chmod will work with a /proc/self/fd/N | ||
| 404 | * filename that names an O_PATH file descriptor. */ | ||
| 405 | string procPath = "/proc/self/fd/" + std::to_string(pathfd); | ||
| 406 | if (chmod(procPath.c_str(), S_IRUSR | S_IWUSR | S_IXUSR) != 0) { | ||
| 407 | if (errno != ENOENT) | ||
| 408 | throw SysError(std::format("chmod of `{}", procPath)); | ||
| 409 | /* Fall through */ | ||
| 410 | } else { | ||
| 411 | goto retry_open; | ||
| 412 | } | ||
| 413 | } | ||
| 414 | #endif | ||
| 415 | /* !!! If a malicious process can have replaced the directory at | ||
| 416 | PATH with a hardlink to an important file, this may change | ||
| 417 | its permissions to become overly-strict! This should only | ||
| 418 | be a concern where /proc/sys/fs/protected_hardlinks is 0, or | ||
| 419 | on systems without protected_hardlinks. */ | ||
| 420 | if (fchmodat(fd, path.c_str(), S_IRUSR | S_IWUSR | S_IXUSR, AT_SYMLINK_NOFOLLOW) != 0) | ||
| 421 | throw SysError(std::format("fchmodat of `{}'", fullPath)); | ||
| 422 | |||
| 423 | retry_open: | ||
| 424 | dirfd = openat(fd, path.c_str(), | ||
| 425 | O_RDONLY | | ||
| 426 | O_DIRECTORY | | ||
| 427 | O_NOFOLLOW | | ||
| 428 | O_CLOEXEC); | ||
| 429 | if (!dirfd.isOpen()) | ||
| 430 | throw SysError(std::format("opening `{}'", fullPath)); | ||
| 431 | } | ||
| 385 | 432 | ||
| 386 | /* st.st_mode may currently be from a different file than what we | 433 | /* st.st_mode may currently be from a different file than what we |
| 387 | actually opened, get it straight from the file instead */ | 434 | actually opened, get it straight from the file instead */ |
