Subject: Locked mounts

From: Ludovic Courtès <[email-masked]>
Date: Tue, 11 Mar 2025 13:41:50 +0100

Hi Reepca,

Reepca Russelstein <[email-masked]> 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’.

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 [email-masked] 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"