Found out it is not possible to exec procrank as non-root/shell.
But it is ok to disable procrank since the information can be obtain via
dumpsys meminfo
Optimize the code style a bit.
Bug: 72510307
Test: atest incidentd_test
Change-Id: If06201aa708461b569d9fb007f20d1e51342d5c2
diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index 5065a56..b59a32c 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -20,7 +20,6 @@
#include <dirent.h>
#include <errno.h>
-#include <wait.h>
#include <mutex>
#include <set>
@@ -53,49 +52,15 @@
const int FIELD_ID_INCIDENT_METADATA = 2;
// incident section parameters
-const int WAIT_MAX = 5;
-const struct timespec WAIT_INTERVAL_NS = {0, 200 * 1000 * 1000};
const char INCIDENT_HELPER[] = "/system/bin/incident_helper";
-const char GZIP[] = "/system/bin/gzip";
+const char* GZIP[] = {"/system/bin/gzip", NULL};
static pid_t fork_execute_incident_helper(const int id, Fpipe* p2cPipe, Fpipe* c2pPipe) {
const char* ihArgs[]{INCIDENT_HELPER, "-s", String8::format("%d", id).string(), NULL};
- return fork_execute_cmd(INCIDENT_HELPER, const_cast<char**>(ihArgs), p2cPipe, c2pPipe);
+ return fork_execute_cmd(const_cast<char**>(ihArgs), p2cPipe, c2pPipe);
}
// ================================================================================
-static status_t statusCode(int status) {
- if (WIFSIGNALED(status)) {
- VLOG("return by signal: %s", strerror(WTERMSIG(status)));
- return -WTERMSIG(status);
- } else if (WIFEXITED(status) && WEXITSTATUS(status) > 0) {
- VLOG("return by exit: %s", strerror(WEXITSTATUS(status)));
- return -WEXITSTATUS(status);
- }
- return NO_ERROR;
-}
-
-static status_t kill_child(pid_t pid) {
- int status;
- VLOG("try to kill child process %d", pid);
- kill(pid, SIGKILL);
- if (waitpid(pid, &status, 0) == -1) return -1;
- return statusCode(status);
-}
-
-static status_t wait_child(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 kill_child(pid);
- return statusCode(status);
-}
-// ================================================================================
static status_t write_section_header(int fd, int sectionId, size_t size) {
uint8_t buf[20];
uint8_t* p = write_length_delimited_tag_header(buf, sectionId, size);
@@ -328,12 +293,15 @@
}
// ================================================================================
GZipSection::GZipSection(int id, const char* filename, ...) : Section(id) {
- name = "gzip ";
- name += filename;
va_list args;
va_start(args, filename);
mFilenames = varargs(filename, args);
va_end(args);
+ name = "gzip";
+ for (int i = 0; mFilenames[i] != NULL; i++) {
+ name += " ";
+ name += mFilenames[i];
+ }
}
GZipSection::~GZipSection() {}
@@ -362,8 +330,7 @@
return -errno;
}
- const char* gzipArgs[]{GZIP, NULL};
- pid_t pid = fork_execute_cmd(GZIP, const_cast<char**>(gzipArgs), &p2cPipe, &c2pPipe);
+ pid_t pid = fork_execute_cmd((char* const*)GZIP, &p2cPipe, &c2pPipe);
if (pid == -1) {
ALOGW("GZipSection '%s' failed to fork", this->name.string());
return -errno;
@@ -559,19 +526,27 @@
// ================================================================================
CommandSection::CommandSection(int id, const int64_t timeoutMs, const char* command, ...)
: Section(id, timeoutMs) {
- name = command;
va_list args;
va_start(args, command);
mCommand = varargs(command, args);
va_end(args);
+ name = "cmd";
+ for (int i = 0; mCommand[i] != NULL; i++) {
+ name += " ";
+ name += mCommand[i];
+ }
}
CommandSection::CommandSection(int id, const char* command, ...) : Section(id) {
- name = command;
va_list args;
va_start(args, command);
mCommand = varargs(command, args);
va_end(args);
+ name = "cmd";
+ for (int i = 0; mCommand[i] != NULL; i++) {
+ name += " ";
+ name += mCommand[i];
+ }
}
CommandSection::~CommandSection() { free(mCommand); }
@@ -586,26 +561,11 @@
return -errno;
}
- pid_t cmdPid = fork();
+ pid_t cmdPid = fork_execute_cmd((char* const*)mCommand, NULL, &cmdPipe);
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().get(), STDOUT_FILENO) != 1 || !ihPipe.close() ||
- !cmdPipe.close()) {
- ALOGW("CommandSection '%s' failed to set up stdout: %s", this->name.string(),
- strerror(errno));
- _exit(EXIT_FAILURE);
- }
- execvp(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 = fork_execute_incident_helper(this->id, &cmdPipe, &ihPipe);
if (ihPid == -1) {
ALOGW("CommandSection '%s' failed to fork", this->name.string());
diff --git a/cmds/incidentd/src/incidentd_util.cpp b/cmds/incidentd/src/incidentd_util.cpp
index d799513..7db1fa7 100644
--- a/cmds/incidentd/src/incidentd_util.cpp
+++ b/cmds/incidentd/src/incidentd_util.cpp
@@ -19,6 +19,7 @@
#include "incidentd_util.h"
#include <sys/prctl.h>
+#include <wait.h>
#include "section_list.h"
@@ -57,27 +58,28 @@
unique_fd& Fpipe::writeFd() { return mWrite; }
-pid_t fork_execute_cmd(const char* cmd, char* const argv[], Fpipe* input, Fpipe* output) {
+pid_t fork_execute_cmd(char* const argv[], Fpipe* input, Fpipe* output) {
// fork used in multithreaded environment, avoid adding unnecessary code in child process
pid_t pid = fork();
if (pid == 0) {
- if (TEMP_FAILURE_RETRY(dup2(input->readFd().get(), STDIN_FILENO)) < 0 || !input->close() ||
- TEMP_FAILURE_RETRY(dup2(output->writeFd().get(), STDOUT_FILENO)) < 0 ||
- !output->close()) {
- ALOGW("Can't setup stdin and stdout for command %s", cmd);
+ VLOG("[In child]cmd %s", argv[0]);
+ if (input != NULL && (TEMP_FAILURE_RETRY(dup2(input->readFd().get(), STDIN_FILENO)) < 0 ||
+ !input->close())) {
+ ALOGW("Failed to dup2 stdin.");
_exit(EXIT_FAILURE);
}
-
+ if (TEMP_FAILURE_RETRY(dup2(output->writeFd().get(), STDOUT_FILENO)) < 0 ||
+ !output->close()) {
+ ALOGW("Failed to dup2 stdout.");
+ _exit(EXIT_FAILURE);
+ }
/* make sure the child dies when incidentd dies */
prctl(PR_SET_PDEATHSIG, SIGKILL);
-
- execv(cmd, argv);
-
- ALOGW("%s failed in the child process: %s", cmd, strerror(errno));
- _exit(EXIT_FAILURE); // always exits with failure if any
+ execvp(argv[0], argv);
+ _exit(errno); // always exits with failure if any
}
// close the fds used in child process.
- input->readFd().reset();
+ if (input != NULL) input->readFd().reset();
output->writeFd().reset();
return pid;
}
@@ -111,3 +113,39 @@
clock_gettime(CLOCK_MONOTONIC, &ts);
return static_cast<uint64_t>(ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec);
}
+
+// ================================================================================
+const int WAIT_MAX = 5;
+const struct timespec WAIT_INTERVAL_NS = {0, 200 * 1000 * 1000};
+
+static status_t statusCode(int status) {
+ if (WIFSIGNALED(status)) {
+ VLOG("return by signal: %s", strerror(WTERMSIG(status)));
+ return -WTERMSIG(status);
+ } else if (WIFEXITED(status) && WEXITSTATUS(status) > 0) {
+ VLOG("return by exit: %s", strerror(WEXITSTATUS(status)));
+ return -WEXITSTATUS(status);
+ }
+ return NO_ERROR;
+}
+
+status_t kill_child(pid_t pid) {
+ int status;
+ VLOG("try to kill child process %d", pid);
+ kill(pid, SIGKILL);
+ if (waitpid(pid, &status, 0) == -1) return -1;
+ return statusCode(status);
+}
+
+status_t wait_child(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 kill_child(pid);
+ return statusCode(status);
+}
diff --git a/cmds/incidentd/src/incidentd_util.h b/cmds/incidentd/src/incidentd_util.h
index 228d776..b5f6e21 100644
--- a/cmds/incidentd/src/incidentd_util.h
+++ b/cmds/incidentd/src/incidentd_util.h
@@ -18,12 +18,15 @@
#ifndef INCIDENTD_UTIL_H
#define INCIDENTD_UTIL_H
-#include <android-base/unique_fd.h>
-
#include <stdarg.h>
+#include <unistd.h>
+
+#include <android-base/unique_fd.h>
+#include <utils/Errors.h>
#include "Privacy.h"
+using namespace android;
using namespace android::base;
/**
@@ -52,8 +55,9 @@
/**
* Forks and exec a command with two pipes, one connects stdin for input,
* one connects stdout for output. It returns the pid of the child.
+ * Input pipe can be NULL to indicate child process doesn't read stdin.
*/
-pid_t fork_execute_cmd(const char* cmd, char* const argv[], Fpipe* input, Fpipe* output);
+pid_t fork_execute_cmd(char* const argv[], Fpipe* input, Fpipe* output);
/**
* Grabs varargs from stack and stores them in heap with NULL-terminated array.
@@ -65,4 +69,10 @@
*/
uint64_t Nanotime();
+/**
+ * Methods to wait or kill child process, return exit status code.
+ */
+status_t kill_child(pid_t pid);
+status_t wait_child(pid_t pid);
+
#endif // INCIDENTD_UTIL_H