From bfe82fe2f6e9f34c0774fe2114cdc7e937ba8bd2 Mon Sep 17 00:00:00 2001 From: Ludovic Courtès Date: Sat, 31 Oct 2020 23:02:33 +0100 Subject: pack: Relocatable wrapper leaves root available to child processes. Fixes . Reported by Jan Nieuwenhuizen . * gnu/packages/aux-files/run-in-namespace.c (exec_in_user_namespace): Add call to 'prctl'. Call 'mount' for NEW_ROOT and define 'is_tmpfs'. When IS_TMPFS is true, call 'umount' and 'rmdir' after 'waitpid'; otherwise, call 'rm_rf' only when 'waitpid' returns -1 the second time. (exec_with_loader): Call 'prctl'. Remove NEW_ROOT only when 'waitpid' returns -1 the second time, otherwise leave it behind. * tests/guix-pack-relocatable.sh (wait_for_file): New function. Add test. --- gnu/packages/aux-files/run-in-namespace.c | 52 ++++++++++++++++++---- tests/guix-pack-relocatable.sh | 73 +++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 8 deletions(-) diff --git a/gnu/packages/aux-files/run-in-namespace.c b/gnu/packages/aux-files/run-in-namespace.c index 947ff02dda..44c3c5a6cb 100644 --- a/gnu/packages/aux-files/run-in-namespace.c +++ b/gnu/packages/aux-files/run-in-namespace.c @@ -41,6 +41,7 @@ #include #include #include +#include /* 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,25 @@ 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); + + chdir ("/"); /* avoid EBUSY */ + 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) + rm_rf (new_root); + free (new_root); if (WIFEXITED (status)) @@ -490,6 +516,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,12 +536,19 @@ 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 */ if (WIFEXITED (status)) diff --git a/tests/guix-pack-relocatable.sh b/tests/guix-pack-relocatable.sh index eb04231d16..2beb1b1eb6 100644 --- a/tests/guix-pack-relocatable.sh +++ b/tests/guix-pack-relocatable.sh @@ -59,6 +59,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 @@ -131,6 +144,66 @@ 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 . + cat > "$test_directory/manifest.scm" <