Merge "Add an option to zip incident report" into rvc-dev

cmds/incident/main.cpp

@@ -231,6 +231,7 @@ usage(FILE* out)
     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");
@@ -255,7 +256,7 @@ main(int argc, char** argv)
 
     // 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);
@@ -302,6 +303,9 @@ main(int argc, char** argv)
                 destination = DEST_BROADCAST;
                 receiverArg = optarg;
                 break;
+            case 'z':
+                args.setGzip(true);
+                break;
             default:
                 usage(stderr);
                 return 1;

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).

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;
 };
 
 // ================================================================================

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;
 }
 
@@ -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++) {

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) {
+pid_t fork_execute_cmd(char* const argv[], Fpipe* input, Fpipe* output, int* status) {
-    // fork used in multithreaded environment, avoid adding unnecessary code in child process
+    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 ||
+        // In child
-                              !input->close())) {
+        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 ||
+        if (TEMP_FAILURE_RETRY(dup2(out, STDOUT_FILENO)) < 0 || close(out)) {
-            !output->close()) {
             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.
+    // In parent
-    if (input != NULL) input->readFd().reset();
+    if ((in >= 0 && close(in) < 0) || close(out) < 0) {
-    output->writeFd().reset();
+        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,26 +156,65 @@ 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;
+    if (waitpid_with_timeout(pid, timeout_ms, &status)) {
-    // 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);
     }
+    return kill_child(pid);
+}
 
 }  // namespace incidentd
 }  // namespace os

cmds/incidentd/src/incidentd_util.h

@@ -56,11 +56,24 @@ private:
 };
 
 /**
- * Forks and exec a command with two pipes, one connects stdin for input,
+ * Forks and exec a command with two pipes and returns the pid of the child, or -1 when it fails.
- * 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.
+ * 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 @@ 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);
 

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;
     }
 
     /**

libs/incident/include_priv/android/os/IncidentReportArgs.h

@@ -53,6 +53,7 @@ public:
     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 @@ public:
     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 @@ private:
     int mPrivacyPolicy;
     string mReceiverPkg;
     string mReceiverCls;
+    bool mGzip;
 };
 
 }

libs/incident/src/IncidentReportArgs.cpp

@@ -26,7 +26,8 @@ namespace os {
 IncidentReportArgs::IncidentReportArgs()
     :mSections(),
      mAll(false),
-     mPrivacyPolicy(-1)
+     mPrivacyPolicy(-1),
+     mGzip(false)
 {
 }
 
@@ -36,7 +37,8 @@ IncidentReportArgs::IncidentReportArgs(const IncidentReportArgs& that)
      mAll(that.mAll),
      mPrivacyPolicy(that.mPrivacyPolicy),
      mReceiverPkg(that.mReceiverPkg),
-     mReceiverCls(that.mReceiverCls)
+     mReceiverCls(that.mReceiverCls),
+     mGzip(that.mGzip)
 {
 }
 
@@ -93,6 +95,11 @@ IncidentReportArgs::writeToParcel(Parcel* out) const
         return err;
     }
 
+    err = out->writeInt32(mGzip);
+    if (err != NO_ERROR) {
+        return err;
+    }
+
     return NO_ERROR;
 }
 
@@ -149,6 +156,15 @@ IncidentReportArgs::readFromParcel(const Parcel* in)
     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 @@ IncidentReportArgs::addHeader(const vector<uint8_t>& headerProto)
     mHeaders.push_back(headerProto);
 }
 
+void
+IncidentReportArgs::setGzip(bool gzip)
+{
+    mGzip = gzip;
+}
+
 bool
 IncidentReportArgs::containsSection(int section, bool specific) const
 {