Propagate errors from early sandbox initialization to the parent

This should help with issues like
https://github.com/DeterminateSystems/nix-installer/issues/1227, which
currently just print "unable to start build process".
This commit is contained in:
Eelco Dolstra 2024-10-09 20:53:43 +02:00
parent 26c3fc11ea
commit 0be70469dc
3 changed files with 81 additions and 35 deletions

View File

@ -433,6 +433,41 @@ static void doBind(const Path & source, const Path & target, bool optional = fal
}; };
#endif #endif
/**
* Rethrow the current exception as a subclass of `Error`.
*/
static void rethrowExceptionAsError()
{
try {
throw;
} catch (Error &) {
throw;
} catch (std::exception & e) {
throw Error(e.what());
} catch (...) {
throw Error("unknown exception");
}
}
/**
* Send the current exception to the parent in the format expected by
* `LocalDerivationGoal::processSandboxSetupMessages()`.
*/
static void handleChildException(bool sendException)
{
try {
rethrowExceptionAsError();
} catch (Error & e) {
if (sendException) {
writeFull(STDERR_FILENO, "\1\n");
FdSink sink(STDERR_FILENO);
sink << e;
sink.flush();
} else
std::cerr << e.msg();
}
}
void LocalDerivationGoal::startBuilder() void LocalDerivationGoal::startBuilder()
{ {
if ((buildUser && buildUser->getUIDCount() != 1) if ((buildUser && buildUser->getUIDCount() != 1)
@ -949,6 +984,7 @@ void LocalDerivationGoal::startBuilder()
root. */ root. */
openSlave(); openSlave();
try {
/* Drop additional groups here because we can't do it /* Drop additional groups here because we can't do it
after we've created the new user namespace. */ after we've created the new user namespace. */
if (setgroups(0, 0) == -1) { if (setgroups(0, 0) == -1) {
@ -969,12 +1005,19 @@ void LocalDerivationGoal::startBuilder()
writeFull(sendPid.writeSide.get(), fmt("%d\n", child)); writeFull(sendPid.writeSide.get(), fmt("%d\n", child));
_exit(0); _exit(0);
} catch (...) {
handleChildException(true);
_exit(1);
}
}); });
sendPid.writeSide.close(); sendPid.writeSide.close();
if (helper.wait() != 0) if (helper.wait() != 0) {
processSandboxSetupMessages();
// Only reached if the child process didn't send an exception.
throw Error("unable to start build process"); throw Error("unable to start build process");
}
userNamespaceSync.readSide = -1; userNamespaceSync.readSide = -1;
@ -1050,7 +1093,12 @@ void LocalDerivationGoal::startBuilder()
pid.setSeparatePG(true); pid.setSeparatePG(true);
worker.childStarted(shared_from_this(), {builderOut.get()}, true, true); worker.childStarted(shared_from_this(), {builderOut.get()}, true, true);
/* Check if setting up the build environment failed. */ processSandboxSetupMessages();
}
void LocalDerivationGoal::processSandboxSetupMessages()
{
std::vector<std::string> msgs; std::vector<std::string> msgs;
while (true) { while (true) {
std::string msg = [&]() { std::string msg = [&]() {
@ -1078,7 +1126,8 @@ void LocalDerivationGoal::startBuilder()
} }
void LocalDerivationGoal::initTmpDir() { void LocalDerivationGoal::initTmpDir()
{
/* In a sandbox, for determinism, always use the same temporary /* In a sandbox, for determinism, always use the same temporary
directory. */ directory. */
#if __linux__ #if __linux__
@ -2237,14 +2286,8 @@ void LocalDerivationGoal::runChild()
throw SysError("executing '%1%'", drv->builder); throw SysError("executing '%1%'", drv->builder);
} catch (Error & e) { } catch (...) {
if (sendException) { handleChildException(sendException);
writeFull(STDERR_FILENO, "\1\n");
FdSink sink(STDERR_FILENO);
sink << e;
sink.flush();
} else
std::cerr << e.msg();
_exit(1); _exit(1);
} }
} }

View File

@ -210,6 +210,11 @@ struct LocalDerivationGoal : public DerivationGoal
*/ */
void initEnv(); void initEnv();
/**
* Process messages send by the sandbox initialization.
*/
void processSandboxSetupMessages();
/** /**
* Setup tmp dir location. * Setup tmp dir location.
*/ */

View File

@ -9,7 +9,7 @@ needLocalStore "The test uses --store always so we would just be bypassing the d
TODO_NixOS TODO_NixOS
unshare --mount --map-root-user bash <<EOF unshare --mount --map-root-user -- bash -e -x <<EOF
source common.sh source common.sh
# Avoid store dir being inside sandbox build-dir # Avoid store dir being inside sandbox build-dir
@ -24,15 +24,13 @@ unshare --mount --map-root-user bash <<EOF
cmd=(nix-build ./hermetic.nix --arg busybox "$busybox" --arg seed 1 --no-out-link) cmd=(nix-build ./hermetic.nix --arg busybox "$busybox" --arg seed 1 --no-out-link)
# Fails with default setting # Fails with default setting
# TODO better error
setLocalStore store1 setLocalStore store1
expectStderr 1 "\${cmd[@]}" | grepQuiet "unable to start build process" expectStderr 1 "\${cmd[@]}" | grepQuiet "setgroups failed"
# Fails with `require-drop-supplementary-groups` # Fails with `require-drop-supplementary-groups`
# TODO better error
setLocalStore store2 setLocalStore store2
NIX_CONFIG='require-drop-supplementary-groups = true' \ NIX_CONFIG='require-drop-supplementary-groups = true' \
expectStderr 1 "\${cmd[@]}" | grepQuiet "unable to start build process" expectStderr 1 "\${cmd[@]}" | grepQuiet "setgroups failed"
# Works without `require-drop-supplementary-groups` # Works without `require-drop-supplementary-groups`
setLocalStore store3 setLocalStore store3