[bug#43857] Supporting chroot builds on GNU/Hurd

[bug#43857] Supporting chroot builds on GNU/Hurd

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 5199 bytes --]

Hello!

The patch below is an attempt at supporting “chroot builds” on GNU/Hurd;
it’s “almost working”.  The main feature we need is firmlinks (or “bind
mounts”) and commenting out Linux-specific things (private bind mounts,
‘pivot_root’, etc.).

The patch introduces a ‘firmlink’ function that creates a bind mount on
GNU/Linux and otherwise spawns the /hurd/firmlink translator.  It also
adjusts ‘deletePath’ to remove any active translators from files it’s
about to delete, the goal being to terminate those firmlink translators.
(Apparently that code isn’t reached yet though.)

The set of /dev nodes firmlinked into the chroot is reduced compared to
GNU/Linux; I added /servers, meaning that /servers is firmlinked as is,
which is not ideal (see below).

With this patch, I can run “guix build hello --check” in a chroot…  but
it eventually hangs somewhere in ‘DerivationGoal::buildDone’ (I presume)
once the build has completed.  It leaves behind it all its firmlink
processes:

--8<---------------cut here---------------start------------->8---
root@childhurd ~# ps --width=200 aux|grep firmlink
root     16426 120.0 0.0  147M  616K p1 RNm  11:38PM  0:00.00 grep --color=auto firmlink
root     11191  0.0  0.0  147M  888K  - S<o  11:27PM  0:00.00 /hurd/firmlink /dev/full
root     11192  0.0  0.0  147M  936K  - S<o  11:27PM  0:00.01 /hurd/firmlink /dev/null
root     11193  0.0  0.0  147M  888K  - S<o  11:27PM  0:00.00 /hurd/firmlink /dev/random
root     11194  0.0  0.0  147M  936K  - S<o  11:27PM  0:00.00 /hurd/firmlink /dev/urandom
root     11195  0.0  0.0  147M  888K  - S<o  11:27PM  0:00.00 /hurd/firmlink /dev/zero
root     11196  0.0  0.0  147M  936K  - S<o  11:27PM  0:00.02 /hurd/firmlink /gnu/store/1brrgqhgjni8g8dbp97s8c7d7nq0p9a0-libgc-8.0.4
root     11197  0.0  0.0  147M  936K  - S<o  11:27PM  0:00.05 /hurd/firmlink /gnu/store/1gmsfg4cfmj3l54s3nkn3lyas3nnzjgs-hurd-core-headers-0.9-1.91a5167
root     11198  0.0  0.0  147M  936K  - S<o  11:27PM  0:00.00 /hurd/firmlink /gnu/store/5ndcmqx0pzksq5c4mznbv20xhfnk3p97-zlib-1.2.11
root     11199  0.0  0.0  147M  936K  - S<o  11:27PM  0:00.02 /hurd/firmlink /gnu/store/631ixr5flpk66ziigbmkxwafz8skpyzs-findutils-4.7.0
root     11200  0.0  0.0  147M  936K  - S<o  11:27PM  0:00.59 /hurd/firmlink /gnu/store/6zfpncmmz1aqpsg1ba7zjna8qb4nmwdx-file-5.38
[...]
root     11233  0.0  0.0  147M  936K  - S<o  11:27PM  0:00.58 /hurd/firmlink /gnu/store/zjlh3sljylr3y1cavxp2z7g37g922mbb-gcc-7.5.0
root     11234  0.0  0.0  147M  936K  - S<o  11:27PM  0:00.02 /hurd/firmlink /servers
root     11235  0.0  0.0  147M  936K  - S<o  11:27PM  0:00.07 /hurd/firmlink /tmp/guix-build-hello-2.10.drv-6
--8<---------------cut here---------------end--------------->8---

This has yet to be debugged.  :-)

