diff options
author | Mike Ma <yanmin@google.com> | 2020-03-03 17:58:35 -0800 |
---|---|---|
committer | Mike Ma <yanmin@google.com> | 2020-03-05 12:42:50 -0800 |
commit | b6f7c4725253de798cd7487f671c66663614a158 (patch) | |
tree | 0c23162f1db918f57f51cd9feb56b13550bc55c0 /cmds/incidentd | |
parent | f7ed9bc589774fcc911321bae5c6521397e95aef (diff) |
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
Diffstat (limited to 'cmds/incidentd')
-rw-r--r-- | cmds/incidentd/src/Reporter.cpp | 60 | ||||
-rw-r--r-- | cmds/incidentd/src/Reporter.h | 7 | ||||
-rw-r--r-- | cmds/incidentd/src/WorkDirectory.cpp | 36 | ||||
-rw-r--r-- | cmds/incidentd/src/incidentd_util.cpp | 107 | ||||
-rw-r--r-- | cmds/incidentd/src/incidentd_util.h | 23 | ||||
-rw-r--r-- | cmds/incidentd/src/report_file.proto | 5 |
6 files changed, 203 insertions, 35 deletions
diff --git a/cmds/incidentd/src/Reporter.cpp b/cmds/incidentd/src/Reporter.cpp index aa40f85fd340..ad253422452e 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 @@ using namespace android::util; * 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 @@ ReportRequest::ReportRequest(const IncidentReportArgs& a, mListener(listener), mFd(fd), mIsStreaming(fd >= 0), - mStatus(NO_ERROR) { + mStatus(OK), + mZipPid(-1) { } ReportRequest::~ReportRequest() { @@ -153,7 +158,14 @@ ReportRequest::~ReportRequest() { } 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 @@ bool ReportRequest::containsSection(int sectionId) const { } 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 @@ void Reporter::runReport(size_t* reportByteSize) { 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 cbc8b136e7e3..bd47a23d369e 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 @@ public: 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 @@ private: 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 9963533c08ac..1944d6ecc720 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 @@ const ComponentName DROPBOX_SENTINEL("android", "DROPBOX"); /** 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 @@ void ReportFile::addReport(const IncidentReportArgs& args) { 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 @@ status_t ReportFile::startFilteringData(int writeFd, const IncidentReportArgs& a 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 @@ status_t ReportFile::startFilteringData(int writeFd, const IncidentReportArgs& a } 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 @@ void WorkDirectory::commitAll(const string& pkg) { 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 @@ void get_args_from_report(IncidentReportArgs* out, const ReportFileProto_Report& 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 dfaf89392f90..2649fb975792 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::readFd() { return mRead; } 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 @@ uint64_t Nanotime() { } // ================================================================================ -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 @@ static status_t statusCode(int status) { 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 cc30768fa704..a54993fed42d 100644 --- a/cmds/incidentd/src/incidentd_util.h +++ b/cmds/incidentd/src/incidentd_util.h @@ -56,11 +56,24 @@ private: }; /** - * 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, 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[], Fpipe* input, Fpipe* output); +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 @@ 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); +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 7563da2c2148..85fd2da6c65c 100644 --- a/cmds/incidentd/src/report_file.proto +++ b/cmds/incidentd/src/report_file.proto @@ -65,6 +65,11 @@ message ReportFileProto { * the given client. */ optional bool share_approved = 8; + + /** + * Whether the report is gzipped. + */ + optional bool gzip = 9; } /** |