diff --git a/lix/libexec/meson.build b/lix/libexec/meson.build index 30e54d922..add540602 100644 --- a/lix/libexec/meson.build +++ b/lix/libexec/meson.build @@ -14,6 +14,13 @@ kill_user = executable( install_dir : libexecdir / 'lix', ) +run_build_hook = executable( + 'run-build-hook', + files('run-build-hook.cc'), + install : true, + install_dir : libexecdir / 'lix', +) + run_diff_hook = executable( 'run-diff-hook', files('run-diff-hook.cc'), diff --git a/lix/libexec/run-build-hook.cc b/lix/libexec/run-build-hook.cc new file mode 100644 index 000000000..0d57df3ab --- /dev/null +++ b/lix/libexec/run-build-hook.cc @@ -0,0 +1,17 @@ +#include "common.hh" +#include + +LIBEXEC_HELPER(2) + +int helperMain(const char * name, std::span args) noexcept +{ + DIE_UNLESS_SYS("chdir", chdir("/")); + DIE_UNLESS_SYS("setsid", setsid()); + + static_assert(STDIN_FILENO == 0); + DIE_UNLESS_SYS("close(stdin)", close(STDIN_FILENO)); + DIE_UNLESS_SYS("stdin = open(/dev/null)", open("/dev/null", O_RDWR)); + + execv(args[0], args.subspan(1).data()); + die("exec failed"); +} diff --git a/lix/libstore/build/hook-instance.cc b/lix/libstore/build/hook-instance.cc index 26c6c9e90..e95b2d411 100644 --- a/lix/libstore/build/hook-instance.cc +++ b/lix/libstore/build/hook-instance.cc @@ -15,6 +15,7 @@ #include "lix/libutil/logging-rpc.hh" // IWYU pragma: keep #include "lix/libutil/types-rpc.hh" // IWYU pragma: keep #include +#include #include #include #include @@ -74,32 +75,27 @@ try { throw Error("'build-hook' setting is empty"); auto buildHook = canonPath(buildHookArgs.front()); - buildHookArgs.pop_front(); + buildHookArgs.emplace(std::next(buildHookArgs.begin()), baseNameOf(buildHook)); + buildHookArgs.push_back(std::to_string(verbosity)); /* Create the communication pipes. */ auto [selfRPC, hookRPC] = SocketPair::stream(); - AutoCloseFD devNull(open("/dev/null", O_RDWR | O_CLOEXEC)); - if (!devNull) { - throw SysError("cannot open /dev/null"); - } - RunOptions options{ - .program = buildHook, - .searchPath = false, - .argv0 = std::string(baseNameOf(buildHook)), - .args = buildHookArgs, - .chdir = "/", - .createSession = true, - .redirections = { - {.dup = STDIN_FILENO, .from = devNull.get()}, - {.dup = STDOUT_FILENO, .from = hookRPC.get()}, - }, - }; - - options.args.push_back(std::to_string(verbosity)); - /* Fork the hook. */ - auto [pid, _stdout] = runProgram2(options).release(); + auto pid = runHelper( + "run-build-hook", + { + .args = buildHookArgs, + .redirections = {{.dup = STDOUT_FILENO, .from = hookRPC.get()}}, + } + ); + KJ_DEFER({ + // kill the hook if the promise is cancelled. the hook helper creates + // a session, so we'll kill the entire process group just to be safe. + if (pid) { + pid.killProcessGroup(); + } + }); std::map settings; globalConfig.getSettings(settings, true); @@ -116,7 +112,7 @@ try { } co_return std::make_unique( - kj::heap(std::move(rpc)).attach(std::move(conn), std::move(client)), ProcessGroup(std::move(pid)) + kj::heap(std::move(rpc)).attach(std::move(conn), std::move(client)), std::move(pid) ); } catch (...) { co_return result::current_exception(); diff --git a/lix/libstore/build/hook-instance.hh b/lix/libstore/build/hook-instance.hh index 3802109a8..59b4e8572 100644 --- a/lix/libstore/build/hook-instance.hh +++ b/lix/libstore/build/hook-instance.hh @@ -34,42 +34,41 @@ struct HookInstance static kj::Promise>> create(const Activity & act); - HookInstance(kj::Own rpc, ProcessGroup pg) + HookInstance(kj::Own rpc, RunningHelper hook) : rpc(std::move(rpc)) - , pgOrStatus(std::move(pg)) + , hookOrStatus(std::move(hook)) { } ~HookInstance(); int wait() { - return childStatusOr<&ProcessGroup::wait>(); + return childStatusOr([](auto & p) { return p.wait(); }); } int kill() { - return childStatusOr<&ProcessGroup::kill>(); + return childStatusOr([](auto & p) { return p.killProcessGroup(); }); } private: /** - * The process group of the hook if it's running, or its exit status if not. + * The process of the hook if it's running, or its exit status if not. */ - std::variant pgOrStatus; + std::variant hookOrStatus; - template - int childStatusOr() + int childStatusOr(auto ifRunning) { return std::visit( overloaded{ - [&](ProcessGroup & pg) { - int status = (pg.*fn)(); - pgOrStatus = status; + [&](RunningHelper & hook) { + int status = ifRunning(hook); + hookOrStatus = status; return status; }, [](int status) { return status; }, }, - pgOrStatus + hookOrStatus ); } }; diff --git a/lix/libutil/processes.cc b/lix/libutil/processes.cc index b5beecf06..5fefa9103 100644 --- a/lix/libutil/processes.cc +++ b/lix/libutil/processes.cc @@ -288,6 +288,11 @@ int RunningProgram::wait() return pid.wait(); } +int RunningHelper::killProcessGroup() +{ + return ProcessGroup{std::move(pid)}.kill(); +} + void RunningProgram::waitAndCheck() { if (std::uncaught_exceptions() == 0) { diff --git a/lix/libutil/processes.hh b/lix/libutil/processes.hh index a4fedca04..bb4eefaef 100644 --- a/lix/libutil/processes.hh +++ b/lix/libutil/processes.hh @@ -123,7 +123,7 @@ struct [[nodiscard("you must call RunningProgram::wait()")]] RunningProgram { friend RunningProgram runProgram2(const RunOptions & options); -private: +protected: Path program; Pid pid; std::unique_ptr childStdout; @@ -187,6 +187,12 @@ public: using RunningProgram::kill; using RunningProgram::wait; + /** + * Kill the entire process group the helper runs in. This is usually **unsafe** + * unless the helper process has made itself a process group or session leader! + */ + int killProcessGroup(); + void waitAndCheck(); };