Apart from that, this raises the question of what to put in the build
environment.  As written in the blog post about childhurds that should
go out tomorrow, on GNU/Linux, we do not include any piece of userland
software in the environment.  But here, we’d be doing just that: running
Hurd translators that are not specified as derivation inputs.  It’s OK
for /dev/null, but maybe questionable for /servers/socket/*.

Regarding /servers, we’ll probably want to spawn separate servers (a
separate /servers/proc would give up a new “PID namespace”.)  Should
that be done magically by the daemon, or should we leave it up to build
processes, so that build processes really specify all the user-land
software they depend on?  We could imagine a new phase in
‘gnu-build-system’, on GNU/Hurd, that would start by setting up a bunch
of translators.  Food for thought…  As a first step, we can firmlink it
from the host, with an eye towards “doing it right”.

I felt a need to hack on this as I was investigating an util-linux test
failure in a ‘--disable-chroot’ setup: the test would find /proc and
would later fail for some reason.  Had /proc been missing from the build
environment (as is the case with this patch), the test would have been
skipped (it explicitly handles that case).  I think we’d rather not
fiddle too much with test suites until we have defined what’s available
in the default build environment.

Thoughts?

For the record, I’ve been testing this by:

  1. Adding ‘hurd’ as an input to the ‘guix’ package.

  2. Cross-building the daemon using code from my branch:

    ~/src/guix/pre-inst-env guix build --with-git-url=guix-daemon=$PWD --with-branch=guix-daemon=wip-hurd-chroot --target=i586-pc-gnu  -e '(@ (gnu packages package-management) guix-daemon)'

  3. Sending it over to the childhurd:

     guix copy --to=localhost:10022 /gnu/store/…

  4. Running it in the childhurd with:

     GUIX=/run/current-system/profile/bin/guix /gnu/store/…-guix-daemon-git.wip-hurd-chroot/bin/guix-daemon --build-users-group=guixbuild --disable-deduplication

I guess I should probably go ahead and do everything natively in the
childhurd, but I thought I’d share my weird workflow in case that gives
me a chance to participate in some weirdness contest.

Ludo’.


[-- Attachment #2: the patch --]
[-- Type: text/x-patch, Size: 12521 bytes --]

From 1887d0dee0031df0de117b3a6339495504b4b489 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Tue, 6 Oct 2020 23:53:24 +0200
Subject: [PATCH] DRAFT daemon: Support chroot builds on GNU/Hurd.

* nix/libutil/util.cc (firmlink): New functions.
(_deletePath) [__GNU__]: Check whether a translator is set on PATH.
Call 'fsys_goaway' if this is the case.
* nix/libutil/util.hh (firmlink): New declaration.
* nix/libstore/build.cc (CHROOT_ENABLED): Define to 1.  Error out when
both __GNU__ and __linux__ are undefined.
(DerivationGoal::runChild): Remove special treatment of /proc.  Use
'firmlink' instead of 'mount' with MS_BIND.
Wrap /proc, /dev/shm, /dev/pts, and /proc/self handling in #ifdef
__linux__.  Same for 'pivot_root' call.
* config-daemon.ac: Set and substitute 'HURD_LIBS'.
* nix/local.mk (guix_daemon_LDADD): Add $(HURD_LIBS).
---
 config-daemon.ac      |  13 ++++
 nix/libstore/build.cc |  43 +++++++++----
 nix/libutil/util.cc   | 139 ++++++++++++++++++++++++++++++++++++++++++
 nix/libutil/util.hh   |   3 +
 nix/local.mk          |   3 +-
 5 files changed, 189 insertions(+), 12 deletions(-)

diff --git a/config-daemon.ac b/config-daemon.ac
index 50ead355a8..bdaee82fb8 100644
--- a/config-daemon.ac
+++ b/config-daemon.ac
@@ -38,6 +38,19 @@ if test "x$guix_build_daemon" = "xyes"; then
   AC_DEFINE_UNQUOTED([SYSTEM], ["$guix_system"],
     [Guix host system type--i.e., platform and OS kernel tuple.])
 
+  dnl On GNU/Hurd guix-daemon depends on libfshelp.
+  case "$guix_system" in
+    *-gnu)
+      AC_CHECK_LIB([fshelp], [fshelp_start_translator])
+      if test "x$ac_cv_lib_fshelp_fshelp_start_translator" != "xyes"; then
+        AC_MSG_ERROR([libfshelp (GNU Hurd) could not be found])
+      fi
+      HURD_LIBS="-lfshelp";;
+    *)
+      HURD_LIBS="";;
+  esac
+  AC_SUBST([HURD_LIBS])
+
   case "$LIBGCRYPT_PREFIX" in
     no)
       LIBGCRYPT_CFLAGS=""
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index ccec513d8d..7151932403 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -52,7 +52,13 @@
 #endif
 
 
-#define CHROOT_ENABLED HAVE_CHROOT && HAVE_SYS_MOUNT_H && defined(MS_BIND) && defined(MS_PRIVATE)
+/* Chroot builds are supported both on GNU/Linux and on GNU/Hurd.  */
+#if defined __linux__ || defined __GNU__
+# define CHROOT_ENABLED 1
+#else
+# error unsupported operating system
+#endif
+
 #define CLONE_ENABLED defined(CLONE_NEWNS)
 
 #if defined(SYS_pivot_root)
