From 202683a4fc148dc228de226e9980a3f27754b854 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 9 Jun 2016 16:15:58 +0200 Subject: [PATCH] Use O_CLOEXEC in most places --- configure.ac | 2 +- src/libstore/build.cc | 3 +-- src/libstore/gc.cc | 5 ++--- src/libstore/local-store.cc | 4 ++-- src/libstore/pathlocks.cc | 4 +--- src/libstore/remote-store.cc | 6 +++++- src/libutil/archive.cc | 4 ++-- src/libutil/hash.cc | 2 +- src/libutil/util.cc | 12 ++++++++---- 9 files changed, 23 insertions(+), 19 deletions(-) diff --git a/configure.ac b/configure.ac index ff8e39fa0..a9e6b4313 100644 --- a/configure.ac +++ b/configure.ac @@ -80,7 +80,7 @@ static char buf[1024];]], AC_LANG_POP(C++) -AC_CHECK_FUNCS([statvfs]) +AC_CHECK_FUNCS([statvfs pipe2]) # Check for lutimes, optionally used for changing the mtime of diff --git a/src/libstore/build.cc b/src/libstore/build.cc index cbb4c4a75..15fff8a6b 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -509,10 +509,9 @@ void UserLock::acquire() /* We already have a lock on this one. */ continue; - AutoCloseFD fd = open(fnUserLock.c_str(), O_RDWR | O_CREAT, 0600); + AutoCloseFD fd = open(fnUserLock.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0600); if (fd == -1) throw SysError(format("opening user lock ‘%1%’") % fnUserLock); - closeOnExec(fd); if (lockFile(fd, ltWrite, false)) { fdUserLock = fd.borrow(); diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 77d13bbdc..986608d6b 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -33,10 +33,9 @@ int LocalStore::openGCLock(LockType lockType) debug(format("acquiring global GC lock ‘%1%’") % fnGCLock); - AutoCloseFD fdGCLock = open(fnGCLock.c_str(), O_RDWR | O_CREAT, 0600); + AutoCloseFD fdGCLock = open(fnGCLock.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0600); if (fdGCLock == -1) throw SysError(format("opening global GC lock ‘%1%’") % fnGCLock); - closeOnExec(fdGCLock); if (!lockFile(fdGCLock, lockType, false)) { printMsg(lvlError, format("waiting for the big garbage collector lock...")); @@ -211,7 +210,7 @@ void LocalStore::readTempRoots(PathSet & tempRoots, FDs & fds) Path path = (format("%1%/%2%/%3%") % stateDir % tempRootsDir % i.name).str(); debug(format("reading temporary root file ‘%1%’") % path); - FDPtr fd(new AutoCloseFD(open(path.c_str(), O_RDWR, 0666))); + FDPtr fd(new AutoCloseFD(open(path.c_str(), O_CLOEXEC | O_RDWR, 0666))); if (*fd == -1) { /* It's okay if the file has disappeared. */ if (errno == ENOENT) continue; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 67da5c1cf..409eb1a8a 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -117,7 +117,7 @@ LocalStore::LocalStore(const Params & params) if (stat(reservedPath.c_str(), &st) == -1 || st.st_size != settings.reservedSize) { - AutoCloseFD fd = open(reservedPath.c_str(), O_WRONLY | O_CREAT, 0600); + AutoCloseFD fd = open(reservedPath.c_str(), O_WRONLY | O_CREAT | O_CLOEXEC, 0600); int res = -1; #if HAVE_POSIX_FALLOCATE res = posix_fallocate(fd, 0, settings.reservedSize); @@ -1245,7 +1245,7 @@ static void makeMutable(const Path & path) /* The O_NOFOLLOW is important to prevent us from changing the mutable bit on the target of a symlink (which would be a security hole). */ - AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_NOFOLLOW); + AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_NOFOLLOW | O_CLOEXEC); if (fd == -1) { if (errno == ELOOP) return; // it's a symlink throw SysError(format("opening file ‘%1%’") % path); diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc index eddf5bcbd..d0a0f812e 100644 --- a/src/libstore/pathlocks.cc +++ b/src/libstore/pathlocks.cc @@ -16,12 +16,10 @@ int openLockFile(const Path & path, bool create) { AutoCloseFD fd; - fd = open(path.c_str(), O_RDWR | (create ? O_CREAT : 0), 0600); + fd = open(path.c_str(), O_CLOEXEC | O_RDWR | (create ? O_CREAT : 0), 0600); if (fd == -1 && (create || errno != ENOENT)) throw SysError(format("opening lock file ‘%1%’") % path); - closeOnExec(fd); - return fd.borrow(); } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 3654ffbff..50ad409a9 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -61,7 +61,11 @@ ref RemoteStore::openConnection() auto conn = make_ref(); /* Connect to a daemon that does the privileged work for us. */ - conn->fd = socket(PF_UNIX, SOCK_STREAM, 0); + conn->fd = socket(PF_UNIX, SOCK_STREAM + #ifdef SOCK_CLOEXEC + | SOCK_CLOEXEC + #endif + , 0); if (conn->fd == -1) throw SysError("cannot create Unix domain socket"); closeOnExec(conn->fd); diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 5363496c2..c3e4c87a5 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -41,7 +41,7 @@ static void dumpContents(const Path & path, size_t size, { sink << "contents" << size; - AutoCloseFD fd = open(path.c_str(), O_RDONLY); + AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); if (fd == -1) throw SysError(format("opening file ‘%1%’") % path); unsigned char buf[65536]; @@ -304,7 +304,7 @@ struct RestoreSink : ParseSink { Path p = dstPath + path; fd.close(); - fd = open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY, 0666); + fd = open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666); if (fd == -1) throw SysError(format("creating file ‘%1%’") % p); } diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index c17f1c4d5..69ea95852 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -254,7 +254,7 @@ Hash hashFile(HashType ht, const Path & path) Hash hash(ht); start(ht, ctx); - AutoCloseFD fd = open(path.c_str(), O_RDONLY); + AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); if (fd == -1) throw SysError(format("opening file ‘%1%’") % path); unsigned char buf[8192]; diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 788d01f59..4cc4649c9 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -273,7 +273,7 @@ string readFile(int fd) string readFile(const Path & path, bool drain) { - AutoCloseFD fd = open(path.c_str(), O_RDONLY); + AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); if (fd == -1) throw SysError(format("opening file ‘%1%’") % path); return drain ? drainFD(fd) : readFile(fd); @@ -282,7 +282,7 @@ string readFile(const Path & path, bool drain) void writeFile(const Path & path, const string & s) { - AutoCloseFD fd = open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT, 0666); + AutoCloseFD fd = open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, 0666); if (fd == -1) throw SysError(format("opening file ‘%1%’") % path); writeFull(fd, s); @@ -633,11 +633,15 @@ int AutoCloseFD::borrow() void Pipe::create() { int fds[2]; +#if HAVE_PIPE2 + if (pipe2(fds, O_CLOEXEC) != 0) throw SysError("creating pipe"); +#else if (pipe(fds) != 0) throw SysError("creating pipe"); + closeOnExec(fds[0]); + closeOnExec(fds[1]); +#endif readSide = fds[0]; writeSide = fds[1]; - closeOnExec(readSide); - closeOnExec(writeSide); }