From 352ca238a909fd2b81f5f5b58cac6892a03d5096 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 26 May 2025 23:51:24 +0200 Subject: [PATCH] Move cgroup support --- .../build/derivation-building-goal.cc | 2 + src/libstore/unix/build/derivation-builder.cc | 85 +++---------------- .../unix/build/linux-derivation-builder.cc | 73 +++++++++++++++- 3 files changed, 83 insertions(+), 77 deletions(-) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 2b5473b81..19296fac3 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -86,6 +86,8 @@ void DerivationBuildingGoal::killChild() if (builder && builder->pid != -1) { worker.childTerminated(this); + // FIXME: move this into DerivationBuilder. + /* If we're using a build user, then there is a tricky race condition: if we kill the build user before the child has done its setuid() to the build user uid, then it won't be diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index bc1ca959a..142f75d14 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -32,10 +32,6 @@ # include #endif -#ifdef __linux__ -# include "nix/util/cgroup.hh" -#endif - #include #include #include @@ -88,12 +84,6 @@ protected: */ std::unique_ptr buildUser; - /** - * The cgroup of the builder, if any. - */ - // FIXME: move - std::optional cgroup; - /** * The temporary directory used for the build. */ @@ -236,6 +226,15 @@ protected: return topTmpDir; } + /** + * Ensure that there are no processes running that conflict with + * `buildUser`. + */ + virtual void prepareUser() + { + killSandbox(false); + } + /** * Called by prepareBuild() to do any setup in the parent to * prepare for a sandboxed build. @@ -422,19 +421,7 @@ static LocalStore & getLocalStore(Store & store) void DerivationBuilderImpl::killSandbox(bool getStats) { - if (cgroup) { - #ifdef __linux__ - auto stats = destroyCgroup(*cgroup); - if (getStats) { - buildResult.cpuUser = stats.cpuUser; - buildResult.cpuSystem = stats.cpuSystem; - } - #else - unreachable(); - #endif - } - - else if (buildUser) { + if (buildUser) { auto uid = buildUser->getUID(); assert(uid != 0); killUser(uid); @@ -693,60 +680,10 @@ static void handleChildException(bool sendException) void DerivationBuilderImpl::startBuilder() { - if ((buildUser && buildUser->getUIDCount() != 1) - #ifdef __linux__ - || settings.useCgroups - #endif - ) - { - #ifdef __linux__ - experimentalFeatureSettings.require(Xp::Cgroups); - - /* If we're running from the daemon, then this will return the - root cgroup of the service. Otherwise, it will return the - current cgroup. */ - auto rootCgroup = getRootCgroup(); - auto cgroupFS = getCgroupFS(); - if (!cgroupFS) - throw Error("cannot determine the cgroups file system"); - auto rootCgroupPath = canonPath(*cgroupFS + "/" + rootCgroup); - if (!pathExists(rootCgroupPath)) - throw Error("expected cgroup directory '%s'", rootCgroupPath); - - static std::atomic counter{0}; - - cgroup = buildUser - ? fmt("%s/nix-build-uid-%d", rootCgroupPath, buildUser->getUID()) - : fmt("%s/nix-build-pid-%d-%d", rootCgroupPath, getpid(), counter++); - - debug("using cgroup '%s'", *cgroup); - - /* When using a build user, record the cgroup we used for that - user so that if we got interrupted previously, we can kill - any left-over cgroup first. */ - if (buildUser) { - auto cgroupsDir = settings.nixStateDir + "/cgroups"; - createDirs(cgroupsDir); - - auto cgroupFile = fmt("%s/%d", cgroupsDir, buildUser->getUID()); - - if (pathExists(cgroupFile)) { - auto prevCgroup = readFile(cgroupFile); - destroyCgroup(prevCgroup); - } - - writeFile(cgroupFile, *cgroup); - } - - #else - throw Error("cgroups are not supported on this platform"); - #endif - } - /* Make sure that no other processes are executing under the sandbox uids. This must be done before any chownToBuilder() calls. */ - killSandbox(false); + prepareUser(); /* Right platform? */ if (!drvOptions.canBuildLocally(store, drv)) { diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 5c6c80a0e..8d4d973e1 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -1,6 +1,10 @@ #ifdef __linux__ +# include "nix/store/personality.hh" +# include "nix/util/cgroup.hh" +# include "nix/util/namespaces.hh" # include "linux/fchmodat2-compat.hh" + # include # include # include @@ -9,13 +13,12 @@ # include # include # include -# include "nix/util/namespaces.hh" + # if HAVE_SECCOMP # include # endif + # define pivot_root(new_root, put_old) (syscall(SYS_pivot_root, new_root, put_old)) -# include "nix/util/cgroup.hh" -# include "nix/store/personality.hh" namespace nix { @@ -182,6 +185,11 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl PathsInChroot pathsInChroot; + /** + * The cgroup of the builder, if any. + */ + std::optional cgroup; + LinuxDerivationBuilder( Store & store, std::unique_ptr miscMethods, DerivationBuilderParams params) : DerivationBuilderImpl(store, std::move(miscMethods), std::move(params)) @@ -235,6 +243,51 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl return settings.sandboxBuildDir; } + void prepareUser() override + { + if ((buildUser && buildUser->getUIDCount() != 1) || settings.useCgroups) { + experimentalFeatureSettings.require(Xp::Cgroups); + + /* If we're running from the daemon, then this will return the + root cgroup of the service. Otherwise, it will return the + current cgroup. */ + auto rootCgroup = getRootCgroup(); + auto cgroupFS = getCgroupFS(); + if (!cgroupFS) + throw Error("cannot determine the cgroups file system"); + auto rootCgroupPath = canonPath(*cgroupFS + "/" + rootCgroup); + if (!pathExists(rootCgroupPath)) + throw Error("expected cgroup directory '%s'", rootCgroupPath); + + static std::atomic counter{0}; + + cgroup = buildUser ? fmt("%s/nix-build-uid-%d", rootCgroupPath, buildUser->getUID()) + : fmt("%s/nix-build-pid-%d-%d", rootCgroupPath, getpid(), counter++); + + debug("using cgroup '%s'", *cgroup); + + /* When using a build user, record the cgroup we used for that + user so that if we got interrupted previously, we can kill + any left-over cgroup first. */ + if (buildUser) { + auto cgroupsDir = settings.nixStateDir + "/cgroups"; + createDirs(cgroupsDir); + + auto cgroupFile = fmt("%s/%d", cgroupsDir, buildUser->getUID()); + + if (pathExists(cgroupFile)) { + auto prevCgroup = readFile(cgroupFile); + destroyCgroup(prevCgroup); + } + + writeFile(cgroupFile, *cgroup); + } + } + + // Kill any processes left in the cgroup or build user. + DerivationBuilderImpl::prepareUser(); + } + void prepareSandbox() override { /* Create a temporary directory in which we set up the chroot @@ -747,6 +800,20 @@ struct LinuxDerivationBuilder : DerivationBuilderImpl return DerivationBuilderImpl::unprepareBuild(); } + void killSandbox(bool getStats) override + { + if (cgroup) { + auto stats = destroyCgroup(*cgroup); + if (getStats) { + buildResult.cpuUser = stats.cpuUser; + buildResult.cpuSystem = stats.cpuSystem; + } + return; + } + + DerivationBuilderImpl::killSandbox(getStats); + } + void cleanupBuild() override { DerivationBuilderImpl::cleanupBuild();