Add an API to dump incident report for dumpstate

Instead of just relying on the regular iteration through the system
services inside dumpstate, add another API to IIncidentManager
dedicated for dumpstate.

- It is only callable by dumpstate() (check the calling uid)
- It has the same behavior as the current call inside dump()

Advantages:
- More explicit function name, right next to takeIncidentReport will
  make it easier to keep them in sync.
- Nobody else can call it, make security easier.
- If dumpstate calls it explicitly, it can skip the 10 second timeout
- The regular dump() call should provide debugging data about
  incidentd itself, for example timestamps for the most recent N
  incident reports taken and the current state of the work directory,
  allowing us to debug incidentd itself.

Bug: 137493082
Test: Manually trigger a bug report, and verify
      /proto/incident_log.proto in the zip file.

Change-Id: I19139c765b53ede63d3beb3ea3ac40ada1aba42d

4 files changed, 82 insertions, 62 deletions

diff --git a/cmds/incident/main.cpp b/cmds/incident/main.cpp
index dfb4f994b866..6c3d19715c2e 100644
--- a/cmds/incident/main.cpp
+++ b/cmds/incident/main.cpp
@@ -198,6 +198,26 @@ parse_receiver_arg(const string& arg, string* pkg, string* cls)
 }
 
 // ================================================================================
+static int
+stream_output(const int read_fd, const int write_fd) {
+    while (true) {
+        uint8_t buf[4096];
+        ssize_t amt = TEMP_FAILURE_RETRY(read(read_fd, buf, sizeof(buf)));
+        if (amt < 0) {
+            break;
+        } else if (amt == 0) {
+            break;
+        }
+
+        ssize_t wamt = TEMP_FAILURE_RETRY(write(write_fd, buf, amt));
+        if (wamt != amt) {
+            return errno;
+        }
+    }
+    return 0;
+}
+
+// ================================================================================
 static void
 usage(FILE* out)
 {
@@ -208,11 +228,13 @@ usage(FILE* out)
     fprintf(out, "OPTIONS\n");
     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, "\n");
     fprintf(out, "and one of these destinations:\n");
     fprintf(out, "  -b           (default) print the report to stdout (in proto format)\n");
     fprintf(out, "  -d           send the report into dropbox\n");
-    fprintf(out, "  -r REASON    human readable description of why the report is taken.\n");
+    fprintf(out, "  -u           print a full report to stdout for dumpstate to zip as a bug\n");
+    fprintf(out, "               report. SECTION is ignored. Should only be called by dumpstate.\n");
     fprintf(out, "  -s PKG/CLS   send broadcast to the broadcast receiver.\n");
     fprintf(out, "\n");
     fprintf(out, "  SECTION     the field numbers of the incident report fields to include\n");
