Add an option to zip incident report
Incident reports are getting bigger as we add more sections. Add an
option (-z, default off) to zip incident report.
Bug: 150160547
Test: atest incidentd_test
Test: adb shell incident -z -p EXPLICIT | gunzip | ./out/soong/host/linux-x86/bin/aprotoc --decode=android.os.IncidentProto --proto_path=./ --proto_path=external/protobuf/src frameworks/base/core/proto/android/os/incident.proto
Change-Id: I7c8ff1d91df842c200462ee29f15feae68e62739
diff --git a/cmds/incident/main.cpp b/cmds/incident/main.cpp
index eb2b98a..cdf742d 100644
--- a/cmds/incident/main.cpp
+++ b/cmds/incident/main.cpp
@@ -229,6 +229,7 @@
fprintf(out, " -l list available sections\n");
fprintf(out, " -p privacy spec, LOCAL, EXPLICIT or AUTOMATIC. Default AUTOMATIC.\n");
fprintf(out, " -r REASON human readable description of why the report is taken.\n");
+ fprintf(out, " -z gzip the incident report, i.e. pipe the output through gzip.\n");
fprintf(out, "\n");
fprintf(out, "and one of these destinations:\n");
fprintf(out, " -b (default) print the report to stdout (in proto format)\n");
@@ -253,7 +254,7 @@
// Parse the args
int opt;
- while ((opt = getopt(argc, argv, "bhdlp:r:s:u")) != -1) {
+ while ((opt = getopt(argc, argv, "bhdlp:r:s:uz")) != -1) {
switch (opt) {
case 'h':
usage(stdout);
@@ -300,6 +301,9 @@
destination = DEST_BROADCAST;
receiverArg = optarg;
break;
+ case 'z':
+ args.setGzip(true);
+ break;
default:
usage(stderr);
return 1;
diff --git a/cmds/incidentd/src/Reporter.cpp b/cmds/incidentd/src/Reporter.cpp
index aa40f85..ad25342 100644
--- a/cmds/incidentd/src/Reporter.cpp
+++ b/cmds/incidentd/src/Reporter.cpp
@@ -35,10 +35,12 @@
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
+#include <sys/prctl.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <string>
#include <time.h>
+#include <wait.h>
namespace android {
namespace os {
@@ -51,6 +53,8 @@
* frameworks/base/core/proto/android/os/incident.proto
*/
const int FIELD_ID_METADATA = 2;
+// Args for exec gzip
+static const char* GZIP[] = {"/system/bin/gzip", NULL};
IncidentMetadata_Destination privacy_policy_to_dest(uint8_t privacyPolicy) {
switch (privacyPolicy) {
@@ -142,7 +146,8 @@
mListener(listener),
mFd(fd),
mIsStreaming(fd >= 0),
- mStatus(NO_ERROR) {
+ mStatus(OK),
+ mZipPid(-1) {
}
ReportRequest::~ReportRequest() {
@@ -153,7 +158,14 @@
}
bool ReportRequest::ok() {
- return mFd >= 0 && mStatus == NO_ERROR;
+ if (mStatus != OK) {
+ return false;
+ }
+ if (!args.gzip()) {
+ return mFd >= 0;
+ }
+ // Send a blank signal to check if mZipPid is alive
+ return mZipPid > 0 && kill(mZipPid, 0) == 0;
}
bool ReportRequest::containsSection(int sectionId) const {
@@ -161,10 +173,45 @@
}
void ReportRequest::closeFd() {
- if (mIsStreaming && mFd >= 0) {
+ if (!mIsStreaming) {
+ return;
+ }
+ if (mFd >= 0) {
close(mFd);
mFd = -1;
}
+ if (mZipPid > 0) {
+ mZipPipe.close();
+ // Gzip may take some time.
+ status_t err = wait_child(mZipPid, /* timeout_ms= */ 10 * 1000);
+ if (err != 0) {
+ ALOGW("[ReportRequest] abnormal child process: %s", strerror(-err));
+ }
+ }
+}
+
+int ReportRequest::getFd() {
+ return mZipPid > 0 ? mZipPipe.writeFd().get() : mFd;
+}
+
+status_t ReportRequest::initGzipIfNecessary() {
+ if (!mIsStreaming || !args.gzip()) {
+ return OK;
+ }
+ if (!mZipPipe.init()) {
+ ALOGE("[ReportRequest] Failed to setup pipe for gzip");
+ mStatus = -errno;
+ return mStatus;
+ }
+ int status = 0;
+ pid_t pid = fork_execute_cmd((char* const*)GZIP, mZipPipe.readFd().release(), mFd, &status);
+ if (pid < 0 || status != 0) {
+ mStatus = status;
+ return mStatus;
+ }
+ mZipPid = pid;
+ mFd = -1;
+ return OK;
}
// ================================================================================
@@ -562,6 +609,13 @@
reportId = (spec.tv_sec) * 1000 + spec.tv_nsec;
}
+ mBatch->forEachStreamingRequest([](const sp<ReportRequest>& request) {
+ status_t err = request->initGzipIfNecessary();
+ if (err != 0) {
+ ALOGW("Error forking gzip: %s", strerror(err));
+ }
+ });
+
// Write the incident report headers - each request gets its own headers. It's different
// from the other top-level fields in IncidentReport that are the sections where the rest
// is all shared data (although with their own individual privacy filtering).
diff --git a/cmds/incidentd/src/Reporter.h b/cmds/incidentd/src/Reporter.h
index cbc8b13..bd47a23 100644
--- a/cmds/incidentd/src/Reporter.h
+++ b/cmds/incidentd/src/Reporter.h
@@ -15,6 +15,7 @@
*/
#pragma once
+#include "incidentd_util.h"
#include "FdBuffer.h"
#include "WorkDirectory.h"
@@ -63,10 +64,12 @@
sp<IIncidentReportStatusListener> getListener() { return mListener; }
- int getFd() { return mFd; }
+ int getFd();
int setPersistedFd(int fd);
+ status_t initGzipIfNecessary();
+
void closeFd();
private:
@@ -74,6 +77,8 @@
int mFd;
bool mIsStreaming;
status_t mStatus;
+ pid_t mZipPid;
+ Fpipe mZipPipe;
};
// ================================================================================
diff --git a/cmds/incidentd/src/WorkDirectory.cpp b/cmds/incidentd/src/WorkDirectory.cpp
index 9963533..1944d6e 100644
--- a/cmds/incidentd/src/WorkDirectory.cpp
+++ b/cmds/incidentd/src/WorkDirectory.cpp
@@ -16,10 +16,10 @@
#include "Log.h"
-#include "WorkDirectory.h"
-
+#include "incidentd_util.h"
#include "proto_util.h"
#include "PrivacyFilter.h"
+#include "WorkDirectory.h"
#include <google/protobuf/io/zero_copy_stream_impl.h>
#include <private/android_filesystem_config.h>
@@ -68,6 +68,9 @@
/** metadata field id in IncidentProto */
const int FIELD_ID_INCIDENT_METADATA = 2;
+// Args for exec gzip
+static const char* GZIP[] = {"/system/bin/gzip", NULL};
+
/**
* Read a protobuf from disk into the message.
*/
@@ -292,6 +295,7 @@
report->set_cls(args.receiverCls());
report->set_privacy_policy(args.getPrivacyPolicy());
report->set_all_sections(args.all());
+ report->set_gzip(args.gzip());
for (int section: args.sections()) {
report->add_section(section);
}
@@ -417,6 +421,24 @@
return BAD_VALUE;
}
+ pid_t zipPid = 0;
+ if (args.gzip()) {
+ Fpipe zipPipe;
+ if (!zipPipe.init()) {
+ ALOGE("[ReportFile] Failed to setup pipe for gzip");
+ close(writeFd);
+ return -errno;
+ }
+ int status = 0;
+ zipPid = fork_execute_cmd((char* const*)GZIP, zipPipe.readFd().release(), writeFd, &status);
+ close(writeFd);
+ if (zipPid < 0 || status != 0) {
+ ALOGE("[ReportFile] Failed to fork and exec gzip");
+ return status;
+ }
+ writeFd = zipPipe.writeFd().release();
+ }
+
status_t err;
for (const auto& report : mEnvelope.report()) {
@@ -437,6 +459,13 @@
}
close(writeFd);
+ if (zipPid > 0) {
+ status_t err = wait_child(zipPid, /* timeout_ms= */ 10 * 1000);
+ if (err != 0) {
+ ALOGE("[ReportFile] abnormal child process: %s", strerror(-err));
+ }
+ return err;
+ }
return NO_ERROR;
}
@@ -621,7 +650,7 @@
map<string,WorkDirectoryEntry> files;
get_directory_contents_locked(&files, 0);
-
+
for (map<string,WorkDirectoryEntry>::iterator it = files.begin();
it != files.end(); it++) {
sp<ReportFile> reportFile = new ReportFile(this, it->second.timestampNs,
@@ -815,6 +844,7 @@
out->setAll(report.all_sections());
out->setReceiverPkg(report.pkg());
out->setReceiverCls(report.cls());
+ out->setGzip(report.gzip());
const int sectionCount = report.section_size();
for (int i = 0; i < sectionCount; i++) {
diff --git a/cmds/incidentd/src/incidentd_util.cpp b/cmds/incidentd/src/incidentd_util.cpp
index dfaf893..2649fb9 100644
--- a/cmds/incidentd/src/incidentd_util.cpp
+++ b/cmds/incidentd/src/incidentd_util.cpp
@@ -18,6 +18,7 @@
#include "incidentd_util.h"
+#include <fcntl.h>
#include <sys/prctl.h>
#include <wait.h>
@@ -64,28 +65,52 @@
unique_fd& Fpipe::writeFd() { return mWrite; }
-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 fork_execute_cmd(char* const argv[], Fpipe* input, Fpipe* output, int* status) {
+ int in = -1;
+ if (input != nullptr) {
+ in = input->readFd().release();
+ // Auto close write end of the input pipe on exec to prevent leaking fd in child process
+ fcntl(input->writeFd().get(), F_SETFD, FD_CLOEXEC);
+ }
+ int out = output->writeFd().release();
+ // Auto close read end of the output pipe on exec
+ fcntl(output->readFd().get(), F_SETFD, FD_CLOEXEC);
+ return fork_execute_cmd(argv, in, out, status);
+}
+
+pid_t fork_execute_cmd(char* const argv[], int in, int out, int* status) {
+ int dummy_status = 0;
+ if (status == nullptr) {
+ status = &dummy_status;
+ }
+ *status = 0;
pid_t pid = fork();
+ if (pid < 0) {
+ *status = -errno;
+ return -1;
+ }
if (pid == 0) {
- if (input != NULL && (TEMP_FAILURE_RETRY(dup2(input->readFd().get(), STDIN_FILENO)) < 0 ||
- !input->close())) {
+ // In child
+ if (in >= 0 && (TEMP_FAILURE_RETRY(dup2(in, STDIN_FILENO)) < 0 || close(in))) {
ALOGW("Failed to dup2 stdin.");
_exit(EXIT_FAILURE);
}
- if (TEMP_FAILURE_RETRY(dup2(output->writeFd().get(), STDOUT_FILENO)) < 0 ||
- !output->close()) {
+ if (TEMP_FAILURE_RETRY(dup2(out, STDOUT_FILENO)) < 0 || close(out)) {
ALOGW("Failed to dup2 stdout.");
_exit(EXIT_FAILURE);
}
- /* make sure the child dies when incidentd dies */
+ // Make sure the child dies when incidentd dies
prctl(PR_SET_PDEATHSIG, SIGKILL);
execvp(argv[0], argv);
_exit(errno); // always exits with failure if any
}
- // close the fds used in child process.
- if (input != NULL) input->readFd().reset();
- output->writeFd().reset();
+ // In parent
+ if ((in >= 0 && close(in) < 0) || close(out) < 0) {
+ ALOGW("Failed to close pd. Killing child process");
+ *status = -errno;
+ kill_child(pid);
+ return -1;
+ }
return pid;
}
@@ -120,9 +145,6 @@
}
// ================================================================================
-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)));
@@ -134,25 +156,64 @@
return NO_ERROR;
}
+static bool waitpid_with_timeout(pid_t pid, int timeout_ms, int* status) {
+ sigset_t child_mask, old_mask;
+ sigemptyset(&child_mask);
+ sigaddset(&child_mask, SIGCHLD);
+
+ if (sigprocmask(SIG_BLOCK, &child_mask, &old_mask) == -1) {
+ ALOGW("sigprocmask failed: %s", strerror(errno));
+ return false;
+ }
+
+ timespec ts;
+ ts.tv_sec = timeout_ms / 1000;
+ ts.tv_nsec = (timeout_ms % 1000) * 1000000;
+ int ret = TEMP_FAILURE_RETRY(sigtimedwait(&child_mask, nullptr, &ts));
+ int saved_errno = errno;
+
+ // Set the signals back the way they were.
+ if (sigprocmask(SIG_SETMASK, &old_mask, nullptr) == -1) {
+ ALOGW("sigprocmask failed: %s", strerror(errno));
+ if (ret == 0) {
+ return false;
+ }
+ }
+ if (ret == -1) {
+ errno = saved_errno;
+ if (errno == EAGAIN) {
+ errno = ETIMEDOUT;
+ } else {
+ ALOGW("sigtimedwait failed: %s", strerror(errno));
+ }
+ return false;
+ }
+
+ pid_t child_pid = waitpid(pid, status, WNOHANG);
+ if (child_pid == pid) {
+ return true;
+ }
+ if (child_pid == -1) {
+ ALOGW("waitpid failed: %s", strerror(errno));
+ } else {
+ ALOGW("Waiting for pid %d, got pid %d instead", pid, child_pid);
+ }
+ return false;
+}
+
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) {
+status_t wait_child(pid_t pid, int timeout_ms) {
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 (waitpid_with_timeout(pid, timeout_ms, &status)) {
+ return statusCode(status);
}
- if (!died) return kill_child(pid);
- return statusCode(status);
+ return kill_child(pid);
}
} // namespace incidentd
diff --git a/cmds/incidentd/src/incidentd_util.h b/cmds/incidentd/src/incidentd_util.h
index cc30768..a54993fe 100644
--- a/cmds/incidentd/src/incidentd_util.h
+++ b/cmds/incidentd/src/incidentd_util.h
@@ -56,11 +56,24 @@
};
/**
- * 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.
+ * Forks and exec a command with two pipes and returns the pid of the child, or -1 when it fails.
+ *
+ * input connects stdin for input. output connects stdout for output. input can be nullptr to
+ * indicate that child process doesn't read stdin. This function will close in and out fds upon
+ * success. If status is not NULL, the status information will be stored in the int to which it
+ * points.
*/
-pid_t fork_execute_cmd(char* const argv[], Fpipe* input, Fpipe* output);
+pid_t fork_execute_cmd(char* const argv[], Fpipe* input, Fpipe* output, int* status = nullptr);
+
+/**
+ * Forks and exec a command that reads from in fd and writes to out fd and returns the pid of the
+ * child, or -1 when it fails.
+ *
+ * in can be -1 to indicate that child process doesn't read stdin. This function will close in and
+ * out fds upon success. If status is not NULL, the status information will be stored in the int
+ * to which it points.
+ */
+pid_t fork_execute_cmd(char* const argv[], int in, int out, int* status = nullptr);
/**
* Grabs varargs from stack and stores them in heap with NULL-terminated array.
@@ -76,7 +89,7 @@
* 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);
+status_t wait_child(pid_t pid, int timeout_ms = 1000);
status_t start_detached_thread(const function<void ()>& func);
diff --git a/cmds/incidentd/src/report_file.proto b/cmds/incidentd/src/report_file.proto
index 7563da2..85fd2da 100644
--- a/cmds/incidentd/src/report_file.proto
+++ b/cmds/incidentd/src/report_file.proto
@@ -65,6 +65,11 @@
* the given client.
*/
optional bool share_approved = 8;
+
+ /**
+ * Whether the report is gzipped.
+ */
+ optional bool gzip = 9;
}
/**
diff --git a/libs/incident/include_priv/android/os/IncidentReportArgs.h b/libs/incident/include_priv/android/os/IncidentReportArgs.h
index 0e61590..ec3aabb 100644
--- a/libs/incident/include_priv/android/os/IncidentReportArgs.h
+++ b/libs/incident/include_priv/android/os/IncidentReportArgs.h
@@ -53,6 +53,7 @@
void setReceiverPkg(const string& pkg);
void setReceiverCls(const string& cls);
void addHeader(const vector<uint8_t>& headerProto);
+ void setGzip(bool gzip);
inline bool all() const { return mAll; }
bool containsSection(int section, bool specific) const;
@@ -61,6 +62,7 @@
inline const string& receiverPkg() const { return mReceiverPkg; }
inline const string& receiverCls() const { return mReceiverCls; }
inline const vector<vector<uint8_t>>& headers() const { return mHeaders; }
+ inline bool gzip() const {return mGzip; }
void merge(const IncidentReportArgs& that);
@@ -71,6 +73,7 @@
int mPrivacyPolicy;
string mReceiverPkg;
string mReceiverCls;
+ bool mGzip;
};
}
diff --git a/libs/incident/src/IncidentReportArgs.cpp b/libs/incident/src/IncidentReportArgs.cpp
index 9d8a983..db495cf 100644
--- a/libs/incident/src/IncidentReportArgs.cpp
+++ b/libs/incident/src/IncidentReportArgs.cpp
@@ -26,7 +26,8 @@
IncidentReportArgs::IncidentReportArgs()
:mSections(),
mAll(false),
- mPrivacyPolicy(-1)
+ mPrivacyPolicy(-1),
+ mGzip(false)
{
}
@@ -36,7 +37,8 @@
mAll(that.mAll),
mPrivacyPolicy(that.mPrivacyPolicy),
mReceiverPkg(that.mReceiverPkg),
- mReceiverCls(that.mReceiverCls)
+ mReceiverCls(that.mReceiverCls),
+ mGzip(that.mGzip)
{
}
@@ -93,6 +95,11 @@
return err;
}
+ err = out->writeInt32(mGzip);
+ if (err != NO_ERROR) {
+ return err;
+ }
+
return NO_ERROR;
}
@@ -149,6 +156,15 @@
mReceiverPkg = String8(in->readString16()).string();
mReceiverCls = String8(in->readString16()).string();
+ int32_t gzip;
+ err = in->readInt32(&gzip);
+ if (err != NO_ERROR) {
+ return err;
+ }
+ if (gzip != 0) {
+ mGzip = gzip;
+ }
+
return OK;
}
@@ -193,6 +209,12 @@
mHeaders.push_back(headerProto);
}
+void
+IncidentReportArgs::setGzip(bool gzip)
+{
+ mGzip = gzip;
+}
+
bool
IncidentReportArgs::containsSection(int section, bool specific) const
{