Merge pull request #8077 from edolstra/pts-hang

Open slave pseudoterminal before CLONE_NEWUSER
This commit is contained in:
Eelco Dolstra 2023-03-20 20:33:28 +01:00 committed by GitHub
commit 1de5b0e4e6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 38 additions and 27 deletions

View file

@ -35,7 +35,10 @@ HookInstance::HookInstance()
/* Fork the hook. */ /* Fork the hook. */
pid = startProcess([&]() { pid = startProcess([&]() {
commonChildInit(fromHook.writeSide.get()); if (dup2(fromHook.writeSide.get(), STDERR_FILENO) == -1)
throw SysError("cannot pipe standard error into log file");
commonChildInit();
if (chdir("/") == -1) throw SysError("changing into /"); if (chdir("/") == -1) throw SysError("changing into /");

View file

@ -827,6 +827,27 @@ void LocalDerivationGoal::startBuilder()
if (unlockpt(builderOut.get())) if (unlockpt(builderOut.get()))
throw SysError("unlocking pseudoterminal"); throw SysError("unlocking pseudoterminal");
/* Open the slave side of the pseudoterminal and use it as stderr. */
auto openSlave = [&]()
{
AutoCloseFD builderOut = open(slaveName.c_str(), O_RDWR | O_NOCTTY);
if (!builderOut)
throw SysError("opening pseudoterminal slave");
// Put the pt into raw mode to prevent \n -> \r\n translation.
struct termios term;
if (tcgetattr(builderOut.get(), &term))
throw SysError("getting pseudoterminal attributes");
cfmakeraw(&term);
if (tcsetattr(builderOut.get(), TCSANOW, &term))
throw SysError("putting pseudoterminal into raw mode");
if (dup2(builderOut.get(), STDERR_FILENO) == -1)
throw SysError("cannot pipe standard error into log file");
};
buildResult.startTime = time(0); buildResult.startTime = time(0);
/* Fork a child to build the package. */ /* Fork a child to build the package. */
@ -880,6 +901,11 @@ void LocalDerivationGoal::startBuilder()
Pid helper = startProcess([&]() { Pid helper = startProcess([&]() {
sendPid.readSide.close(); sendPid.readSide.close();
/* We need to open the slave early, before
CLONE_NEWUSER. Otherwise we get EPERM when running as
root. */
openSlave();
/* 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. FIXME: after we've created the new user namespace. FIXME:
this means that if we're not root in the parent this means that if we're not root in the parent
@ -898,7 +924,7 @@ void LocalDerivationGoal::startBuilder()
if (usingUserNamespace) if (usingUserNamespace)
options.cloneFlags |= CLONE_NEWUSER; options.cloneFlags |= CLONE_NEWUSER;
pid_t child = startProcess([&]() { runChild(slaveName); }, options); pid_t child = startProcess([&]() { runChild(); }, options);
writeFull(sendPid.writeSide.get(), fmt("%d\n", child)); writeFull(sendPid.writeSide.get(), fmt("%d\n", child));
_exit(0); _exit(0);
@ -974,7 +1000,8 @@ void LocalDerivationGoal::startBuilder()
#endif #endif
{ {
pid = startProcess([&]() { pid = startProcess([&]() {
runChild(slaveName); openSlave();
runChild();
}); });
} }
@ -1620,7 +1647,7 @@ void setupSeccomp()
} }
void LocalDerivationGoal::runChild(const Path & slaveName) void LocalDerivationGoal::runChild()
{ {
/* Warning: in the child we should absolutely not make any SQLite /* Warning: in the child we should absolutely not make any SQLite
calls! */ calls! */
@ -1629,22 +1656,7 @@ void LocalDerivationGoal::runChild(const Path & slaveName)
try { /* child */ try { /* child */
/* Open the slave side of the pseudoterminal. */ commonChildInit();
AutoCloseFD builderOut = open(slaveName.c_str(), O_RDWR | O_NOCTTY);
if (!builderOut)
throw SysError("opening pseudoterminal slave");
// Put the pt into raw mode to prevent \n -> \r\n translation.
struct termios term;
if (tcgetattr(builderOut.get(), &term))
throw SysError("getting pseudoterminal attributes");
cfmakeraw(&term);
if (tcsetattr(builderOut.get(), TCSANOW, &term))
throw SysError("putting pseudoterminal into raw mode");
commonChildInit(builderOut.get());
try { try {
setupSeccomp(); setupSeccomp();

View file

@ -169,7 +169,7 @@ struct LocalDerivationGoal : public DerivationGoal
int getChildStatus() override; int getChildStatus() override;
/* Run the builder's process. */ /* Run the builder's process. */
void runChild(const std::string & slaveName); void runChild();
/* Check that the derivation outputs all exist and register them /* Check that the derivation outputs all exist and register them
as valid. */ as valid. */

View file

@ -1968,7 +1968,7 @@ std::string showBytes(uint64_t bytes)
// FIXME: move to libstore/build // FIXME: move to libstore/build
void commonChildInit(int stderrFd) void commonChildInit()
{ {
logger = makeSimpleLogger(); logger = makeSimpleLogger();
@ -1982,10 +1982,6 @@ void commonChildInit(int stderrFd)
if (setsid() == -1) if (setsid() == -1)
throw SysError("creating a new session"); throw SysError("creating a new session");
/* Dup the write side of the logger pipe into stderr. */
if (dup2(stderrFd, STDERR_FILENO) == -1)
throw SysError("cannot pipe standard error into log file");
/* Dup stderr to stdout. */ /* Dup stderr to stdout. */
if (dup2(STDERR_FILENO, STDOUT_FILENO) == -1) if (dup2(STDERR_FILENO, STDOUT_FILENO) == -1)
throw SysError("cannot dup stderr into stdout"); throw SysError("cannot dup stderr into stdout");

View file

@ -704,7 +704,7 @@ typedef std::function<bool(const Path & path)> PathFilter;
extern PathFilter defaultPathFilter; extern PathFilter defaultPathFilter;
/* Common initialisation performed in child processes. */ /* Common initialisation performed in child processes. */
void commonChildInit(int stderrFd); void commonChildInit();
/* Create a Unix domain socket. */ /* Create a Unix domain socket. */
AutoCloseFD createUnixDomainSocket(); AutoCloseFD createUnixDomainSocket();