Package: guix-patches;
Reported by: Ludovic Courtès <ludo <at> gnu.org>
Date: Fri, 24 Jan 2025 17:24:02 UTC
Severity: normal
Tags: patch
Done: Ludovic Courtès <ludo <at> gnu.org>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Ludovic Courtès <ludo <at> gnu.org> To: Reepca Russelstein <reepca <at> russelstein.xyz> Cc: 75810 <at> debbugs.gnu.org Subject: [bug#75810] Locked mounts Date: Tue, 11 Mar 2025 13:41:50 +0100
[Message part 1 (text/plain, inline)]
Hi Reepca, Reepca Russelstein <reepca <at> russelstein.xyz> skribis: > I still think it would be a good idea to call unshare to create an extra > user and mount namespace just before executing the builder in the > unprivileged case, just to be sure that the mount-locking behavior is > triggered in a way that is documented. For some reason, it’s not working as advertised: mounts are seemingly not locked together and umount(2) on one of them returns EPERM (instead of EINVAL). I suspect chroot, pivot_root, & co. spoil it all. Attached is a patch and test case. To be sure, I wrote a minimal C program: umount returns EINVAL as expected. However, when compiling it with -DWITH_CHROOT, unshare(2) fails with EPERM because “the caller's root directory does not match the root directory of the mount namespace in which it resides” (quoting unshare(2)). So I now get the idea of “locked mounts” but I’m at loss as to how this is supposed to interact with chroots. Thoughts? Ludo’.
[Message part 2 (text/x-patch, inline)]
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index 057a15ccd0..6a6a960a1c 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -2244,6 +2244,13 @@ void DerivationGoal::runChild() /* Remount root as read-only. */ if (mount("/", "/", 0, MS_BIND | MS_REMOUNT | MS_RDONLY, 0) == -1) throw SysError(format("read-only remount of build root '%1%' failed") % chrootRootDir); + + if (getuid() != 0) { + /* Create a new mount namespace to "lock" previous mounts. + See mount_namespaces(7). */ + if (unshare(CLONE_NEWNS | CLONE_NEWUSER) == -1) + throw SysError(format("creating new user and mount namespaces")); + } } #endif diff --git a/tests/store.scm b/tests/store.scm index c22739afe6..9da9345dd0 100644 --- a/tests/store.scm +++ b/tests/store.scm @@ -37,6 +37,8 @@ (define-module (test-store) #:use-module (guix gexp) #:use-module (gnu packages) #:use-module (gnu packages bootstrap) + #:use-module ((gnu packages make-bootstrap) + #:select (%guile-static-stripped)) #:use-module (ice-9 match) #:use-module (ice-9 regex) #:use-module (rnrs bytevectors) @@ -59,6 +61,8 @@ (define %shell (test-begin "store") +(test-skip 25) + (test-assert "open-connection with file:// URI" (let ((store (open-connection (string-append "file://" (%daemon-socket-uri))))) @@ -455,7 +459,7 @@ (define %shell (drv (run-with-store %store (gexp->derivation - "attempt-to-remount-input-read-write" + "attempt-to-write-to-input" (with-imported-modules (source-module-closure '((guix build syscalls))) #~(begin @@ -496,6 +500,58 @@ (define %shell (build-derivations %store (list drv)) #f))) +(let ((guile (with-external-store external-store + (and external-store + (run-with-store external-store + (mlet %store-monad ((drv (lower-object %guile-static-stripped))) + (mbegin %store-monad + (built-derivations (list drv)) + (return (derivation->output-path (pk 'GDRV drv)))))))))) + + (unless (and guile (unprivileged-user-namespace-supported?)) + (test-skip 1)) + (test-equal "input mount is locked" + EINVAL + ;; Check that mounts within the build environment are "locked" together and + ;; cannot be separated from within the build environment namespace--see + ;; mount_namespaces(7). + ;; + ;; Since guile-bootstrap <at> 2.0 lacks 'umount', resort to the hack below to + ;; get a statically-linked Guile with 'umount'. + (let* ((guile (computed-file "guile-with-umount" + ;; The #:guile-for-build argument must be a + ;; derivation, hence this silly thing. + #~(symlink #$(local-file guile "guile-with-umount" + #:recursive? #t) + #$output) + #:guile %bootstrap-guile)) + (drv + (run-with-store %store + (mlet %store-monad ((guile (lower-object guile))) + (gexp->derivation + "attempt-to-unmount-input" + (with-imported-modules (source-module-closure + '((guix build syscalls))) + #~(begin + (use-modules (guix build syscalls)) + + (let ((input #$(plain-file "input-that-might-be-unmounted" + (random-text)))) + (catch 'system-error + (lambda () + ;; umount(2) returns EINVAL when the target is locked. + (umount input)) + (lambda args + (call-with-output-file #$output + (lambda (port) + (write (system-error-errno args) port)))))))) + #:guile-for-build guile))))) + (build-derivations %store (list (pk 'UMDRV drv))) + (call-with-input-file (derivation->output-path drv) + read)))) + +(test-skip 100) + (unless (unprivileged-user-namespace-supported?) (test-skip 1)) (test-assert "build root cannot be made world-readable"
[mount-namespace-locking.c (text/plain, attachment)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.