Merge "logwrapper: open child_ptty in child process and remove ignore_int_quit"

This commit is contained in:
Tom Cherry 2019-09-26 13:52:25 +00:00 committed by Gerrit Code Review
commit 7d91385cf5
3 changed files with 117 additions and 65 deletions

View file

@ -15,8 +15,7 @@
* limitations under the License.
*/
#ifndef __LIBS_LOGWRAP_H
#define __LIBS_LOGWRAP_H
#pragma once
#include <stdbool.h>
#include <stdint.h>
@ -26,12 +25,6 @@ __BEGIN_DECLS
/*
* Run a command while logging its stdout and stderr
*
* WARNING: while this function is running it will clear all SIGCHLD handlers
* if you rely on SIGCHLD in the caller there is a chance zombies will be
* created if you're not calling waitpid after calling this. This function will
* log a warning when it clears SIGCHLD for processes other than the child it
* created.
*
* Arguments:
* argc: the number of elements in argv
* argv: an array of strings containing the command to be executed and its
@ -40,10 +33,10 @@ __BEGIN_DECLS
* status: the equivalent child status as populated by wait(status). This
* value is only valid when logwrap successfully completes. If NULL
* the return value of the child will be the function's return value.
* ignore_int_quit: set to true if you want to completely ignore SIGINT and
* SIGQUIT while logwrap is running. This may force the end-user to
* send a signal twice to signal the caller (once for the child, and
* once for the caller)
* forward_signals: set to true if you want to forward SIGINT, SIGQUIT, and
* SIGHUP to the child process, while it is running. You likely do
* not need to use this; it is primarily for the logwrapper
* executable itself.
* log_target: Specify where to log the output of the child, either LOG_NONE,
* LOG_ALOG (for the Android system log), LOG_KLOG (for the kernel
* log), or LOG_FILE (and you need to specify a pathname in the
@ -54,8 +47,6 @@ __BEGIN_DECLS
* the specified log until the child has exited.
* file_path: if log_target has the LOG_FILE bit set, then this parameter
* must be set to the pathname of the file to log to.
* unused_opts: currently unused.
* unused_opts_len: currently unused.
*
* Return value:
* 0 when logwrap successfully run the child process and captured its status
@ -71,10 +62,18 @@ __BEGIN_DECLS
#define LOG_KLOG 2
#define LOG_FILE 4
// TODO: Remove unused_opts / unused_opts_len in a followup change.
int android_fork_execvp_ext(int argc, char* argv[], int *status, bool ignore_int_quit,
int log_target, bool abbreviated, char *file_path, void* unused_opts,
int unused_opts_len);
int android_fork_execvp_ext2(int argc, char* argv[], int* status, bool forward_signals,
int log_target, bool abbreviated, char* file_path);
// TODO: Actually deprecate this and the below.
static inline int android_fork_execvp_ext(int argc, char* argv[], int* status, bool ignore_int_quit,
int log_target, bool abbreviated, char* file_path,
void* unused_opts, int unused_opts_len) {
(void)ignore_int_quit;
(void)unused_opts;
(void)unused_opts_len;
return android_fork_execvp_ext2(argc, argv, status, false, log_target, abbreviated, file_path);
}
/* Similar to above, except abbreviated logging is not available, and if logwrap
* is true, logging is to the Android system log, and if false, there is no
@ -89,5 +88,3 @@ static inline int android_fork_execvp(int argc, char* argv[], int *status,
}
__END_DECLS
#endif /* __LIBS_LOGWRAP_H */

View file

@ -36,6 +36,11 @@
#define MIN(a,b) (((a)<(b))?(a):(b))
static pthread_mutex_t fd_mutex = PTHREAD_MUTEX_INITIALIZER;
// Protected by fd_mutex. These signals must be blocked while modifying as well.
static pid_t child_pid;
static struct sigaction old_int;
static struct sigaction old_quit;
static struct sigaction old_hup;
#define ERROR(fmt, args...) \
do { \
@ -289,8 +294,47 @@ static void print_abbr_buf(struct log_info *log_info) {
}
}
static int parent(const char *tag, int parent_read, pid_t pid,
int *chld_sts, int log_target, bool abbreviated, char *file_path) {
static void signal_handler(int signal_num);
static void block_signals(sigset_t* oldset) {
sigset_t blockset;
sigemptyset(&blockset);
sigaddset(&blockset, SIGINT);
sigaddset(&blockset, SIGQUIT);
sigaddset(&blockset, SIGHUP);
pthread_sigmask(SIG_BLOCK, &blockset, oldset);
}
static void unblock_signals(sigset_t* oldset) {
pthread_sigmask(SIG_SETMASK, oldset, NULL);
}
static void setup_signal_handlers(pid_t pid) {
struct sigaction handler = {.sa_handler = signal_handler};
child_pid = pid;
sigaction(SIGINT, &handler, &old_int);
sigaction(SIGQUIT, &handler, &old_quit);
sigaction(SIGHUP, &handler, &old_hup);
}
static void restore_signal_handlers() {
sigaction(SIGINT, &old_int, NULL);
sigaction(SIGQUIT, &old_quit, NULL);
sigaction(SIGHUP, &old_hup, NULL);
child_pid = 0;
}
static void signal_handler(int signal_num) {
if (child_pid == 0 || kill(child_pid, signal_num) != 0) {
restore_signal_handlers();
raise(signal_num);
}
}
static int parent(const char* tag, int parent_read, pid_t pid, int* chld_sts, int log_target,
bool abbreviated, char* file_path, bool forward_signals) {
int status = 0;
char buffer[4096];
struct pollfd poll_fds[] = {
@ -308,6 +352,11 @@ static int parent(const char *tag, int parent_read, pid_t pid,
int b = 0; // end index of unprocessed data
int sz;
bool found_child = false;
// There is a very small chance that opening child_ptty in the child will fail, but in this case
// POLLHUP will not be generated below. Therefore, we use a 1 second timeout for poll() until
// we receive a message from child_ptty. If this times out, we call waitpid() with WNOHANG to
// check the status of the child process and exit appropriately if it has terminated.
bool received_messages = false;
char tmpbuf[256];
log_info.btag = basename(tag);
@ -347,13 +396,15 @@ static int parent(const char *tag, int parent_read, pid_t pid,
log_info.abbreviated = abbreviated;
while (!found_child) {
if (TEMP_FAILURE_RETRY(poll(poll_fds, ARRAY_SIZE(poll_fds), -1)) < 0) {
int timeout = received_messages ? -1 : 1000;
if (TEMP_FAILURE_RETRY(poll(poll_fds, ARRAY_SIZE(poll_fds), timeout)) < 0) {
ERROR("poll failed\n");
rc = -1;
goto err_poll;
}
if (poll_fds[0].revents & POLLIN) {
received_messages = true;
sz = TEMP_FAILURE_RETRY(
read(parent_read, &buffer[b], sizeof(buffer) - 1 - b));
@ -396,10 +447,20 @@ static int parent(const char *tag, int parent_read, pid_t pid,
}
}
if (poll_fds[0].revents & POLLHUP) {
if (!received_messages || (poll_fds[0].revents & POLLHUP)) {
int ret;
sigset_t oldset;
ret = TEMP_FAILURE_RETRY(waitpid(pid, &status, 0));
if (forward_signals) {
// Our signal handlers forward these signals to 'child_pid', but waitpid() may reap
// the child, so we must block these signals until we either 1) conclude that the
// child is still running or 2) determine the child has been reaped and we have
// reset the signals to their original disposition.
block_signals(&oldset);
}
int flags = (poll_fds[0].revents & POLLHUP) ? 0 : WNOHANG;
ret = TEMP_FAILURE_RETRY(waitpid(pid, &status, flags));
if (ret < 0) {
rc = errno;
ALOG(LOG_ERROR, "logwrap", "waitpid failed with %s\n", strerror(errno));
@ -408,6 +469,13 @@ static int parent(const char *tag, int parent_read, pid_t pid,
if (ret > 0) {
found_child = true;
}
if (forward_signals) {
if (found_child) {
restore_signal_handlers();
}
unblock_signals(&oldset);
}
}
}
@ -472,21 +540,14 @@ static void child(int argc, char* argv[]) {
}
}
int android_fork_execvp_ext(int argc, char* argv[], int *status, bool ignore_int_quit,
int log_target, bool abbreviated, char *file_path,
void *unused_opts, int unused_opts_len) {
int android_fork_execvp_ext2(int argc, char* argv[], int* status, bool forward_signals,
int log_target, bool abbreviated, char* file_path) {
pid_t pid;
int parent_ptty;
int child_ptty;
struct sigaction intact;
struct sigaction quitact;
sigset_t blockset;
sigset_t oldset;
int rc = 0;
LOG_ALWAYS_FATAL_IF(unused_opts != NULL);
LOG_ALWAYS_FATAL_IF(unused_opts_len != 0);
rc = pthread_mutex_lock(&fd_mutex);
if (rc) {
ERROR("failed to lock signal_fd mutex\n");
@ -494,7 +555,7 @@ int android_fork_execvp_ext(int argc, char* argv[], int *status, bool ignore_int
}
/* Use ptty instead of socketpair so that STDOUT is not buffered */
parent_ptty = TEMP_FAILURE_RETRY(open("/dev/ptmx", O_RDWR));
parent_ptty = TEMP_FAILURE_RETRY(posix_openpt(O_RDWR | O_CLOEXEC));
if (parent_ptty < 0) {
ERROR("Cannot create parent ptty\n");
rc = -1;
@ -509,27 +570,26 @@ int android_fork_execvp_ext(int argc, char* argv[], int *status, bool ignore_int
goto err_ptty;
}
child_ptty = TEMP_FAILURE_RETRY(open(child_devname, O_RDWR));
if (child_ptty < 0) {
ERROR("Cannot open child_ptty\n");
rc = -1;
goto err_child_ptty;
if (forward_signals) {
// Block these signals until we have the child pid and our signal handlers set up.
block_signals(&oldset);
}
sigemptyset(&blockset);
sigaddset(&blockset, SIGINT);
sigaddset(&blockset, SIGQUIT);
pthread_sigmask(SIG_BLOCK, &blockset, &oldset);
pid = fork();
if (pid < 0) {
close(child_ptty);
ERROR("Failed to fork\n");
rc = -1;
goto err_fork;
} else if (pid == 0) {
pthread_mutex_unlock(&fd_mutex);
pthread_sigmask(SIG_SETMASK, &oldset, NULL);
unblock_signals(&oldset);
setsid();
int child_ptty = TEMP_FAILURE_RETRY(open(child_devname, O_RDWR | O_CLOEXEC));
if (child_ptty < 0) {
FATAL_CHILD("Cannot open child_ptty: %s\n", strerror(errno));
}
close(parent_ptty);
dup2(child_ptty, 1);
@ -538,27 +598,23 @@ int android_fork_execvp_ext(int argc, char* argv[], int *status, bool ignore_int
child(argc, argv);
} else {
close(child_ptty);
if (ignore_int_quit) {
struct sigaction ignact;
memset(&ignact, 0, sizeof(ignact));
ignact.sa_handler = SIG_IGN;
sigaction(SIGINT, &ignact, &intact);
sigaction(SIGQUIT, &ignact, &quitact);
if (forward_signals) {
setup_signal_handlers(pid);
unblock_signals(&oldset);
}
rc = parent(argv[0], parent_ptty, pid, status, log_target,
abbreviated, file_path);
rc = parent(argv[0], parent_ptty, pid, status, log_target, abbreviated, file_path,
forward_signals);
if (forward_signals) {
restore_signal_handlers();
}
}
if (ignore_int_quit) {
sigaction(SIGINT, &intact, NULL);
sigaction(SIGQUIT, &quitact, NULL);
}
err_fork:
pthread_sigmask(SIG_SETMASK, &oldset, NULL);
err_child_ptty:
if (forward_signals) {
unblock_signals(&oldset);
}
err_ptty:
close(parent_ptty);
err_open:

View file

@ -79,8 +79,7 @@ int main(int argc, char* argv[]) {
usage();
}
rc = android_fork_execvp_ext(argc, &argv[0], &status, true,
log_target, abbreviated, NULL, NULL, 0);
rc = android_fork_execvp_ext2(argc, &argv[0], &status, true, log_target, abbreviated, NULL);
if (!rc) {
if (WIFEXITED(status))
rc = WEXITSTATUS(status);