diff --git a/src/libpriv/rpmostree-bwrap.c b/src/libpriv/rpmostree-bwrap.c index 936caf30..05ff2b4b 100644 --- a/src/libpriv/rpmostree-bwrap.c +++ b/src/libpriv/rpmostree-bwrap.c @@ -294,44 +294,62 @@ rpmostree_bwrap_set_child_setup (RpmOstreeBwrap *bwrap, bwrap->child_setup_data = data; } +/* Execute @bwrap - must have been configured. After executing this method, the + * @bwrap instance cannot be run again. + */ gboolean rpmostree_bwrap_run (RpmOstreeBwrap *bwrap, + GCancellable *cancellable, GError **error) { - int estatus; - const char *current_lang = getenv ("LANG"); - g_assert (!bwrap->executed); bwrap->executed = TRUE; + const char *current_lang = getenv ("LANG"); if (!current_lang) current_lang = "C"; - { - const char *lang_var = glnx_strjoina ("LANG=", current_lang); - /* This is similar to what systemd does, except: - * - We drop /usr/local, since scripts shouldn't see it. - * - We pull in the current process' LANG, since that's what people - * have historically expected from RPM scripts. - */ - const char *bwrap_env[] = {"PATH=/usr/sbin:/usr/bin", - lang_var, - NULL}; + const char *lang_var = glnx_strjoina ("LANG=", current_lang); + /* This is similar to what systemd does, except: + * - We drop /usr/local, since scripts shouldn't see it. + * - We pull in the current process' LANG, since that's what people + * have historically expected from RPM scripts. + */ + const char *bwrap_env[] = {"PATH=/usr/sbin:/usr/bin", lang_var, NULL}; - /* Add the final NULL */ - g_ptr_array_add (bwrap->argv, NULL); + /* Set up our error message */ + const char *errmsg = glnx_strjoina ("Executing bwrap(", bwrap->child_argv0, ")"); + GLNX_AUTO_PREFIX_ERROR (errmsg, error); - const char *errmsg = glnx_strjoina ("Executing bwrap(", bwrap->child_argv0, ")"); - GLNX_AUTO_PREFIX_ERROR (errmsg, error); + /* Add the final NULL */ + g_ptr_array_add (bwrap->argv, NULL); - if (!g_spawn_sync (NULL, (char**)bwrap->argv->pdata, (char**) bwrap_env, - G_SPAWN_SEARCH_PATH, bwrap_child_setup, bwrap, NULL, NULL, - &estatus, error)) + g_autoptr(GSubprocessLauncher) launcher = + g_subprocess_launcher_new (G_SUBPROCESS_FLAGS_NONE); + g_subprocess_launcher_set_environ (launcher, (char**)bwrap_env); + g_subprocess_launcher_set_child_setup (launcher, bwrap_child_setup, bwrap, NULL); + g_autoptr(GSubprocess) subproc = + g_subprocess_launcher_spawnv (launcher, (const char *const*)bwrap->argv->pdata, + error); + if (!subproc) + return FALSE; + if (!g_subprocess_wait (subproc, cancellable, error)) + { + /* Now, it's possible @cancellable has been set, which means the process + * hasn't terminated yet. AFAIK that should be the only cause for the + * process not having exited now, but we just kill the process regardless + * on error here. The GSubprocess code ignores the request if we've + * already reaped it. + * + * Right now we run bwrap --die-with-parent, but until we do the whole txn + * as a subprocess, the script would leak until rpm-ostreed exited. + */ + g_subprocess_force_exit (subproc); return FALSE; - - if (!g_spawn_check_exit_status (estatus, error)) - return FALSE; - } + } + int estatus = g_subprocess_get_exit_status (subproc); + if (!g_spawn_check_exit_status (estatus, error)) + return FALSE; return TRUE; } @@ -352,7 +370,7 @@ rpmostree_bwrap_selftest (GError **error) rpmostree_bwrap_append_child_argv (bwrap, "true", NULL); - if (!rpmostree_bwrap_run (bwrap, error)) + if (!rpmostree_bwrap_run (bwrap, NULL, error)) { g_prefix_error (error, "bwrap test failed, see : "); return FALSE; diff --git a/src/libpriv/rpmostree-bwrap.h b/src/libpriv/rpmostree-bwrap.h index 77ecb9ed..6e09a267 100644 --- a/src/libpriv/rpmostree-bwrap.h +++ b/src/libpriv/rpmostree-bwrap.h @@ -51,6 +51,7 @@ void rpmostree_bwrap_set_child_setup (RpmOstreeBwrap *bwrap, gpointer data); gboolean rpmostree_bwrap_run (RpmOstreeBwrap *bwrap, + GCancellable *cancellable, GError **error); gboolean rpmostree_bwrap_selftest (GError **error); diff --git a/src/libpriv/rpmostree-kernel.c b/src/libpriv/rpmostree-kernel.c index c80c3d04..a68174aa 100644 --- a/src/libpriv/rpmostree-kernel.c +++ b/src/libpriv/rpmostree-kernel.c @@ -456,7 +456,7 @@ rpmostree_run_dracut (int rootfs_dfd, rpmostree_bwrap_set_child_setup (bwrap, dracut_child_setup, GINT_TO_POINTER (tmpf.fd)); - if (!rpmostree_bwrap_run (bwrap, error)) + if (!rpmostree_bwrap_run (bwrap, cancellable, error)) goto out; if (rebuild_from_initramfs) diff --git a/src/libpriv/rpmostree-postprocess.c b/src/libpriv/rpmostree-postprocess.c index d201da7a..3688b1c4 100644 --- a/src/libpriv/rpmostree-postprocess.c +++ b/src/libpriv/rpmostree-postprocess.c @@ -57,7 +57,8 @@ static gboolean run_bwrap_mutably (int rootfs_fd, const char *binpath, char **child_argv, - GError **error) + GCancellable *cancellable, + GError **error) { struct stat stbuf; const char *etc_bind; @@ -93,7 +94,7 @@ run_bwrap_mutably (int rootfs_fd, } } - if (!rpmostree_bwrap_run (bwrap, error)) + if (!rpmostree_bwrap_run (bwrap, cancellable, error)) return FALSE; return TRUE; @@ -324,7 +325,7 @@ process_kernel_and_initramfs (int rootfs_dfd, */ { char *child_argv[] = { "depmod", (char*)kver, NULL }; - if (!run_bwrap_mutably (rootfs_dfd, "depmod", child_argv, error)) + if (!run_bwrap_mutably (rootfs_dfd, "depmod", child_argv, cancellable, error)) return FALSE; } @@ -1605,7 +1606,7 @@ rpmostree_treefile_postprocessing (int rootfs_fd, { char *child_argv[] = { binpath, NULL }; - if (!run_bwrap_mutably (rootfs_fd, binpath, child_argv, error)) + if (!run_bwrap_mutably (rootfs_fd, binpath, child_argv, cancellable, error)) return glnx_prefix_error (error, "While executing postprocessing script '%s'", bn); } diff --git a/src/libpriv/rpmostree-scripts.c b/src/libpriv/rpmostree-scripts.c index 8a24e7d3..003423bf 100644 --- a/src/libpriv/rpmostree-scripts.c +++ b/src/libpriv/rpmostree-scripts.c @@ -313,7 +313,7 @@ run_script_in_bwrap_container (int rootfs_fd, script_arg, NULL); - if (!rpmostree_bwrap_run (bwrap, error)) + if (!rpmostree_bwrap_run (bwrap, cancellable, error)) { if (error) { @@ -819,7 +819,7 @@ rpmostree_deployment_sanitycheck (int rootfs_fd, if (!bwrap) return FALSE; rpmostree_bwrap_append_child_argv (bwrap, "/usr/bin/true", NULL); - if (!rpmostree_bwrap_run (bwrap, error)) + if (!rpmostree_bwrap_run (bwrap, cancellable, error)) return FALSE; sd_journal_print (LOG_INFO, "sanitycheck(/usr/bin/true) successful"); return TRUE; diff --git a/tests/common/libvm.sh b/tests/common/libvm.sh index 969c3e96..24acf6e0 100644 --- a/tests/common/libvm.sh +++ b/tests/common/libvm.sh @@ -345,6 +345,30 @@ vm_get_journal_cursor() { vm_cmd journalctl -o json -n 1 | jq -r '.["__CURSOR"]' } +# Wait for a message logged after $cursor matching a regexp to appear +vm_wait_content_after_cursor() { + from_cursor=$1; shift + regex=$1; shift + cat > wait.sh < \${tmpf} + if grep -q -e "${regex}" \${tmpf}; then + exit 0 + else + cat \${tmpf} + sleep 1 + fi +done +echo "timed out after 60s" 1>&2 +journalctl -u rpm-ostreed --after-cursor "${from_cursor}" | tail -100 +exit 1 +EOF + vm_cmdfile wait.sh +} + vm_assert_journal_has_content() { from_cursor=$1; shift # add an extra helping of quotes for hungry ssh diff --git a/tests/vmcheck/test-layering-scripts.sh b/tests/vmcheck/test-layering-scripts.sh index 41713b4d..10407eac 100755 --- a/tests/vmcheck/test-layering-scripts.sh +++ b/tests/vmcheck/test-layering-scripts.sh @@ -159,6 +159,33 @@ if vm_rpmostree install rofiles-violation; then assert_not_reached "installed test-post-rofiles-violation!" fi +# Test cancellation via having a script hang +cursor=$(vm_get_journal_cursor) +vm_build_rpm post-that-hangs \ + post "echo entering post-that-hangs-infloop 1>&2; while true; do sleep 1h; done" +# Enable job control so we can do a background job, then foreground. +set -m +nohup $SSH -t -t rpm-ostree install post-that-hangs & +if ! vm_wait_content_after_cursor "${cursor}" "entering post-that-hangs-infloop"; then + kill -s INT %1 + fg %1 || true + exit 1 +fi +# Explicitly kill the client process in the VM; I originally tried to kill the +# ssh binary and have it propagate the signal, on our side but couldn't figure +# out how to make that work. +vm_cmd pkill --signal INT -f "'rpm-ostree install post-that-hangs'" +# But do also kill the ssh binary on our side +kill -s INT %1 +# And wait for that to complete so it doesn't hang the shell +fg %1 || true +set +m +# Wait for our expected result +vm_wait_content_after_cursor "${cursor}" "Txn.*failed.*Running %post for post-that-hangs" +# Forcibly restart now to avoid any races with the txn finally exiting +vm_cmd systemctl restart rpm-ostreed + +# Test rm -rf /! vm_cmd 'useradd testuser || true' vm_cmd touch /home/testuser/somedata /tmp/sometmpfile /var/tmp/sometmpfile vm_build_rpm rmrf post "rm --no-preserve-root -rf / &>/dev/null || true"