From d3fe6ab024df7764f4de2a9dcf88e2daa981f786 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 Dec 2006 00:19:50 +0000 Subject: [PATCH] * Also for convenience, change the ownership of the build output even in case of failure. --- src/libstore/build.cc | 58 +++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 9aeed6efa..033cc43d9 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -862,9 +862,17 @@ void DerivationGoal::buildDone() pid_t savedPid = pid; int status = pid.wait(true); + debug(format("builder process for `%1%' finished") % drvPath); + /* So the child is gone now. */ worker.childTerminated(savedPid); + /* Close the read side of the logger pipe. */ + logPipe.readSide.close(); + + /* Close the log file. */ + fdLogFile.close(); + /* When running under a build user, make sure that all processes running under that uid are gone. This is to prevent a malicious user from leaving behind a process that keeps files @@ -873,14 +881,36 @@ void DerivationGoal::buildDone() if (buildUser.enabled()) buildUser.kill(); - /* Close the read side of the logger pipe. */ - logPipe.readSide.close(); + /* Some cleanup per path. We do this here and not in + computeClosure() for convenience when the build has failed. */ + for (DerivationOutputs::iterator i = drv.outputs.begin(); + i != drv.outputs.end(); ++i) + { + Path path = i->second.path; + if (!pathExists(path)) continue; - /* Close the log file. */ - fdLogFile.close(); - - debug(format("builder process for `%1%' finished") % drvPath); + struct stat st; + if (lstat(path.c_str(), &st)) + throw SysError(format("getting attributes of path `%1%'") % path); + +#ifndef __CYGWIN__ + /* Check that the output is not group or world writable, as + that means that someone else can have interfered with the + build. Also, the output should be owned by the build + user. */ + if ((st.st_mode & (S_IWGRP | S_IWOTH)) || + (buildUser.enabled() && st.st_uid != buildUser.getUID())) + throw Error(format("suspicious ownership or permission on `%1%'; rejecting this build output") % path); +#endif + /* Gain ownership of the build result using the setuid wrapper + if we're not root. If we *are* root, then + canonicalisePathMetaData() will take care of this later + on. */ + if (buildUser.enabled() && !amPrivileged()) + getOwnership(path); + } + /* Check the exit status. */ if (!statusOk(status)) { deleteTmpDir(false); @@ -1560,22 +1590,6 @@ void DerivationGoal::computeClosure() % path % algo % printHash(h) % printHash(h2)); } -#ifndef __CYGWIN__ - /* Check that the output is not group or world writable, as - that means that someone else can have interfered with the - build. Also, the output should be owned by the build - user. */ - if ((st.st_mode & (S_IWGRP | S_IWOTH)) || - (buildUser.enabled() && st.st_uid != buildUser.getUID())) - throw Error(format("suspicious ownership or permission on `%1%'; rejecting this build output") % path); -#endif - - if (buildUser.enabled() && !amPrivileged()) - /* Call the setuid helper to change ownership from the - build user to our uid. If we *are* root, then - canonicalisePathMetaData() will take care of this. */ - getOwnership(path); - /* Get rid of all weird permissions. */ canonicalisePathMetaData(path);