diff --git a/Source/IO/AuIOHandle.Unix.cpp b/Source/IO/AuIOHandle.Unix.cpp index 5442da1c..4466b209 100644 --- a/Source/IO/AuIOHandle.Unix.cpp +++ b/Source/IO/AuIOHandle.Unix.cpp @@ -29,12 +29,7 @@ namespace Aurora::IO { - AuUInt64 AFileHandle::DupHandle(AuUInt64 uOSHandle, bool bWriteAccess) - { - return DupHandle(uOSHandle, bWriteAccess, false); - } - - AuUInt64 AFileHandle::DupHandle(AuUInt64 uOSHandle, bool bWriteAccess, bool bShareAccess) + AuUInt64 DupHandle(AuUInt64 uOSHandle, bool bWriteAccess, bool bShareAccess) { int fd = ::dup(uOSHandle); @@ -42,13 +37,34 @@ namespace Aurora::IO { return 0; } - - int flags = ::fcntl(fd, F_GETFL, 0); - if (flags == -1) + + if (!SetFDShareAccess(uOSHandle, bShareAccess)) { ::close(fd); return 0; } + + return AuUInt64(fd); + } + + AuUInt64 AFileHandle::DupHandle(AuUInt64 uOSHandle, bool bWriteAccess) + { + return Aurora::IO::DupHandle(uOSHandle, bWriteAccess, false); + } + + + AuUInt64 AFileHandle::DupHandle(AuUInt64 uOSHandle, bool bWriteAccess, bool bShareAccess) + { + return Aurora::IO::DupHandle(uOSHandle, bWriteAccess, bShareAccess); + } + + int SetFDShareAccess(AuUInt64 uOSHandle, bool bShareAccess) + { + int flags = ::fcntl(uOSHandle, F_GETFL, 0); + if (flags == -1) + { + return 0; + } if (bShareAccess) { @@ -59,14 +75,13 @@ namespace Aurora::IO flags |= FD_CLOEXEC; } - flags = ::fcntl(fd, F_SETFL, flags); + flags = ::fcntl(uOSHandle, F_SETFL, flags); if (flags == -1) { - ::close(fd); return 0; } - return AuUInt64(fd); + return 1; } void AFileHandle::CloseHandle(AuUInt64 uOSHandle) diff --git a/Source/IO/AuIOHandle.Unix.hpp b/Source/IO/AuIOHandle.Unix.hpp index 0d5df020..919ee2a3 100644 --- a/Source/IO/AuIOHandle.Unix.hpp +++ b/Source/IO/AuIOHandle.Unix.hpp @@ -9,5 +9,6 @@ namespace Aurora::IO { - + int SetFDShareAccess(AuUInt64 uOSHandle, bool bShareAccess); + AuUInt64 DupHandle(AuUInt64 uOSHandle, bool bWriteAccess, bool bShareAccess); } \ No newline at end of file diff --git a/Source/Processes/AuProcess.Unix.cpp b/Source/Processes/AuProcess.Unix.cpp index 1999ac68..e7742df7 100644 --- a/Source/Processes/AuProcess.Unix.cpp +++ b/Source/Processes/AuProcess.Unix.cpp @@ -17,6 +17,7 @@ #include #include +#include #if defined(AURORA_COMPILER_CLANG) // warning: enumeration values 'kEnumCount' and 'kEnumInvalid' not handled in switch [-Wswitch @@ -225,7 +226,8 @@ namespace Aurora::Processes do { tmp = ::read(handle, destination.ptr, destination.length); - } while ((tmp == -1 && errno == EINTR)); + } + while ((tmp == -1 && errno == EINTR)); if (tmp <= 0) { @@ -267,16 +269,62 @@ namespace Aurora::Processes ::fcntl(handle, F_SETFL, control); } - return ::write(handle, source.ptr, source.length) == source.length; } - static bool InitProcessStdHandles(EStreamForward fwd, int *fds) + static bool InitProcessStdHandles(EStreamForward fwd, int *fds, AuIO::EStandardStream stream, IO::IIOHandle *pHandle) { + AuOptionalEx optHandle; + bool bIsRead = stream == AuIO::EStandardStream::eInputStream; + switch (fwd) { + case EStreamForward::eIOHandle: + optHandle = pHandle->GetOSHandleSafe(); + + if (!optHandle) + { + return false; + } + + if (bIsRead) + { + fds[0] = dup((int)optHandle.value()); + } + else + { + fds[1] = dup((int)optHandle.value()); + } + break; case EStreamForward::eCurrentProcess: - // Intentionally NO-OP + if (stream == AuIO::EStandardStream::eInputStream) + { + fds[0] = dup(STDIN_FILENO); + if (fds[0] < 0) + { + return false; + } + } + else if (stream == AuIO::EStandardStream::eErrorStream) + { + fds[1] = dup(STDERR_FILENO); + if (fds[1] < 0) + { + return false; + } + } + else if (stream == AuIO::EStandardStream::eOutputStream) + { + fds[1] = dup(STDOUT_FILENO); + if (fds[1] < 0) + { + return false; + } + } + else + { + return false; + } break; case EStreamForward::eAsyncPipe: if (::pipe(fds)) @@ -286,22 +334,52 @@ namespace Aurora::Processes } break; case EStreamForward::eNull: - fds[0] = ::open("/dev/null", O_RDWR); - fds[1] = ::open("/dev/null", O_RDWR); + if (bIsRead) + { + fds[0] = ::open("/dev/null", O_RDWR); + } + else + { + fds[1] = ::open("/dev/null", O_RDWR); + } break; case EStreamForward::eNewConsoleWindow: SysPushErrorGeneric("AuProcesses is not the right place for PTY support. At least not in this form (this level of abstraction only cares for pipes)."); - break; + return false; } + if (fds[0] > 0) + { + if (!AuIO::SetFDShareAccess(fds[0], bIsRead)) + { + return false; + } + } + + if (fds[1] > 0) + { + if (!AuIO::SetFDShareAccess(fds[1], !bIsRead)) + { + return false; + } + } + + // Yea, I guess we can't really guard against threads racing to leak close on exec-less fds + // Let's just not care on UNIX. + // Worst case, we leak a daemons input/output FD to a different daemon of a differing permission level + // ...and somehow you can find that fd + // ...and probably dup it before ::start() + // ...and issue the right commands over the stream to do something bad + // Shouldn't need to worry about his for a while. + return true; } bool ProcessImpl::Init() { - InitProcessStdHandles(this->startup_.fwdOut, this->pipeStdOut_); - InitProcessStdHandles(this->startup_.fwdErr, this->pipeStdErr_); - InitProcessStdHandles(this->startup_.fwdIn, this->pipeStdIn_); + InitProcessStdHandles(this->startup_.fwdOut, this->pipeStdOut_, AuIO::EStandardStream::eOutputStream, this->startup_.handleOutStream); + InitProcessStdHandles(this->startup_.fwdErr, this->pipeStdErr_, AuIO::EStandardStream::eErrorStream, this->startup_.handleErrorStream); + InitProcessStdHandles(this->startup_.fwdIn, this->pipeStdIn_, AuIO::EStandardStream::eInputStream, this->startup_.handleInputStream); this->loopSource_ = AuMakeShared(); if (!this->loopSource_) @@ -331,7 +409,8 @@ namespace Aurora::Processes return false; } - this->fsHandle_->InitFromPairMove(this->pipeStdOut_[0], this->pipeStdIn_[1]); + this->fsHandle_->InitFromPairMove(this->pipeStdOut_[0] == -1 ? AuOptionalEx {} : AuOptionalEx { (AuUInt64)this->pipeStdOut_[0] } , + this->pipeStdIn_[1] == -1 ? AuOptionalEx {} : AuOptionalEx { (AuUInt64)this->pipeStdIn_[1] }); this->fsStream_->Init(this->fsHandle_); } @@ -349,7 +428,7 @@ namespace Aurora::Processes return false; } - this->fsErrorHandle_->InitFromPairMove(this->pipeStdErr_[0], -1); + this->fsErrorHandle_->InitFromPairMove(this->pipeStdErr_[0] == -1 ? AuOptionalEx {} : AuOptionalEx { (AuUInt64)this->pipeStdErr_[0] }, {}); this->fsErrorStream_->Init(this->fsErrorHandle_); } @@ -368,25 +447,19 @@ namespace Aurora::Processes void ProcessImpl::ForkMain() { - if (this->startup_.fwdIn != EStreamForward::eCurrentProcess) { - ::dup2(pipeStdIn_[0], STDIN_FILENO); - ::close(pipeStdIn_[0]); - ::close(pipeStdIn_[1]); + ::dup2(this->pipeStdIn_[0], STDIN_FILENO); + ::close(this->pipeStdIn_[0]); } - if (this->startup_.fwdErr != EStreamForward::eCurrentProcess) { - ::dup2(pipeStdErr_[1], STDERR_FILENO); - ::close(pipeStdIn_[0]); - ::close(pipeStdIn_[1]); + ::dup2(this->pipeStdErr_[1], STDERR_FILENO); + ::close(this->pipeStdErr_[1]); } - if (this->startup_.fwdOut != EStreamForward::eCurrentProcess) { - ::dup2(pipeStdOut_[1], STDOUT_FILENO); - ::close(pipeStdIn_[0]); - ::close(pipeStdIn_[1]); + ::dup2(this->pipeStdOut_[1], STDOUT_FILENO); + ::close(this->pipeStdOut_[1]); } #if defined(AURORA_IS_LINUX_DERIVED) @@ -421,8 +494,7 @@ namespace Aurora::Processes if (this->type_ == ESpawnType::eSpawnOvermap) { - ::execv(this->startup_.process.c_str(), (char *const *)this->cargs_.data()); // https://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html - SysPushErrorGen("execv didn't overwrite the process map, given {} ({})", this->startup_.process, this->debug_); + this->ForkMain(); return false; } diff --git a/Source/Processes/AuProcess.Unix.hpp b/Source/Processes/AuProcess.Unix.hpp index 256da95f..5c12b589 100644 --- a/Source/Processes/AuProcess.Unix.hpp +++ b/Source/Processes/AuProcess.Unix.hpp @@ -56,9 +56,9 @@ namespace Aurora::Processes private: - int pipeStdOut_[2]{}; - int pipeStdErr_[2]{}; - int pipeStdIn_ [2]{}; + int pipeStdOut_[2] { -1, -1 }; + int pipeStdErr_[2] { -1, -1 }; + int pipeStdIn_ [2] { -1, -1 }; bool bDontRelOut_ {}; bool bDontRelIn_ {};