From: Philipp Gesang Date: Mon, 12 Feb 2018 10:52:23 +0000 (+0100) Subject: distinguish child errors from failing to exec X-Git-Tag: v2.10~1^2~5 X-Git-Url: http://developer.intra2net.com/git/?a=commitdiff_plain;h=08199c66480e64cd7fa5e84bd8af5893191fda5f;p=libi2ncommon distinguish child errors from failing to exec Use the old pipe/cloexec trick to separate errors from the spawned program from such that occur before execv[p]e(). Only errors on the far side of exec() may be handled by the user. --- diff --git a/src/pipestream.cpp b/src/pipestream.cpp index 7816279..57b1dbe 100644 --- a/src/pipestream.cpp +++ b/src/pipestream.cpp @@ -210,22 +210,26 @@ mk_argv (const std::vector &command) * acquire one. * * @param fd The open file descriptor to operate on. + * @param save_errno Out parameter: stores errno here after a syscall failure. * * @returns \c true on success, \c false otherwise (the call to - * either \c open(2) or \c dup2(2) failed). + * either \c open(2) or \c dup2(2) failed), with errno + * communicated through saved_errno. */ static bool -redirect_devnull (const int fd) +redirect_devnull (const int fd, int &save_errno) { static int nullfd = -1; errno = 0; if (nullfd == -1 && (nullfd = open ("/dev/null", O_RDWR)) == -1) { + save_errno = errno; return false; } errno = 0; if (dup2 (nullfd, fd) == -1) { + save_errno = errno; return false; } @@ -248,7 +252,8 @@ inpipebuf::init_without_shell (const char *const *argv, const bool path) const { FILE *pipeobj = NULL; - int pipefd [2]; + int pipefd [2]; /* for reading output from the child */ + int errfd [2]; /* for determining a successful exec() */ sigset_t oldmask, newmask; if (!out && !err) { @@ -257,7 +262,8 @@ inpipebuf::init_without_shell (const char *const *argv, } errno = 0; - if (::pipe2 (pipefd, O_CLOEXEC) == -1) { + if ( ::pipe2 (pipefd, O_CLOEXEC) == -1 + || ::pipe2 (errfd , O_CLOEXEC) == -1) { PIPE_CTOR_FAIL("pipe2"); } @@ -274,27 +280,29 @@ inpipebuf::init_without_shell (const char *const *argv, } case 0: { close (pipefd [0]); + close (errfd [0]); fcntl (pipefd [1], F_SETFD, 0); + int save_errno = 0; if (!out) { - if (!redirect_devnull (STDOUT_FILENO)) { - fprintf(stderr, "redirect_devnull/stdout: %m\n"); - /* XXX should we bail here? */ + if (!redirect_devnull (STDOUT_FILENO, save_errno)) { + (void)write (errfd [1], (char *)&save_errno, sizeof(save_errno)); + exit (EXIT_FAILURE); } } else if (dup2 (pipefd[1], STDOUT_FILENO) == -1) { - fprintf(stderr, "dup2/stdout: %m\n"); - exit(EXIT_FAILURE); + (void)write (errfd [1], (char *)&save_errno, sizeof(save_errno)); + exit (EXIT_FAILURE); } if (!err) { - if (!redirect_devnull (STDERR_FILENO)) { - fprintf(stderr, "redirect_devnull/stderr: %m\n"); - /* XXX should we bail here? */ + if (!redirect_devnull (STDERR_FILENO, save_errno)) { + (void)write (errfd [1], (char *)&save_errno, sizeof(save_errno)); + exit (EXIT_FAILURE); } } else if (dup2 (pipefd[1], STDERR_FILENO) == -1) { - fprintf(stderr, "dup2/stderr: %m\n"); - exit(EXIT_FAILURE); + (void)write (errfd [1], (char *)&save_errno, sizeof(save_errno)); + exit (EXIT_FAILURE); } close (pipefd [1]); @@ -307,22 +315,65 @@ inpipebuf::init_without_shell (const char *const *argv, } else { execve (argv [0], const_cast (argv), NULL); } - exit(EXIT_FAILURE); + + (void)write (errfd [1], (char *)&errno, sizeof(errno)); + exit (EXIT_FAILURE); break; } default: { - close (pipefd [1]); - - sigprocmask (SIG_SETMASK, &oldmask, NULL); - - errno = 0; - if ((pipeobj = fdopen (pipefd [0], "r")) == NULL) { - PIPE_CTOR_FAIL("fdopen"); - } break; } } + close (pipefd [1]); + close (errfd [1]); + + /* + * Check whether the child exec()’ed by reading from the error pipe. + * The call to read(2) will block, uninterruptible due to signals being + * blocked. If all went well, the read(2) will return zero bytes and we can + * ditch the error channel. + * + * Otherwise either the read(2) failed or we actually received something + * through the error pipe. Both cases are treated as errors and cause an + * exit from the ctor. + */ + char buf [sizeof (errno)]; + int ret; + memset (buf, 0, sizeof (buf)); + errno = 0; + if ((ret = read (errfd [0], buf, sizeof (buf))) != 0) { + close (pipefd [0]); + close (errfd [0]); + sigprocmask (SIG_SETMASK, &oldmask, NULL); + if (ret == - 1) { + /* read(2) failed */ + PIPE_CTOR_FAIL("read"); + } else { + /* + * We received data on the error channel indicating the child + * process never successfully exec()’ed. We grab the error code + * from the buffer and bail. + */ + errno = *((int *)&buf[0]); + PIPE_CTOR_FAIL("child failed to exec()"); + } + } + + /* + * read(2) yielded zero bytes; it’s safe to use the pipe so close our end + * and continue. + */ + close (errfd [0]); + + sigprocmask (SIG_SETMASK, &oldmask, NULL); + + errno = 0; + if ((pipeobj = fdopen (pipefd [0], "r")) == NULL) { + close (pipefd [0]); + PIPE_CTOR_FAIL("fdopen"); + } + return std::make_pair (childpid, pipeobj); } diff --git a/test/test_pipestream.cpp b/test/test_pipestream.cpp index 1b4bf8e..b03d6c0 100644 --- a/test/test_pipestream.cpp +++ b/test/test_pipestream.cpp @@ -123,9 +123,12 @@ BOOST_AUTO_TEST_SUITE(pipestream) ExecResult exres = ExecResult (); const std::string result = capture_exec (bad_command, exres); - BOOST_CHECK(exres.normal_exit); + BOOST_CHECK(!exres.normal_exit); /* failed to exec() */ BOOST_CHECK(!exres.terminated_by_signal); BOOST_CHECK_EQUAL(result.size (), 0); + BOOST_CHECK_EQUAL(exres.error_message, + "child failed to exec(): " + "error 2 (No such file or directory)"); } BOOST_AUTO_TEST_CASE(abspath_bad_noshell_stderr) @@ -135,10 +138,12 @@ BOOST_AUTO_TEST_SUITE(pipestream) ExecResult exres = ExecResult (); const std::string result = capture_exec (bad_command, exres, false, true); - BOOST_CHECK(exres.normal_exit); + BOOST_CHECK(!exres.normal_exit); /* failed to exec() */ BOOST_CHECK(!exres.terminated_by_signal); - BOOST_CHECK_EQUAL(exres.return_code, EXIT_FAILURE); BOOST_CHECK_EQUAL(result.size (), 0); + BOOST_CHECK_EQUAL(exres.error_message, + "child failed to exec(): " + "error 2 (No such file or directory)"); } const char *const false_argv_abs [] = { "/bin/false", NULL }; @@ -185,9 +190,12 @@ BOOST_AUTO_TEST_SUITE(pipestream) const std::string result = capture_exec (true_argv_rel, exres, true, false, false); - BOOST_CHECK(exres.normal_exit); - /* no return code check since we couln't exit */ + BOOST_CHECK(!exres.normal_exit); /* failed to exec() */ + /* no return code check since we couldn't exit */ BOOST_CHECK_EQUAL(result.size (), 0); + BOOST_CHECK_EQUAL(exres.error_message, + "child failed to exec(): " + "error 2 (No such file or directory)"); } BOOST_AUTO_TEST_CASE(abspath_true_noshell_ok) @@ -208,7 +216,7 @@ BOOST_AUTO_TEST_SUITE(pipestream) capture_exec (false_argv_rel, exres, true, false, true); BOOST_CHECK(exres.normal_exit); - /* no return code check since we couln't exit */ + /* no return code check since we couldn't exit */ BOOST_CHECK_EQUAL(result.size (), 0); }