@@ -1991,6 +1997,7 @@ void DerivationGoal::runChild()
             if (setdomainname(domainname, sizeof(domainname)) == -1)
                 throw SysError("cannot set domain name");
 
+#ifdef __linux__
             /* Make all filesystems private.  This is necessary
                because subtrees may have been mounted as "shared"
                (MS_SHARED).  (Systemd does this, for instance.)  Even
@@ -2007,27 +2014,30 @@ void DerivationGoal::runChild()
                different filesystem from /, as needed for pivot_root. */
             if (mount(chrootRootDir.c_str(), chrootRootDir.c_str(), 0, MS_BIND, 0) == -1)
                 throw SysError(format("unable to bind mount ‘%1%’") % chrootRootDir);
+#endif
 
             /* Set up a nearly empty /dev, unless the user asked to
                bind-mount the host /dev. */
             Strings ss;
             if (dirsInChroot.find("/dev") == dirsInChroot.end()) {
+#ifdef __linux__
                 createDirs(chrootRootDir + "/dev/shm");
                 createDirs(chrootRootDir + "/dev/pts");
-                ss.push_back("/dev/full");
-#ifdef __linux__
+                createSymlink("/proc/self/fd", chrootRootDir + "/dev/fd");
+                createSymlink("/proc/self/fd/0", chrootRootDir + "/dev/stdin");
+                createSymlink("/proc/self/fd/1", chrootRootDir + "/dev/stdout");
+                createSymlink("/proc/self/fd/2", chrootRootDir + "/dev/stderr");
+                ss.push_back("/dev/tty");
                 if (pathExists("/dev/kvm"))
                     ss.push_back("/dev/kvm");
+#elif __GNU__
+		ss.push_back("/servers");
 #endif
+                ss.push_back("/dev/full");
                 ss.push_back("/dev/null");
                 ss.push_back("/dev/random");
-                ss.push_back("/dev/tty");
                 ss.push_back("/dev/urandom");
                 ss.push_back("/dev/zero");
-                createSymlink("/proc/self/fd", chrootRootDir + "/dev/fd");
-                createSymlink("/proc/self/fd/0", chrootRootDir + "/dev/stdin");
-                createSymlink("/proc/self/fd/1", chrootRootDir + "/dev/stdout");
-                createSymlink("/proc/self/fd/2", chrootRootDir + "/dev/stderr");
             }
 
             /* Fixed-output derivations typically need to access the
@@ -2049,7 +2059,7 @@ void DerivationGoal::runChild()
                 struct stat st;
                 Path source = i->second;
                 Path target = chrootRootDir + i->first;
-                if (source == "/proc") continue; // backwards compatibility
+
                 debug(format("bind mounting `%1%' to `%2%'") % source % target);
                 if (stat(source.c_str(), &st) == -1)
                     throw SysError(format("getting attributes of path `%1%'") % source);
@@ -2059,10 +2069,11 @@ void DerivationGoal::runChild()
                     createDirs(dirOf(target));
                     writeFile(target, "");
                 }
-                if (mount(source.c_str(), target.c_str(), "", MS_BIND, 0) == -1)
+                if (firmlink(source, target) == -1)
                     throw SysError(format("bind mount from `%1%' to `%2%' failed") % source % target);
             }
 
+#ifdef __linux__
             /* Bind a new instance of procfs on /proc to reflect our
                private PID namespace. */
             createDirs(chrootRootDir + "/proc");