@@ -224,14 +246,14 @@ main(int argc, char** argv)
 {
     Status status;
     IncidentReportArgs args;
-    enum { DEST_UNSET, DEST_DROPBOX, DEST_STDOUT, DEST_BROADCAST } destination = DEST_UNSET;
+    enum { DEST_UNSET, DEST_DROPBOX, DEST_STDOUT, DEST_BROADCAST, DEST_DUMPSTATE } destination = DEST_UNSET;
     int privacyPolicy = PRIVACY_POLICY_AUTOMATIC;
     string reason;
     string receiverArg;
 
     // Parse the args
     int opt;
-    while ((opt = getopt(argc, argv, "bhdlp:r:s:")) != -1) {
+    while ((opt = getopt(argc, argv, "bhdlp:r:s:u")) != -1) {
         switch (opt) {
             case 'h':
                 usage(stdout);
@@ -253,6 +275,13 @@ main(int argc, char** argv)
                 }
                 destination = DEST_DROPBOX;
                 break;
+            case 'u':
+                if (!(destination == DEST_UNSET || destination == DEST_DUMPSTATE)) {
+                    usage(stderr);
+                    return 1;
+                }
+                destination = DEST_DUMPSTATE;
+                break;
             case 'p':
                 privacyPolicy = get_privacy_policy(optarg);
                 break;
@@ -355,21 +384,16 @@ main(int argc, char** argv)
 
         // Wait for the result and print out the data they send.
         //IPCThreadState::self()->joinThreadPool();
-
-        while (true) {
-            uint8_t buf[4096];
-            ssize_t amt = TEMP_FAILURE_RETRY(read(fds[0], buf, sizeof(buf)));
-            if (amt < 0) {
-                break;
-            } else if (amt == 0) {
-                break;
-            }
-
-            ssize_t wamt = TEMP_FAILURE_RETRY(write(STDOUT_FILENO, buf, amt));
-            if (wamt != amt) {
-                return errno;
-            }
+        return stream_output(fds[0], STDOUT_FILENO);
+    } else if (destination == DEST_DUMPSTATE) {
+        // Call into the service
+        sp<StatusListener> listener(new StatusListener());
+        status = service->reportIncidentToDumpstate(writeEnd, listener);
+        if (!status.isOk()) {
+            fprintf(stderr, "reportIncident returned \"%s\"\n", status.toString8().string());
+            return 1;
         }
+        return stream_output(fds[0], STDOUT_FILENO);
     } else {
         status = service->reportIncident(args);
         if (!status.isOk()) {
diff --git a/cmds/incidentd/src/IncidentService.cpp b/cmds/incidentd/src/IncidentService.cpp
index a52726396b53..999936bda1d3 100644
--- a/cmds/incidentd/src/IncidentService.cpp
+++ b/cmds/incidentd/src/IncidentService.cpp
@@ -46,12 +46,11 @@ enum {
 #define DEFAULT_BYTES_SIZE_LIMIT (96 * 1024 * 1024)        // 96MB
 #define DEFAULT_REFACTORY_PERIOD_MS (24 * 60 * 60 * 1000)  // 1 Day
 
-// Skip these sections for dumpstate only. Dumpstate allows 10s max for each service to dump.
+// Skip these sections (for dumpstate only)
 // Skip logs (1100 - 1108) and traces (1200 - 1202) because they are already in the bug report.
-// Skip 3018 because it takes too long.
-#define SKIPPED_SECTIONS { 1100, 1101, 1102, 1103, 1104, 1105, 1106, 1107, 1108, /* Logs */ \
-                           1200, 1201, 1202, /* Native, hal, java traces */ \
-                           3018  /* "meminfo -a --proto" */ }
+#define SKIPPED_DUMPSTATE_SECTIONS { \
+            1100, 1101, 1102, 1103, 1104, 1105, 1106, 1107, 1108, /* Logs */ \
+            1200, 1201, 1202, /* Native, hal, java traces */ }
 
 namespace android {
 namespace os {
@@ -307,6 +306,39 @@ Status IncidentService::reportIncidentToStream(const IncidentReportArgs& args,
     return Status::ok();
 }
 
+Status IncidentService::reportIncidentToDumpstate(const unique_fd& stream,
+        const sp<IIncidentReportStatusListener>& listener) {
+    uid_t caller = IPCThreadState::self()->getCallingUid();
+    if (caller != AID_ROOT && caller != AID_SHELL) {
+        ALOGW("Calling uid %d does not have permission: only ROOT or SHELL allowed", caller);
+        return Status::fromExceptionCode(Status::EX_SECURITY, "Only ROOT or SHELL allowed");
+    }
+
+    ALOGD("Stream incident report to dumpstate");
+    IncidentReportArgs incidentArgs;
+    // Privacy policy for dumpstate incident reports is always EXPLICIT.
+    incidentArgs.setPrivacyPolicy(PRIVACY_POLICY_EXPLICIT);
+
+    int skipped[] = SKIPPED_DUMPSTATE_SECTIONS;
+    for (const Section** section = SECTION_LIST; *section; section++) {
+        const int id = (*section)->id;
+        if (std::find(std::begin(skipped), std::end(skipped), id) == std::end(skipped)
+                && !section_requires_specific_mention(id)) {
+            incidentArgs.addSection(id);
+        }
+    }
+
+    // The ReportRequest takes ownership of the fd, so we need to dup it.
+    int fd = dup(stream.get());
+    if (fd < 0) {
+        return Status::fromStatusT(-errno);
+    }
+
+    mHandler->scheduleStreamingReport(incidentArgs, listener, fd);
+
+    return Status::ok();
+}
+
 Status IncidentService::systemRunning() {
     if (IPCThreadState::self()->getCallingUid() != AID_SYSTEM) {
         return Status::fromExceptionCode(Status::EX_SECURITY,
@@ -551,43 +583,6 @@ status_t IncidentService::cmd_privacy(FILE* in, FILE* out, FILE* err, Vector<Str
     return NO_ERROR;
 }
 
-status_t IncidentService::dump(int fd, const Vector<String16>& args) {
-    if (std::find(args.begin(), args.end(), String16("--proto")) == args.end()) {
-        ALOGD("Skip dumping incident. Only proto format is supported.");
-        dprintf(fd, "Incident dump only supports proto version.\n");
-        return NO_ERROR;
-    }
-
-    ALOGD("Dump incident proto");
-    IncidentReportArgs incidentArgs;
-    incidentArgs.setPrivacyPolicy(PRIVACY_POLICY_EXPLICIT);
-    int skipped[] = SKIPPED_SECTIONS;
-    for (const Section** section = SECTION_LIST; *section; section++) {
-        const int id = (*section)->id;
-        if (std::find(std::begin(skipped), std::end(skipped), id) == std::end(skipped)
-                && !section_requires_specific_mention(id)) {
-            incidentArgs.addSection(id);
-        }
-    }
-
-    if (!checkIncidentPermissions(incidentArgs).isOk()) {
-        return PERMISSION_DENIED;
-    }
-
-    // The ReportRequest takes ownership of the fd, so we need to dup it.
-    int fd1 = dup(fd);
-    if (fd1 < 0) {
-        return -errno;
-    }
-
-    // TODO: Remove this.  Someone even dumpstate, wanting to get an incident report
-    // should use the API.  That will take making dumpstated call the API, which is a
-    // good thing.  It also means it won't be subject to the timeout.
-    mHandler->scheduleStreamingReport(incidentArgs, NULL, fd1);
-
-    return NO_ERROR;
-}
-
 }  // namespace incidentd
 }  // namespace os
 }  // namespace android
diff --git a/cmds/incidentd/src/IncidentService.h b/cmds/incidentd/src/IncidentService.h
index 6481321bbd69..fb013d0bf92d 100644
--- a/cmds/incidentd/src/IncidentService.h
+++ b/cmds/incidentd/src/IncidentService.h
@@ -123,6 +123,9 @@ public:
                                           const sp<IIncidentReportStatusListener>& listener,
                                           const unique_fd& stream);
 
+    virtual Status reportIncidentToDumpstate(const unique_fd& stream,
+            const sp<IIncidentReportStatusListener>& listener);
+
     virtual Status systemRunning();
 
     virtual Status getIncidentReportList(const String16& pkg, const String16& cls,
@@ -140,7 +143,6 @@ public:
     virtual status_t onTransact(uint32_t code, const Parcel& data, Parcel* reply,
                                 uint32_t flags) override;
     virtual status_t command(FILE* in, FILE* out, FILE* err, Vector<String8>& args);
-    virtual status_t dump(int fd, const Vector<String16>& args);
 
 private:
     sp<WorkDirectory> mWorkDirectory;
diff --git a/cmds/incidentd/src/main.cpp b/cmds/incidentd/src/main.cpp
index 098d74ecd786..494882336611 100644
--- a/cmds/incidentd/src/main.cpp
+++ b/cmds/incidentd/src/main.cpp
@@ -45,8 +45,7 @@ int main(int /*argc*/, char** /*argv*/) {
 
     // Create the service
     sp<IncidentService> service = new IncidentService(looper);
-    if (defaultServiceManager()->addService(String16("incident"), service, false,
-            IServiceManager::DUMP_FLAG_PRIORITY_NORMAL | IServiceManager::DUMP_FLAG_PROTO) != 0) {
+    if (defaultServiceManager()->addService(String16("incident"), service) != 0) {
         ALOGE("Failed to add service");
         return -1;
     }