* 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 public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).