@@ -2090,11 +2101,16 @@ void DerivationGoal::runChild()
                    Linux versions, it is created with permissions 0.  */
                 chmod_(chrootRootDir + "/dev/pts/ptmx", 0666);
             }
+#elif __GNU__
+	    /* Do not mount things that are implemented in user land: /proc,
+	       /dev/shm, /dev/pts, etc.  */
+#endif
 
             /* Do the chroot(). */
             if (chdir(chrootRootDir.c_str()) == -1)
                 throw SysError(format("cannot change directory to '%1%'") % chrootRootDir);
 
+#ifdef __linux__
             if (mkdir("real-root", 0) == -1)
                 throw SysError("cannot create real-root directory");
 
@@ -2109,8 +2125,13 @@ void DerivationGoal::runChild()
 
             if (rmdir("real-root") == -1)
                 throw SysError("cannot remove real-root directory");
-        }
+#elif __GNU__
+            if (chroot(".") == -1)
+                throw SysError(format("cannot change root directory to '%1%'") % chrootRootDir);
+
 #endif
+        }
+#endif	// CHROOT_ENABLED
 
         if (chdir(tmpDirInSandbox.c_str()) == -1)
             throw SysError(format("changing into `%1%'") % tmpDir);
diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc
index 59a2981359..b49a17a6eb 100644
--- a/nix/libutil/util.cc
+++ b/nix/libutil/util.cc
@@ -23,6 +23,41 @@
 #include <sys/prctl.h>
 #endif
 
+#if HAVE_SYS_MOUNT_H
+#include <sys/mount.h>
+#endif
+
+#ifdef __GNU__
+
+extern "C" {
+
+/* XXX: <idvec.h> uses 'new' as a parameter name.  Work around it.  */
+# define new new_param
+
+# include <hurd.h>
+# include <hurd/paths.h>
+# include <hurd/fsys.h>
+# include <argz.h>
+
+# undef new
+
+/* XXX: <fshelp.h> is not C++-compatible.  Copy these declarations to work
+   around it.  */
+
+typedef error_t (*fshelp_open_fn_t) (int flags,
+				     file_t *node,
+				     mach_msg_type_name_t *node_type,
+				     task_t, void *cookie);
+
+extern error_t
+fshelp_start_translator (fshelp_open_fn_t underlying_open_fn, void *cookie,
+			 char *name, char *argz, int argz_len,
+			 int timeout, fsys_t *control);
+
+}
+
+# define _HURD_FIRMLINK  _HURD "firmlink"
+#endif
 
 extern char * * environ;
 
@@ -214,6 +249,89 @@ bool isLink(const Path & path)
     return S_ISLNK(st.st_mode);
 }
 
