* bug#44261: running a daemon with userns in relocateble pack breaks @ 2020-10-27 19:49 Jan Nieuwenhuizen 2020-10-27 20:09 ` Jan Nieuwenhuizen 0 siblings, 1 reply; 6+ messages in thread From: Jan Nieuwenhuizen @ 2020-10-27 19:49 UTC (permalink / raw) To: 44261 [-- Attachment #1: Type: text/plain, Size: 1667 bytes --] Hi! As mentioned on IRC, running a daemon from a guix relocatable pack on a foreign distro using the user namespace feature is troublesome: it looks as if the daemon "loses" (its view of) the file-system once the parent process that creates the daemon exits. I'm attatching a package description for a test package "vork". It builds a program "test" that forks the program "daemon". The daemon program reads a character from /dev/urandom, prints it, and sleeps for a second; 10 times. The "test" parent program exits after 5 seconds. When the parent program exits, the daemon crashes. To reproduce, put "vork.scm" in a fresh directory and do something like: --8<---------------cut here---------------start------------->8--- fakeroot tar xf $(GUIX_PACKAGE_PATH=. guix pack --relocatable\ --symlink=/gnu/bin=bin guile shepherd vork --no-offload) guix gc -D $(guix build -f vork.scm) touch /tmp/daemon.log tail -f /tmp/daemon.log & GUILE_LOAD_COMPILED_PATH=$PWD/$(ls -1d gnu/store/*profile)/lib/guile/3.0/ccache\ :$PWD/$(ls -1d gnu/store/*profile)/lib/guile/3.0/site-ccache gnu/bin/test --8<---------------cut here---------------end--------------->8--- this gives something like --8<---------------cut here---------------start------------->8--- .daemon-start daemon: 10 ? .daemon: 9 ? .daemon: 8 T .daemon: 7 ^O .daemon: 6 O exit 20:42:38 janneke@dundal:~/src/guix/master/vork [env] $ 20:42:38 janneke@dundal:~/src/guix/master/vork [env] $ Backtrace: Exception thrown while printing backtrace: In procedure public-lookup: Module named (system repl debug) does not exist --8<---------------cut here---------------end--------------->8--- Greetings, Janneke [-- Attachment #2: vork.scm --] [-- Type: application/octet-stream, Size: 2424 bytes --] (define-module (vork) #:use-module (guix licenses) #:use-module (guix build-system trivial) #:use-module (guix packages) #:use-module (gnu packages admin) #:use-module (gnu packages guile) #:use-module (gnu packages pkg-config)) (define-public shepherd-guile-3.0-latest (package (inherit shepherd) (native-inputs `(("pkg-config" ,pkg-config) ("guile" ,guile-3.0-latest))) (inputs `(("guile" ,guile-3.0-latest))))) (define-public vork (package (name "vork") (version "0") (source #f) (build-system trivial-build-system) (inputs `(("guile" ,guile-3.0-latest) ("shepherd" ,shepherd-guile-3.0-latest))) (arguments `(#:guile ,guile-3.0-latest #:modules ((ice-9 popen) (guix build utils)) #:builder (begin (use-modules (ice-9 popen) (guix build utils)) (let* ((out (assoc-ref %outputs "out")) (bin (string-append out "/bin")) (guile (assoc-ref %build-inputs "guile")) (guile (string-append guile "/bin/guile")) (daemon (string-append bin "/daemon")) (test (string-append bin "/test"))) (mkdir-p bin) (call-with-output-file test (lambda (p) (format p "#! ~a --no-auto-compile\n" guile p) (format p "!# (use-modules (shepherd service)) (fork+exec-command (list ~s) #:log-file \"/tmp/daemon.log\") (let loop ((count 5)) (unless (zero? count) (display \".\") (sleep 1) (loop (1- count)))) (format #t \"\\nexit\\n\") " daemon))) (chmod test #o755) (call-with-output-file daemon (lambda (p) (format p "#! ~a --no-auto-compile\n" guile p) (display "!# (format #t \"daemon-start\\n\") (let loop ((count 10)) (unless (zero? count) (let ((char (with-input-from-file \"/dev/urandom\" read-char))) (format #t \"daemon: ~a ~a\\n\" count char) (force-output (current-output-port)) (call-with-output-file \"/dev/null\" (lambda (p) (format p \"daemon: ~a ~a\\n\" count char))) (sleep 1) (loop (1- count))))) (format #t \"\ndaemon-exit\\n\") " p))) (chmod daemon #o755)) #t))) (home-page "https://dezyne.org") (synopsis "vork") (description "vork") (license gpl3+))) vork [-- Attachment #3: Type: text/plain, Size: 152 bytes --] -- Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#44261: running a daemon with userns in relocateble pack breaks 2020-10-27 19:49 bug#44261: running a daemon with userns in relocateble pack breaks Jan Nieuwenhuizen @ 2020-10-27 20:09 ` Jan Nieuwenhuizen 2020-10-30 21:33 ` Ludovic Courtès 0 siblings, 1 reply; 6+ messages in thread From: Jan Nieuwenhuizen @ 2020-10-27 20:09 UTC (permalink / raw) To: 44261 [-- Attachment #1: Type: text/plain, Size: 202 bytes --] Jan Nieuwenhuizen writes: Hi! I tried the hint from Ludovic to use MS_PRIVATE in the attached patch and that works for me; not sure if we want a test and even less sure how to write that... Janneke [-- Attachment #2: 0001-pack-Support-running-of-daemons-in-user-namespace-ba.patch --] [-- Type: text/x-patch, Size: 1748 bytes --] From fd3104608c3fa6a2375b6c7df0862e5479976b39 Mon Sep 17 00:00:00 2001 From: "Jan (janneke) Nieuwenhuizen" <janneke@gnu.org> Date: Tue, 27 Oct 2020 20:55:11 +0100 Subject: [PATCH] pack: Support running of daemons in user namespace-based relocation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Add relocation via ld.so and fakechroot. Fixes <https://bugs.gnu.org/44261>. * gnu/packages/aux-files/run-in-namespace.c (bind_mount): Add 'MS_PRIVATE' to avoid unmounting the bind mount when parent process exits. Co-authored-by: Ludovic Courtès <ludo@gnu.org> --- gnu/packages/aux-files/run-in-namespace.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gnu/packages/aux-files/run-in-namespace.c b/gnu/packages/aux-files/run-in-namespace.c index 52a16a5362..67cea4fcd5 100644 --- a/gnu/packages/aux-files/run-in-namespace.c +++ b/gnu/packages/aux-files/run-in-namespace.c @@ -1,5 +1,6 @@ /* GNU Guix --- Functional package management for GNU Copyright (C) 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org> + Copyright (C) 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org> This file is part of GNU Guix. @@ -138,7 +139,7 @@ bind_mount (const char *source, const struct dirent *entry, close (open (target, O_WRONLY | O_CREAT)); return mount (source, target, "none", - MS_BIND | MS_REC | MS_RDONLY, NULL); + MS_BIND | MS_PRIVATE | MS_REC | MS_RDONLY, NULL); } #if HAVE_EXEC_WITH_LOADER -- Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com [-- Attachment #3: Type: text/plain, Size: 152 bytes --] -- Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com ^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#44261: running a daemon with userns in relocateble pack breaks 2020-10-27 20:09 ` Jan Nieuwenhuizen @ 2020-10-30 21:33 ` Ludovic Courtès 2020-10-30 22:05 ` Jan Nieuwenhuizen 0 siblings, 1 reply; 6+ messages in thread From: Ludovic Courtès @ 2020-10-30 21:33 UTC (permalink / raw) To: Jan Nieuwenhuizen; +Cc: 44261 [-- Attachment #1: Type: text/plain, Size: 813 bytes --] Hello! As discussed on IRC, my initial advice about MS_PRIVATE was misguided. The real issue is the “rm_rf (new_root);” call, which removes the root directory and thus leaves child processes (the daemon) with nothing. The attached patch adds a test loosely based on yours and a fix for that. The fix (for the “userns” engine) is to make NEW_ROOT a tmpfs, such that upon completion, all we need to do is to unmount it and remove it; it lives on as the root file system of child processes. In the “fakechroot” case, we have to leave NEW_ROOT behind, which is not great but acceptable (it’s user-owned, #o700, and it’s under /tmp). The test only checks the “userns” engine. If you confirm that it works for you and looks reasonable, we can apply it. Thanks, Ludo’. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Type: text/x-patch, Size: 6809 bytes --] diff --git a/gnu/packages/aux-files/run-in-namespace.c b/gnu/packages/aux-files/run-in-namespace.c index 52a16a5362..1d64ef9f44 100644 --- a/gnu/packages/aux-files/run-in-namespace.c +++ b/gnu/packages/aux-files/run-in-namespace.c @@ -41,6 +41,7 @@ #include <fcntl.h> #include <dirent.h> #include <sys/syscall.h> +#include <sys/prctl.h> /* Whether we're building the ld.so/libfakechroot wrapper. */ #define HAVE_EXEC_WITH_LOADER \ @@ -258,11 +259,20 @@ exec_in_user_namespace (const char *store, int argc, char *argv[]) { /* Spawn @WRAPPED_PROGRAM@ in a separate namespace where STORE is bind-mounted in the right place. */ - int err; + int err, is_tmpfs; char *new_root = mkdtemp (strdup ("/tmp/guix-exec-XXXXXX")); char *new_store = concat (new_root, original_store); char *cwd = get_current_dir_name (); + /* Become the new parent of grand-children when their parent dies. */ + prctl (PR_SET_CHILD_SUBREAPER, 1); + + /* Optionally, make NEW_ROOT a tmpfs. That way, if we have to leave it + behind because there are sub-processes still running when this wrapper + exits, it's OK. */ + err = mount ("none", new_root, "tmpfs", 0, NULL); + is_tmpfs = (err == 0); + /* Create a child with separate namespaces and set up bind-mounts from there. That way, bind-mounts automatically disappear when the child exits, which simplifies cleanup for the parent. Note: clone is more @@ -300,6 +310,7 @@ exec_in_user_namespace (const char *store, int argc, char *argv[]) /* Failure: user namespaces not supported. */ fprintf (stderr, "%s: error: 'clone' failed: %m\n", argv[0]); rm_rf (new_root); + free (new_root); break; default: @@ -312,10 +323,27 @@ exec_in_user_namespace (const char *store, int argc, char *argv[]) write_id_map (child, "uid_map", getuid ()); write_id_map (child, "gid_map", getgid ()); - int status; + int status, status_other; waitpid (child, &status, 0); - chdir ("/"); /* avoid EBUSY */ - rm_rf (new_root); + + if (is_tmpfs) + { + /* NEW_ROOT lives on in child processes and we no longer need it + to exist as an empty directory in the global namespace. */ + umount (new_root); + rmdir (new_root); + } + /* Check whether there are child processes left. If there are none, + we can remove NEW_ROOT just fine. Conversely, if there are + processes left (for example because this wrapper's child forked), + we have to leave NEW_ROOT behind so that those processes can still + access their root file system (XXX). */ + else if (waitpid (-1 , &status_other, WNOHANG) == -1) + { + chdir ("/"); /* avoid EBUSY */ + rm_rf (new_root); + } + free (new_root); if (WIFEXITED (status)) @@ -490,6 +518,9 @@ exec_with_loader (const char *store, int argc, char *argv[]) setenv ("FAKECHROOT_BASE", new_root, 1); + /* Become the new parent of grand-children when their parent dies. */ + prctl (PR_SET_CHILD_SUBREAPER, 1); + pid_t child = fork (); switch (child) { @@ -507,11 +538,18 @@ exec_with_loader (const char *store, int argc, char *argv[]) default: { - int status; + int status, status_other; waitpid (child, &status, 0); - chdir ("/"); /* avoid EBUSY */ - rm_rf (new_root); - free (new_root); + + /* If there are child processes still running, leave NEW_ROOT around + so they can still access it. XXX: In that case NEW_ROOT is left + behind. */ + if (waitpid (-1 , &status_other, WNOHANG) == -1) + { + chdir ("/"); /* avoid EBUSY */ + rm_rf (new_root); + free (new_root); + } close (2); /* flushing stderr should be silent */ diff --git a/tests/guix-pack-relocatable.sh b/tests/guix-pack-relocatable.sh index a960ecd209..88cbe63b59 100644 --- a/tests/guix-pack-relocatable.sh +++ b/tests/guix-pack-relocatable.sh @@ -58,6 +58,19 @@ run_without_store () fi } +# Wait for the given file to show up. Error out if it doesn't show up in a +# timely fashion. +wait_for_file () +{ + i=0 + while ! test -f "$1" && test $i -lt 20 + do + sleep 0.3 + i=`expr $i + 1` + done + test -f "$1" +} + test_directory="`mktemp -d`" export test_directory trap 'chmod -Rf +w "$test_directory"; rm -rf "$test_directory"' EXIT @@ -129,6 +142,65 @@ case "`uname -m`" in ;; esac +if unshare -r true +then + # Check what happens if the wrapped binary forks and leaves child + # processes behind, like a daemon. The root file system should remain + # available to those child processes. See <https://bugs.gnu.org/44261>. + cat > "$test_directory/manifest.scm" <<EOF +(use-modules (guix)) + +(define daemon + (program-file "daemon" + #~(begin + (use-modules (ice-9 match) + (ice-9 ftw)) + + (call-with-output-file "parent-store" + (lambda (port) + (write (scandir (ungexp (%store-prefix))) + port))) + + (match (primitive-fork) + (0 (sigaction SIGHUP (const #t)) + (call-with-output-file "pid" + (lambda (port) + (display (getpid) port))) + (pause) + (call-with-output-file "child-store" + (lambda (port) + (write (scandir (ungexp (%store-prefix))) + port)))) + (_ #t))))) + +(define package + (computed-file "package" + #~(let ((out (ungexp output))) + (mkdir out) + (mkdir (string-append out "/bin")) + (symlink (ungexp daemon) + (string-append out "/bin/daemon"))))) + +(manifest (list (manifest-entry + (name "daemon") + (version "0") + (item package)))) +EOF + + tarball="$(guix pack -S /bin=bin -R -m "$test_directory/manifest.scm")" + (cd "$test_directory"; tar xf "$tarball") + + # Run '/bin/daemon', which forks, then wait for the child, send it SIGHUP + # so that it dumps its view of the store, and make sure the child and + # parent both see the same store contents. + (cd "$test_directory"; run_without_store ./bin/daemon) + wait_for_file "$test_directory/pid" + kill -HUP $(cat "$test_directory/pid") + diff -u "$test_directory/parent-store" "$test_directory/child-store" + + chmod -Rf +w "$test_directory"; rm -rf "$test_directory"/* +fi + # Ensure '-R' works with outputs other than "out". tarball="`guix pack -R -S /share=share groff:doc`" (cd "$test_directory"; tar xf "$tarball") ^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#44261: running a daemon with userns in relocateble pack breaks 2020-10-30 21:33 ` Ludovic Courtès @ 2020-10-30 22:05 ` Jan Nieuwenhuizen 2020-10-31 22:19 ` Ludovic Courtès 0 siblings, 1 reply; 6+ messages in thread From: Jan Nieuwenhuizen @ 2020-10-30 22:05 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 44261 Ludovic Courtès writes: Hi! > As discussed on IRC, my initial advice about MS_PRIVATE was misguided. > The real issue is the “rm_rf (new_root);” call, which removes the root > directory and thus leaves child processes (the daemon) with nothing. Yes, I'm not entirely sure what I thought to see yesterday; anyway the rm_rf (new_root) is indeed the thing that makes the daemon crash. > The attached patch adds a test loosely based on yours and a fix for > that. The fix (for the “userns” engine) is to make NEW_ROOT a tmpfs, > such that upon completion, all we need to do is to unmount it and remove > it; it lives on as the root file system of child processes. > > In the “fakechroot” case, we have to leave NEW_ROOT behind, which is not > great but acceptable (it’s user-owned, #o700, and it’s under /tmp). The > test only checks the “userns” engine. Yes, I think this is acceptable. > If you confirm that it works for you and looks reasonable, we can apply > it. Yes, this works. The test and also my reproducer now work fine. Thanks a lot! Janneke -- Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#44261: running a daemon with userns in relocateble pack breaks 2020-10-30 22:05 ` Jan Nieuwenhuizen @ 2020-10-31 22:19 ` Ludovic Courtès 2020-11-01 6:07 ` Jan Nieuwenhuizen 0 siblings, 1 reply; 6+ messages in thread From: Ludovic Courtès @ 2020-10-31 22:19 UTC (permalink / raw) To: Jan Nieuwenhuizen; +Cc: 44261-done Hi, Jan Nieuwenhuizen <janneke@gnu.org> skribis: > Ludovic Courtès writes: [...] >> The attached patch adds a test loosely based on yours and a fix for >> that. The fix (for the “userns” engine) is to make NEW_ROOT a tmpfs, >> such that upon completion, all we need to do is to unmount it and remove >> it; it lives on as the root file system of child processes. >> >> In the “fakechroot” case, we have to leave NEW_ROOT behind, which is not >> great but acceptable (it’s user-owned, #o700, and it’s under /tmp). The >> test only checks the “userns” engine. > > Yes, I think this is acceptable. > >> If you confirm that it works for you and looks reasonable, we can apply >> it. > > Yes, this works. The test and also my reproducer now work fine. Thanks for checking, I pushed the fix as bfe82fe2f6e9f34c0774fe2114cdc7e937ba8bd2. Ludo’. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#44261: running a daemon with userns in relocateble pack breaks 2020-10-31 22:19 ` Ludovic Courtès @ 2020-11-01 6:07 ` Jan Nieuwenhuizen 0 siblings, 0 replies; 6+ messages in thread From: Jan Nieuwenhuizen @ 2020-11-01 6:07 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 44261-done Ludovic Courtès writes: Hello, > Jan Nieuwenhuizen <janneke@gnu.org> skribis: > >> Ludovic Courtès writes: > > [...] > >>> If you confirm that it works for you and looks reasonable, we can apply >>> it. >> >> Yes, this works. The test and also my reproducer now work fine. > > Thanks for checking, I pushed the fix as > bfe82fe2f6e9f34c0774fe2114cdc7e937ba8bd2. \o/ Thank you Janneke. -- Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-01 6:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-27 19:49 bug#44261: running a daemon with userns in relocateble pack breaks Jan Nieuwenhuizen 2020-10-27 20:09 ` Jan Nieuwenhuizen 2020-10-30 21:33 ` Ludovic Courtès 2020-10-30 22:05 ` Jan Nieuwenhuizen 2020-10-31 22:19 ` Ludovic Courtès 2020-11-01 6:07 ` Jan Nieuwenhuizen
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/guix.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.