guix_mirror_bot pushed a commit to branch master
in repository guix.
commit 865cb0188c282726008c56319a853c7dd82c4057
Author: Reepca Russelstein <[email protected]>
AuthorDate: Sun Feb 15 12:39:34 2026 -0600
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 <[email protected]>
Change-Id: I749127fe5254ebabc8387a2f0ef47e3c116bfcc5
Signed-off-by: Ludovic Courtès <[email protected]>
Merges: #6460
---
nix/libutil/util.cc | 51 +++++++++++++++++++++++++++++++++++++++++++++++++--
nix/libutil/util.hh | 9 ++++++++-
tests/derivations.scm | 16 +++++++++++++++-
3 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc
index ed1a371dfe..95f293ff10 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
O_DIRECTORY |
O_NOFOLLOW |
O_CLOEXEC);
- if(!dirfd.isOpen())
- throw SysError(std::format("opening `{}'", fullPath));
+ if(!dirfd.isOpen()) {
+ if (errno != EACCES)
+ throw SysError(std::format("opening `{}'", fullPath));
+ /* Target directory must not have the right permissions for us to
+ * access it. Try changing them. We only do this after the
+ * initial attempt fails because some of the ways we might try
+ * changing the permissions have race conditions, and we'd rather
+ * avoid them if we can (e.g. because we're root). */
+#ifdef O_PATH
+ {
+ AutoCloseFD pathfd = openat(fd, path.c_str(),
+ O_PATH |
+ O_DIRECTORY |
+ O_NOFOLLOW |
+ O_CLOEXEC);
+ if (!pathfd.isOpen())
+ throw SysError(std::format("opening `{}'", fullPath));
+
+ /* fchmod doesn't work with O_PATH file descriptors. fchmodat
+ * does, but only on very recent kernels (linux 6.6). Despite
+ * this, regular chmod will work with a /proc/self/fd/N
+ * filename that names an O_PATH file descriptor. */
+ string procPath = "/proc/self/fd/" + std::to_string(pathfd);
+ if (chmod(procPath.c_str(), S_IRUSR | S_IWUSR | S_IXUSR) != 0) {
+ if (errno != ENOENT)
+ throw SysError(std::format("chmod of `{}", procPath));
+ /* Fall through */
+ } else {
+ goto retry_open;
+ }
+ }
+#endif
+ /* !!! If a malicious process can have replaced the directory at
+ PATH with a hardlink to an important file, this may change
+ its permissions to become overly-strict! This should only
+ be a concern where /proc/sys/fs/protected_hardlinks is 0, or
+ on systems without protected_hardlinks. */
+ if (fchmodat(fd, path.c_str(), S_IRUSR | S_IWUSR | S_IXUSR,
AT_SYMLINK_NOFOLLOW) != 0)
+ throw SysError(std::format("fchmodat of `{}'", fullPath));
+
+ retry_open:
+ dirfd = openat(fd, path.c_str(),
+ O_RDONLY |
+ O_DIRECTORY |
+ O_NOFOLLOW |
+ O_CLOEXEC);
+ if (!dirfd.isOpen())
+ throw SysError(std::format("opening `{}'", fullPath));
+ }
/* st.st_mode may currently be from a different file than what we
actually opened, get it straight from the file instead */
diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh
index 3bf2f0be75..44d579f3d6 100644
--- a/nix/libutil/util.hh
+++ b/nix/libutil/util.hh
@@ -98,7 +98,14 @@ void writeLine(int fd, string s);
/* Delete a path; i.e., in the case of a directory, it is deleted
recursively. Don't use this at home, kids. The second variant
returns the number of bytes and blocks freed, and 'linkThreshold' denotes
- the number of links under which a file is accounted for in 'bytesFreed'. */
+ the number of links under which a file is accounted for in 'bytesFreed'.
+
+ Note that if a directory is unreadable, chmod will be invoked on it to make
+ it u+rwx. On non-linux systems with no equivalent to
+ /proc/sys/fs/protected_hardlinks, a TOCTTOU race may allow the directory to
+ be replaced with a hardlink to an important file, making that file's
+ permissions overly-strict.
+ */
void deletePath(const Path & path);
void deletePath(const Path & path, unsigned long long & bytesFreed,
diff --git a/tests/derivations.scm b/tests/derivations.scm
index d4cca0f605..b0d74ca50d 100644
--- a/tests/derivations.scm
+++ b/tests/derivations.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012-2025 Ludovic Courtès <[email protected]>
+;;; Copyright © 2012-2026 Ludovic Courtès <[email protected]>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -967,6 +967,20 @@
(build-derivations %store (list drv))
#f)))
+(test-assert "unreadable directories in build tree can be removed"
+ ;; Unreadable directories created in the build tree should be removed
+ ;; without problem. See <https://codeberg.org/guix/guix/issues/5891>.
+ (let* ((drv (build-expression->derivation
+ %store
+ (string-append (number->string (random (expt 2 64) (%seed))
+ 16)
+ "-unreadable-test")
+ '(begin
+ (mkdir "unreadable" #o000)
+ (mkdir %output)))))
+ (build-derivations %store (list drv))
+ (file-is-directory? (derivation->output-path drv))))
+
(define %coreutils
(false-if-exception
