Supporting chroot builds on GNU/Hurd

OpenSubmitted by Ludovic Courtès.
Details
2 participants
  • Jan Nieuwenhuizen
  • Ludovic Courtès
Owner
unassigned
Severity
normal
L
L
Ludovic Courtès wrote on 7 Oct 23:57 +0200
(address . guix-patches@gnu.org)
87sgapsnqz.fsf@gnu.org
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 “bindmounts”) and commenting out Linux-specific things (private bind mounts,‘pivot_root’, etc.).
The patch introduces a ‘firmlink’ function that creates a bind mount onGNU/Linux and otherwise spawns the /hurd/firmlink translator. It alsoadjusts ‘deletePath’ to remove any active translators from files it’sabout 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 toGNU/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… butit eventually hangs somewhere in ‘DerivationGoal::buildDone’ (I presume)once the build has completed. It leaves behind it all its firmlinkprocesses:
Toggle snippet (18 lines)root@childhurd ~# ps --width=200 aux|grep firmlinkroot 16426 120.0 0.0 147M 616K p1 RNm 11:38PM 0:00.00 grep --color=auto firmlinkroot 11191 0.0 0.0 147M 888K - S<o 11:27PM 0:00.00 /hurd/firmlink /dev/fullroot 11192 0.0 0.0 147M 936K - S<o 11:27PM 0:00.01 /hurd/firmlink /dev/nullroot 11193 0.0 0.0 147M 888K - S<o 11:27PM 0:00.00 /hurd/firmlink /dev/randomroot 11194 0.0 0.0 147M 936K - S<o 11:27PM 0:00.00 /hurd/firmlink /dev/urandomroot 11195 0.0 0.0 147M 888K - S<o 11:27PM 0:00.00 /hurd/firmlink /dev/zeroroot 11196 0.0 0.0 147M 936K - S<o 11:27PM 0:00.02 /hurd/firmlink /gnu/store/1brrgqhgjni8g8dbp97s8c7d7nq0p9a0-libgc-8.0.4root 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.91a5167root 11198 0.0 0.0 147M 936K - S<o 11:27PM 0:00.00 /hurd/firmlink /gnu/store/5ndcmqx0pzksq5c4mznbv20xhfnk3p97-zlib-1.2.11root 11199 0.0 0.0 147M 936K - S<o 11:27PM 0:00.02 /hurd/firmlink /gnu/store/631ixr5flpk66ziigbmkxwafz8skpyzs-findutils-4.7.0root 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.0root 11234 0.0 0.0 147M 936K - S<o 11:27PM 0:00.02 /hurd/firmlink /serversroot 11235 0.0 0.0 147M 936K - S<o 11:27PM 0:00.07 /hurd/firmlink /tmp/guix-build-hello-2.10.drv-6
This has yet to be debugged. :-)
Apart from that, this raises the question of what to put in the buildenvironment. As written in the blog post about childhurds that shouldgo out tomorrow, on GNU/Linux, we do not include any piece of userlandsoftware in the environment. But here, we’d be doing just that: runningHurd translators that are not specified as derivation inputs. It’s OKfor /dev/null, but maybe questionable for /servers/socket/*.
Regarding /servers, we’ll probably want to spawn separate servers (aseparate /servers/proc would give up a new “PID namespace”.) Shouldthat be done magically by the daemon, or should we leave it up to buildprocesses, so that build processes really specify all the user-landsoftware they depend on? We could imagine a new phase in‘gnu-build-system’, on GNU/Hurd, that would start by setting up a bunchof translators. Food for thought… As a first step, we can firmlink itfrom the host, with an eye towards “doing it right”.
I felt a need to hack on this as I was investigating an util-linux testfailure in a ‘--disable-chroot’ setup: the test would find /proc andwould later fail for some reason. Had /proc been missing from the buildenvironment (as is the case with this patch), the test would have beenskipped (it explicitly handles that case). I think we’d rather notfiddle too much with test suites until we have defined what’s availablein 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 thechildhurd, but I thought I’d share my weird workflow in case that givesme a chance to participate in some weirdness contest.
Ludo’.
From 1887d0dee0031df0de117b3a6339495504b4b489 Mon Sep 17 00:00:00 2001From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>Date: Tue, 6 Oct 2020 23:53:24 +0200Subject: [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 whenboth __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(-)
Toggle diff (337 lines)diff --git a/config-daemon.ac b/config-daemon.acindex 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.ccindex 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.ccindex 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_sizediff --git a/nix/libutil/util.hh b/nix/libutil/util.hhindex 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 DirEntrydiff --git a/nix/local.mk b/nix/local.mkindex 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
J
J
Jan Nieuwenhuizen wrote on 11 Oct 12:02 +0200
87imbhqdwk.fsf@gnu.org
Ludovic Courtès writes:
Hi!
Toggle quote (5 lines)> 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 italso "almost works" for me.
[..]
Toggle quote (5 lines)> 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.
Toggle quote (8 lines)> 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-chrootedbuild:
Toggle snippet (10 lines)-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 determinethat?
Still failing:
Toggle snippet (4 lines) fdisk: invalid input tests ... FAILED (fdisk/oddinput) ipcs: headers ... FAILED (ipcs/headers)
so, this is already better.
Toggle quote (7 lines)> 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 ;)
[..]
Toggle quote (2 lines)> Thoughts?
What about doing it in small steps, starting with the patch you suggesthere and see how much it "hurts" to go towards more secure/more Hurd'ychrooted builds?
Toggle quote (5 lines)> 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
Toggle quote (2 lines)> 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.orgFreelance IT http://JoyofSource.com| Avatar® http://AvatarAcademy.com
L
L
Ludovic Courtès wrote on 12 Oct 11:44 +0200
(name . Jan Nieuwenhuizen)(address . janneke@gnu.org)(address . 43857@debbugs.gnu.org)
87h7qzvkwn.fsf@gnu.org
Hi,
Jan Nieuwenhuizen <janneke@gnu.org> skribis:
Toggle quote (16 lines)> 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’sfalse.
Ludo’.
?