Commit b258ee31 authored by Rahul Arunapuram Gokul's avatar Rahul Arunapuram Gokul Committed by Facebook GitHub Bot

provide an API to easily redirect stdin/stdout/stderr to /dev/null

Summary:
It's somewhat common to want to ignore `stdout` / `stderr` when spawning a child, and the current mechanism we provide to do this is `folly::Subprocess::CLOSE`.

However, this is somewhat dangerous when used without care, since any new files or sockets the child process opens will take on these low fds. From `man 2 open`,

> The file descriptor returned by a successful call will be the lowest-numbered file descriptor not currently open for the process.

Note that stdin/stdout/stderr are *not* special-cased here.

To provide a safer way to not inherit or pipe the standard fds, provide a `folly::Subprocess::DEV_NULL` option instead, which hooks up the OS-relevant null device to the fd.

`folly::Subprocess::CLOSE` will subsequently be removed.

Reviewed By: yfeldblum, luciang

Differential Revision: D30901400

fbshipit-source-id: 0b59de5da6c6a0f714233c09788fa2d0020f3823
parent 4d5b7ad4
......@@ -41,6 +41,7 @@
#include <folly/io/Cursor.h>
#include <folly/lang/Assume.h>
#include <folly/logging/xlog.h>
#include <folly/portability/Fcntl.h>
#include <folly/portability/Sockets.h>
#include <folly/portability/Stdlib.h>
#include <folly/portability/SysSyscall.h>
......@@ -553,11 +554,26 @@ int Subprocess::prepareChild(
// exec time.
// Close all fds that we're supposed to close.
// Redirect requested FDs to /dev/null or NUL.
for (auto& p : options.fdActions_) {
if (p.second == CLOSE) {
if (::close(p.first) == -1) {
return errno;
}
} else if (p.second == DEV_NULL) {
// folly/portability/Fcntl provides an impl of open that will
// map this to NUL on Windows.
auto devNull = ::open("/dev/null", O_RDWR | O_CLOEXEC);
if (devNull == -1) {
return errno;
}
// note: dup2 will not set CLOEXEC on the destination
if (::dup2(devNull, p.first) == -1) {
// explicit close on error to avoid leaking fds
::close(devNull);
return errno;
}
::close(devNull);
} else if (p.second != p.first) {
if (::dup2(p.second, p.first) == -1) {
return errno;
......
......@@ -253,10 +253,12 @@ class FOLLY_EXPORT SubprocessSpawnError : public SubprocessError {
*/
class Subprocess {
public:
// CLOSE is deprecated and will be removed, consider using DEV_NULL instead
static const int CLOSE = -1;
static const int PIPE = -2;
static const int PIPE_IN = -3;
static const int PIPE_OUT = -4;
static const int DEV_NULL = -5;
/**
* See Subprocess::Options::dangerousPostForkPreExecCallback() for usage.
......
......@@ -714,3 +714,36 @@ TEST(CommunicateSubprocessTest, TakeOwnershipOfPipes) {
buf[2] = 0;
EXPECT_EQ("3\n", std::string(buf));
}
TEST(CommunicateSubprocessTest, RedirectStdioToDevNull) {
std::vector<std::string> cmd({
"stat",
"-Lc",
"%t:%T",
"/dev/null",
"/dev/stdin",
"/dev/stderr",
});
auto options = Subprocess::Options()
.pipeStdout()
.stdinFd(folly::Subprocess::DEV_NULL)
.stderrFd(folly::Subprocess::DEV_NULL)
.usePath();
Subprocess proc(cmd, options);
auto out = proc.communicateIOBuf();
fbstring stdoutStr;
if (out.first.front()) {
stdoutStr = out.first.move()->moveToFbString();
}
LOG(ERROR) << stdoutStr;
std::vector<StringPiece> stdoutLines;
split('\n', stdoutStr, stdoutLines);
// 3 lines + empty string due to trailing newline
EXPECT_EQ(stdoutLines.size(), 4);
EXPECT_EQ(stdoutLines[0], stdoutLines[1]);
EXPECT_EQ(stdoutLines[0], stdoutLines[2]);
EXPECT_EQ(0, proc.wait().exitStatus());
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment