This cl implements CommandSection and use it to add procrank.proto Section
Bug: 63863444
Test: manual - create gtests for CommandSection and Procrank Parser following
instructions in the README.md of incidentd and incident_helper on how to
run them.
Change-Id: I099808fd13bf9ed9a564b122f1126b1691a83291
diff --git a/cmds/incidentd/src/FdBuffer.h b/cmds/incidentd/src/FdBuffer.h
index 7888442..03a6d18 100644
--- a/cmds/incidentd/src/FdBuffer.h
+++ b/cmds/incidentd/src/FdBuffer.h
@@ -98,7 +98,7 @@
bool close() { return !(::close(mFds[0]) || ::close(mFds[1])); }
~Fpipe() { close(); }
- inline status_t init() { return pipe(mFds); }
+ inline bool init() { return pipe(mFds) != -1; }
inline int readFd() const { return mFds[0]; }
inline int writeFd() const { return mFds[1]; }
diff --git a/cmds/incidentd/src/Reporter.cpp b/cmds/incidentd/src/Reporter.cpp
index ba157de6..2a4f89e 100644
--- a/cmds/incidentd/src/Reporter.cpp
+++ b/cmds/incidentd/src/Reporter.cpp
@@ -35,7 +35,7 @@
/**
* The directory where the incident reports are stored.
*/
-static const String8 INCIDENT_DIRECTORY("/data/incidents");
+static const String8 INCIDENT_DIRECTORY("/data/misc/incidents");
// ================================================================================
static status_t write_all(int fd, uint8_t const* buf, size_t size)
diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index ddb54c1..8ef6817 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -19,6 +19,7 @@
#include "Section.h"
#include "protobuf.h"
+#include <private/android_filesystem_config.h>
#include <binder/IServiceManager.h>
#include <mutex>
#include <stdio.h>
@@ -29,15 +30,68 @@
using namespace std;
-const int64_t REMOTE_CALL_TIMEOUT_MS = 10 * 1000; // 10 seconds
-const int64_t INCIDENT_HELPER_TIMEOUT_MS = 5 * 1000; // 5 seconds
+const int WAIT_MAX = 5;
+const struct timespec WAIT_INTERVAL_NS = {0, 200 * 1000 * 1000};
const char* INCIDENT_HELPER = "/system/bin/incident_helper";
-const uid_t IH_UID = 9999; // run incident_helper as nobody
-const gid_t IH_GID = 9999;
+
+static pid_t
+forkAndExecuteIncidentHelper(const int id, const char* name, Fpipe& p2cPipe, Fpipe& c2pPipe)
+{
+ const char* ihArgs[] { INCIDENT_HELPER, "-s", to_string(id).c_str(), NULL };
+
+ // fork used in multithreaded environment, avoid adding unnecessary code in child process
+ pid_t pid = fork();
+ if (pid == 0) {
+ // child process executes incident helper as nobody
+ if (setgid(AID_NOBODY) == -1) {
+ ALOGW("%s can't change gid: %s", name, strerror(errno));
+ _exit(EXIT_FAILURE);
+ }
+ if (setuid(AID_NOBODY) == -1) {
+ ALOGW("%s can't change uid: %s", name, strerror(errno));
+ _exit(EXIT_FAILURE);
+ }
+
+ if (dup2(p2cPipe.readFd(), STDIN_FILENO) != 0 || !p2cPipe.close() ||
+ dup2(c2pPipe.writeFd(), STDOUT_FILENO) != 1 || !c2pPipe.close()) {
+ ALOGW("%s can't setup stdin and stdout for incident helper", name);
+ _exit(EXIT_FAILURE);
+ }
+
+ execv(INCIDENT_HELPER, const_cast<char**>(ihArgs));
+
+ ALOGW("%s failed in incident helper process: %s", name, strerror(errno));
+ _exit(EXIT_FAILURE); // always exits with failure if any
+ }
+ // close the fds used in incident helper
+ close(p2cPipe.readFd());
+ close(c2pPipe.writeFd());
+ return pid;
+}
+
+static status_t killChild(pid_t pid) {
+ int status;
+ kill(pid, SIGKILL);
+ if (waitpid(pid, &status, 0) == -1) return -1;
+ return WIFEXITED(status) == 0 ? NO_ERROR : -WEXITSTATUS(status);
+}
+
+static status_t waitForChild(pid_t pid) {
+ int status;
+ bool died = false;
+ // wait for child to report status up to 1 seconds
+ for(int loop = 0; !died && loop < WAIT_MAX; loop++) {
+ if (waitpid(pid, &status, WNOHANG) == pid) died = true;
+ // sleep for 0.2 second
+ nanosleep(&WAIT_INTERVAL_NS, NULL);
+ }
+ if (!died) return killChild(pid);
+ return WIFEXITED(status) == 0 ? NO_ERROR : -WEXITSTATUS(status);
+}
// ================================================================================
-Section::Section(int i)
- :id(i)
+Section::Section(int i, const int64_t timeoutMs)
+ :id(i), timeoutMs(timeoutMs)
{
}
@@ -55,106 +109,57 @@
}
// ================================================================================
-FileSection::FileSection(int id, const char* filename)
- : Section(id), mFilename(filename) {
- name = "cat ";
- name += filename;
+FileSection::FileSection(int id, const char* filename, const int64_t timeoutMs)
+ : Section(id, timeoutMs), mFilename(filename) {
+ name = filename;
}
FileSection::~FileSection() {}
status_t FileSection::Execute(ReportRequestSet* requests) const {
- Fpipe p2cPipe;
- Fpipe c2pPipe;
- FdBuffer buffer;
-
- // initiate pipes to pass data to/from incident_helper
- if (p2cPipe.init() == -1) {
- return -errno;
- }
- if (c2pPipe.init() == -1) {
- return -errno;
- }
-
- // fork a child process
- pid_t pid = fork();
-
- if (pid == -1) {
- ALOGW("FileSection '%s' failed to fork", this->name.string());
- return -errno;
- }
-
- // child process
- if (pid == 0) {
- if (setgid(IH_GID) == -1) {
- ALOGW("FileSection '%s' can't change gid: %s", this->name.string(), strerror(errno));
- exit(EXIT_FAILURE);
- }
- if (setuid(IH_UID) == -1) {
- ALOGW("FileSection '%s' can't change uid: %s", this->name.string(), strerror(errno));
- exit(EXIT_FAILURE);
- }
-
- if (dup2(p2cPipe.readFd(), STDIN_FILENO) != 0 || !p2cPipe.close()) {
- ALOGW("FileSection '%s' failed to set up stdin: %s", this->name.string(), strerror(errno));
- exit(EXIT_FAILURE);
- }
- if (dup2(c2pPipe.writeFd(), STDOUT_FILENO) != 1 || !c2pPipe.close()) {
- ALOGW("FileSection '%s' failed to set up stdout: %s", this->name.string(), strerror(errno));
- exit(EXIT_FAILURE);
- }
-
- // execute incident_helper to parse raw file data and generate protobuf
- char sectionID[8]; // section id is expected to be smaller than 8 digits
- sprintf(sectionID, "%d", this->id);
- const char* args[]{INCIDENT_HELPER, "-s", sectionID, NULL};
- execv(INCIDENT_HELPER, const_cast<char**>(args));
-
- ALOGW("FileSection '%s' failed in child process: %s", this->name.string(), strerror(errno));
- return -1;
- }
-
- // parent process
-
- // close fds used in child process
- close(p2cPipe.readFd());
- close(c2pPipe.writeFd());
-
- // read from mFilename and pump buffer to incident_helper
- status_t err = NO_ERROR;
- int fd = open(mFilename, O_RDONLY);
+ // read from mFilename first, make sure the file is available
+ // add O_CLOEXEC to make sure it is closed when exec incident helper
+ int fd = open(mFilename, O_RDONLY | O_CLOEXEC, 0444);
if (fd == -1) {
ALOGW("FileSection '%s' failed to open file", this->name.string());
return -errno;
}
- err = buffer.readProcessedDataInStream(fd,
- p2cPipe.writeFd(), c2pPipe.readFd(), INCIDENT_HELPER_TIMEOUT_MS);
- if (err != NO_ERROR) {
- ALOGW("FileSection '%s' failed to read data from incident helper: %s",
- this->name.string(), strerror(-err));
- kill(pid, SIGKILL); // kill child process if meets error
- return err;
- }
-
- if (buffer.timedOut()) {
- ALOGW("FileSection '%s' timed out reading from incident helper!", this->name.string());
- kill(pid, SIGKILL); // kill the child process if timed out
- }
-
- // has to block here to reap child process
- int status;
- int w = waitpid(pid, &status, 0);
- if (w < 0 || status == -1) {
- ALOGW("FileSection '%s' abnormal child process: %s", this->name.string(), strerror(-err));
+ FdBuffer buffer;
+ Fpipe p2cPipe;
+ Fpipe c2pPipe;
+ // initiate pipes to pass data to/from incident_helper
+ if (!p2cPipe.init() || !c2pPipe.init()) {
+ ALOGW("FileSection '%s' failed to setup pipes", this->name.string());
return -errno;
}
- // write parsed data to reporter
- ALOGD("section '%s' wrote %zd bytes in %d ms", this->name.string(), buffer.size(),
+ pid_t pid = forkAndExecuteIncidentHelper(this->id, this->name.string(), p2cPipe, c2pPipe);
+ if (pid == -1) {
+ ALOGW("FileSection '%s' failed to fork", this->name.string());
+ return -errno;
+ }
+
+ // parent process
+ status_t readStatus = buffer.readProcessedDataInStream(fd, p2cPipe.writeFd(), c2pPipe.readFd(),
+ this->timeoutMs);
+ if (readStatus != NO_ERROR || buffer.timedOut()) {
+ ALOGW("FileSection '%s' failed to read data from incident helper: %s, timedout: %s, kill: %s",
+ this->name.string(), strerror(-readStatus), buffer.timedOut() ? "true" : "false",
+ strerror(-killChild(pid)));
+ return readStatus;
+ }
+
+ status_t ihStatus = waitForChild(pid);
+ if (ihStatus != NO_ERROR) {
+ ALOGW("FileSection '%s' abnormal child process: %s", this->name.string(), strerror(-ihStatus));
+ return ihStatus;
+ }
+
+ ALOGD("FileSection '%s' wrote %zd bytes in %d ms", this->name.string(), buffer.size(),
(int)buffer.durationMs());
WriteHeader(requests, buffer.size());
- err = buffer.write(requests);
+ status_t err = buffer.write(requests);
if (err != NO_ERROR) {
ALOGW("FileSection '%s' failed writing: %s", this->name.string(), strerror(-err));
return err;
@@ -263,7 +268,7 @@
pthread_attr_destroy(&attr);
// Loop reading until either the timeout or the worker side is done (i.e. eof).
- err = buffer.read(data->readFd(), REMOTE_CALL_TIMEOUT_MS);
+ err = buffer.read(data->readFd(), this->timeoutMs);
if (err != NO_ERROR) {
// TODO: Log this error into the incident report.
ALOGW("WorkerThreadSection '%s' reader failed with error '%s'", this->name.string(),
@@ -309,7 +314,7 @@
}
// Write the data that was collected
- ALOGD("section '%s' wrote %zd bytes in %d ms", name.string(), buffer.size(),
+ ALOGD("WorkerThreadSection '%s' wrote %zd bytes in %d ms", name.string(), buffer.size(),
(int)buffer.durationMs());
WriteHeader(requests, buffer.size());
err = buffer.write(requests);
@@ -322,42 +327,116 @@
}
// ================================================================================
-CommandSection::CommandSection(int id, const char* first, ...)
- :Section(id)
+void CommandSection::init(const char* command, va_list args)
+{
+ va_list copied_args;
+ va_copy(copied_args, args);
+ int numOfArgs = 0;
+ while(va_arg(args, const char*) != NULL) {
+ numOfArgs++;
+ }
+
+ // allocate extra 1 for command and 1 for NULL terminator
+ mCommand = (const char**)malloc(sizeof(const char*) * (numOfArgs + 2));
+
+ mCommand[0] = command;
+ name = command;
+ for (int i=0; i<numOfArgs; i++) {
+ const char* arg = va_arg(copied_args, const char*);
+ mCommand[i+1] = arg;
+ name += " ";
+ name += arg;
+ }
+ mCommand[numOfArgs+1] = NULL;
+ va_end(copied_args);
+}
+
+CommandSection::CommandSection(int id, const int64_t timeoutMs, const char* command, ...)
+ : Section(id, timeoutMs)
{
va_list args;
- int count = 0;
-
- va_start(args, first);
- while (va_arg(args, const char*) != NULL) {
- count++;
- }
+ va_start(args, command);
+ init(command, args);
va_end(args);
+}
- mCommand = (const char**)malloc(sizeof(const char*) * count);
-
- mCommand[0] = first;
- name = first;
- name += " ";
- va_start(args, first);
- for (int i=0; i<count; i++) {
- const char* arg = va_arg(args, const char*);
- mCommand[i+1] = arg;
- if (arg != NULL) {
- name += va_arg(args, const char*);
- name += " ";
- }
- }
+CommandSection::CommandSection(int id, const char* command, ...)
+ : Section(id)
+{
+ va_list args;
+ va_start(args, command);
+ init(command, args);
va_end(args);
}
CommandSection::~CommandSection()
{
+ free(mCommand);
}
status_t
-CommandSection::Execute(ReportRequestSet* /*requests*/) const
+CommandSection::Execute(ReportRequestSet* requests) const
{
+ FdBuffer buffer;
+ Fpipe cmdPipe;
+ Fpipe ihPipe;
+
+ if (!cmdPipe.init() || !ihPipe.init()) {
+ ALOGW("CommandSection '%s' failed to setup pipes", this->name.string());
+ return -errno;
+ }
+
+ pid_t cmdPid = fork();
+ if (cmdPid == -1) {
+ ALOGW("CommandSection '%s' failed to fork", this->name.string());
+ return -errno;
+ }
+ // child process to execute the command as root
+ if (cmdPid == 0) {
+ // replace command's stdout with ihPipe's write Fd
+ if (dup2(cmdPipe.writeFd(), STDOUT_FILENO) != 1 || !ihPipe.close() || !cmdPipe.close()) {
+ ALOGW("CommandSection '%s' failed to set up stdout: %s", this->name.string(), strerror(errno));
+ _exit(EXIT_FAILURE);
+ }
+ execv(this->mCommand[0], (char *const *) this->mCommand);
+ int err = errno; // record command error code
+ ALOGW("CommandSection '%s' failed in executing command: %s", this->name.string(), strerror(errno));
+ _exit(err); // exit with command error code
+ }
+ pid_t ihPid = forkAndExecuteIncidentHelper(this->id, this->name.string(), cmdPipe, ihPipe);
+ if (ihPid == -1) {
+ ALOGW("CommandSection '%s' failed to fork", this->name.string());
+ return -errno;
+ }
+
+ close(cmdPipe.writeFd());
+ status_t readStatus = buffer.read(ihPipe.readFd(), this->timeoutMs);
+ if (readStatus != NO_ERROR || buffer.timedOut()) {
+ ALOGW("CommandSection '%s' failed to read data from incident helper: %s, "
+ "timedout: %s, kill command: %s, kill incident helper: %s",
+ this->name.string(), strerror(-readStatus), buffer.timedOut() ? "true" : "false",
+ strerror(-killChild(cmdPid)), strerror(-killChild(ihPid)));
+ return readStatus;
+ }
+
+ // TODO: wait for command here has one trade-off: the failed status of command won't be detected until
+ // buffer timeout, but it has advatage on starting the data stream earlier.
+ status_t cmdStatus = waitForChild(cmdPid);
+ status_t ihStatus = waitForChild(ihPid);
+ if (cmdStatus != NO_ERROR || ihStatus != NO_ERROR) {
+ ALOGW("CommandSection '%s' abnormal child processes, return status: command: %s, incidnet helper: %s",
+ this->name.string(), strerror(-cmdStatus), strerror(-ihStatus));
+ return cmdStatus != NO_ERROR ? cmdStatus : ihStatus;
+ }
+
+ ALOGD("CommandSection '%s' wrote %zd bytes in %d ms", this->name.string(), buffer.size(),
+ (int)buffer.durationMs());
+ WriteHeader(requests, buffer.size());
+ status_t err = buffer.write(requests);
+ if (err != NO_ERROR) {
+ ALOGW("CommandSection '%s' failed writing: %s", this->name.string(), strerror(-err));
+ return err;
+ }
return NO_ERROR;
}
diff --git a/cmds/incidentd/src/Section.h b/cmds/incidentd/src/Section.h
index 35740e9..93b4848 100644
--- a/cmds/incidentd/src/Section.h
+++ b/cmds/incidentd/src/Section.h
@@ -19,22 +19,26 @@
#include "FdBuffer.h"
+#include <stdarg.h>
#include <utils/String8.h>
#include <utils/String16.h>
#include <utils/Vector.h>
using namespace android;
+const int64_t REMOTE_CALL_TIMEOUT_MS = 10 * 1000; // 10 seconds
+
/**
* Base class for sections
*/
class Section
{
public:
- int id;
+ const int id;
+ const int64_t timeoutMs; // each section must have a timeout
String8 name;
- Section(int id);
+ Section(int id, const int64_t timeoutMs = REMOTE_CALL_TIMEOUT_MS);
virtual ~Section();
virtual status_t Execute(ReportRequestSet* requests) const = 0;
@@ -48,7 +52,7 @@
class FileSection : public Section
{
public:
- FileSection(int id, const char* filename);
+ FileSection(int id, const char* filename, const int64_t timeoutMs = 5000 /* 5 seconds */);
virtual ~FileSection();
virtual status_t Execute(ReportRequestSet* requests) const;
@@ -77,13 +81,18 @@
class CommandSection : public Section
{
public:
- CommandSection(int id, const char* first, ...);
+ CommandSection(int id, const int64_t timeoutMs, const char* command, ...);
+
+ CommandSection(int id, const char* command, ...);
+
virtual ~CommandSection();
virtual status_t Execute(ReportRequestSet* requests) const;
private:
const char** mCommand;
+
+ void init(const char* command, va_list args);
};
/**
diff --git a/cmds/incidentd/src/section_list.cpp b/cmds/incidentd/src/section_list.cpp
index 72c54d2..80ddb86 100644
--- a/cmds/incidentd/src/section_list.cpp
+++ b/cmds/incidentd/src/section_list.cpp
@@ -21,6 +21,7 @@
*/
const Section* SECTION_LIST[] = {
// Linux Services
+ new CommandSection(2000, "/system/xbin/procrank", NULL),
new FileSection(2002, "/d/wakeup_sources"),
// System Services