+#if __linux__
+
+int firmlink(const Path &source, const Path &target)
+{
+    return mount(source.c_str(), target.c_str(), "", MS_BIND, 0);
+}
+
+#elif __GNU__
+
+static error_t return_node (int flags,
+			    mach_port_t *underlying,
+			    mach_msg_type_name_t *underlying_type,
+			    task_t task, void *node)
+{
+    *underlying = * (mach_port_t *) node;
+    *underlying_type = MACH_MSG_TYPE_COPY_SEND;
+
+    return ESUCCESS;
+}
+
+int firmlink(const Path &source, const Path &target)
+{
+    static char firmlink[] = _HURD_FIRMLINK;
+    char *arg_vec[] = { firmlink, (char *) source.c_str (), NULL };
+    char *args = NULL;
+    size_t args_len;
+
+    error_t err;
+    file_t target_file = MACH_PORT_NULL;
+
+    printMsg (lvlChatty, format("creating firmlink from '%1%' to '%2%'")
+	      % source % target);
+
+    target_file = file_name_lookup (target.c_str (), O_NOTRANS, 0);
+    if (! MACH_PORT_VALID (target_file)) {
+	printMsg (lvlChatty, format("firmlink target '%s' unavailable: %s")
+		  % target % strerror (errno));
+	goto fail;
+    }
+
+    err = argz_create (arg_vec, &args, &args_len);
+    if (err != 0) goto fail;
+
+    mach_port_t control;
+    err = fshelp_start_translator (return_node, &target_file,
+				   firmlink, args, args_len,
+				   3000, &control);
+    if (err) {
+	printMsg (lvlChatty, format("failed to start '%s' translator: %s") %
+		  firmlink % strerror(errno));
+	goto fail;
+    }
+
+    free ((void *) args);
+    args = NULL;
+
+    err = (error_t) file_set_translator (target_file, 0, FS_TRANS_SET, 0,
+					 NULL, 0,
+					 control, MACH_MSG_TYPE_COPY_SEND);
+    mach_port_deallocate (mach_task_self (), control);
+    mach_port_deallocate (mach_task_self (), target_file);
+
+    if (err) {
+	printMsg (lvlChatty, format("failed to set '%s' translator on node '%s': %s") %
+		  firmlink % target % strerror(errno));
+	goto fail;
+    }
+
+    return err;
+
+fail:
+    int saved_errno = errno;
+    if (MACH_PORT_VALID (target_file))
+	mach_port_deallocate (mach_task_self (), target_file);
+    if (args != NULL)
+	free ((void *) args);
+    errno = saved_errno;
+    return -1;
+}
+
+#elif
+# error unsupported operating system
+#endif
 
 DirEntries readDirectory(const Path & path)
 {
@@ -311,6 +429,27 @@ static void _deletePath(const Path & path, unsigned long long & bytesFreed, size
 
     printMsg(lvlVomit, format("%1%") % path);
 
+#ifdef __GNU__
+    /* Check whether there's an active translator on PATH--typically
+       /hurd/firmlink.  If there is one, let it go away.  */
+    {
+	file_t file = file_name_lookup (path.c_str (), O_NOTRANS, 0);
+	if (MACH_PORT_VALID (file)) {
+	    fsys_t fsys;
+	    int err = file_get_translator_cntl (file, &fsys);
+	    mach_port_deallocate (mach_task_self (), file);
+	    if (err == 0) {
+		/* There's a translator, tell it to leave.  */
+		err = fsys_goaway (fsys, FSYS_GOAWAY_FORCE | FSYS_GOAWAY_RECURSE);
+		mach_port_deallocate (mach_task_self (), fsys);
+		if (err != 0) {
+		    throw SysError(format("removing translator from '%1%'") % path);
+		}
+	    }
+	}
+    }
+#endif
+
 #ifdef HAVE_STATX
 # define st_mode stx_mode
 # define st_size stx_size
diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh
index 13cff44316..353c758895 100644
--- a/nix/libutil/util.hh
+++ b/nix/libutil/util.hh
@@ -62,6 +62,9 @@ Path readLink(const Path & path);
 
 bool isLink(const Path & path);
 
+/* Make TARGET a firmlink (aka. "bind mount") to SOURCE.  */
+int firmlink(const Path & source, const Path & target);
+
 /* Read the contents of a directory.  The entries `.' and `..' are
    removed. */
 struct DirEntry
diff --git a/nix/local.mk b/nix/local.mk
index 2bb01041b9..782e2c85cc 100644
--- a/nix/local.mk
+++ b/nix/local.mk
@@ -124,7 +124,8 @@ guix_daemon_CPPFLAGS =				\
 
 guix_daemon_LDADD =				\
   libstore.a libutil.a libformat.a -lz		\
-  $(SQLITE3_LIBS) $(LIBGCRYPT_LIBS)
+  $(SQLITE3_LIBS) $(LIBGCRYPT_LIBS)		\
+  $(HURD_LIBS)
 
 guix_daemon_headers =				\
   %D%/nix-daemon/shared.hh
-- 
2.28.0

[bug#43857] Supporting chroot builds on GNU/Hurd

[Jan Nieuwenhuizen]

Ludovic Courtès writes:

Hi!

> The patch below is an attempt at supporting “chroot builds” on GNU/Hurd;
> it’s “almost working”.  The main feature we need is firmlinks (or “bind
> mounts”) and commenting out Linux-specific things (private bind mounts,
> ‘pivot_root’, etc.).

Yay!  I finally got round to trying this, and I can confirm that it
also "almost works" for me.

[..]

> With this patch, I can run “guix build hello --check” in a chroot…  but
> it eventually hangs somewhere in ‘DerivationGoal::buildDone’ (I presume)
> once the build has completed.  It leaves behind it all its firmlink
> processes:

Yes, get something very similar.

> I felt a need to hack on this as I was investigating an util-linux test
> failure in a ‘--disable-chroot’ setup: the test would find /proc and
> would later fail for some reason.  Had /proc been missing from the build
> environment (as is the case with this patch), the test would have been
> skipped (it explicitly handles that case).  I think we’d rather not
> fiddle too much with test suites until we have defined what’s available
> in the default build environment.

I also tried building util-linux and comparing it with the non-chrooted
build:

--8<---------------cut here---------------start------------->8---
-checking whether make sets $(MAKE)... yes
+checking whether make sets $(MAKE)... no

-                : mountpoint                  ... FAILED (libmount/utils-mountpoint)
+                : mountpoint                  ... SKIPPED (no /proc)

-  3 tests of 204 FAILED
+  2 tests of 204 FAILED
--8<---------------cut here---------------end--------------->8---

Not sure about the configure change, probably it uses /proc to determine
that?

Still failing:
--8<---------------cut here---------------start------------->8---
        fdisk: invalid input tests            ... FAILED (fdisk/oddinput)
         ipcs: headers                        ... FAILED (ipcs/headers)
--8<---------------cut here---------------end--------------->8---

so, this is already better.

> Apart from that, this raises the question of what to put in the build
> environment.  As written in the blog post about childhurds that should
> go out tomorrow, on GNU/Linux, we do not include any piece of userland
> software in the environment.  But here, we’d be doing just that: running
> Hurd translators that are not specified as derivation inputs.  It’s OK
> for /dev/null, but maybe questionable for /servers/socket/*.

Yes, certainly maybe ;)

[..]

> Thoughts?

What about doing it in small steps, starting with the patch you suggest
here and see how much it "hurts" to go towards more secure/more Hurd'y
chrooted builds?

> From 1887d0dee0031df0de117b3a6339495504b4b489 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
> Date: Tue, 6 Oct 2020 23:53:24 +0200
> Subject: [PATCH] DRAFT daemon: Support chroot builds on GNU/Hurd.

So...apart from

> This has yet to be debugged.  :-)

otherwise, LGTM!

Thanks a lot for looking into this!

Greetings,
Janneke

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

[bug#43857] Supporting chroot builds on GNU/Hurd

[Ludovic Courtès]

Hi,

Jan Nieuwenhuizen <janneke@gnu.org> skribis:

> I also tried building util-linux and comparing it with the non-chrooted
> build:
>
> -checking whether make sets $(MAKE)... yes
> +checking whether make sets $(MAKE)... no
>
> -                : mountpoint                  ... FAILED (libmount/utils-mountpoint)
> +                : mountpoint                  ... SKIPPED (no /proc)
>
> -  3 tests of 204 FAILED
> +  2 tests of 204 FAILED
>
>
> Not sure about the configure change, probably it uses /proc to determine
> that?

Yes, the test has an explicit [ -d /proc ] and it is skipped when that’s
false.

Ludo’.