incidentd sections for userdebug and eng are compiled out

Previously, the decision to include or not include them was
done at runtime. This changes them to be behind a compile
time flag. It's just safer, because the code just isn't there
instead of being dependent on a system property.

Test: bit GtsIncidentManagerTestCases:*
Bug: 123543706
Change-Id: If4e611914a7b0acd399ae27e55af8f718aee3ec8

diff --git a/cmds/incidentd/Android.bp b/cmds/incidentd/Android.bp
index 8ac11df..8f9a5f8 100644 (file)
--- a/cmds/incidentd/Android.bp
+++ b/cmds/incidentd/Android.bp
@@ -66,6 +66,13 @@ cc_binary {
         "libplatformprotos",
     ],
 
+    product_variables: {
+        debuggable: {
+            cflags: ["-DALLOW_RESTRICTED_SECTIONS=1"],
+        },
+    },
+
+
     init_rc: ["incidentd.rc"],
 }
 

diff --git a/cmds/incidentd/src/Reporter.cpp b/cmds/incidentd/src/Reporter.cpp
index 7a08dd6..e773e74 100644 (file)
--- a/cmds/incidentd/src/Reporter.cpp
+++ b/cmds/incidentd/src/Reporter.cpp
@@ -26,7 +26,6 @@
 #include "section_list.h"
 
 #include <android-base/file.h>
-#include <android-base/properties.h>
 #include <android/os/DropBoxManager.h>
 #include <android/util/protobuf.h>
 #include <android/util/ProtoOutputStream.h>
@@ -467,8 +466,6 @@ void Reporter::runReport(size_t* reportByteSize) {
 
     IncidentMetadata metadata;
     int persistedPrivacyPolicy = PRIVACY_POLICY_UNSET;
-    std::string buildType = android::base::GetProperty("ro.build.type", "");
-    const bool isUserdebugOrEng = buildType == "userdebug" || buildType == "eng";
 
     (*reportByteSize) = 0;
 
@@ -567,13 +564,6 @@ void Reporter::runReport(size_t* reportByteSize) {
     for (const Section** section = SECTION_LIST; *section; section++) {
         const int sectionId = (*section)->id;
 
-        // If this section is too private for user builds, skip it.
-        if ((*section)->userdebugAndEngOnly && !isUserdebugOrEng) {
-            VLOG("Skipping incident report section %d '%s' because it's limited to userdebug/eng",
-                  sectionId, (*section)->name.string());
-            continue;
-        }
-
         // If nobody wants this section, skip it.
         if (!mBatch->containsSection(sectionId)) {
             continue;

diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index 1e8261e..935a7c4 100644 (file)
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -25,6 +25,7 @@
 #include <set>
 
 #include <android-base/file.h>
+#include <android-base/properties.h>
 #include <android-base/stringprintf.h>
 #include <android/util/protobuf.h>
 #include <android/util/ProtoOutputStream.h>
@@ -63,10 +64,10 @@ static pid_t fork_execute_incident_helper(const int id, Fpipe* p2cPipe, Fpipe* c
 }
 
 // ================================================================================
-Section::Section(int i, int64_t timeoutMs, bool userdebugAndEngOnly)
+Section::Section(int i, int64_t timeoutMs)
     : id(i),
-      timeoutMs(timeoutMs),
-      userdebugAndEngOnly(userdebugAndEngOnly) {}
+      timeoutMs(timeoutMs) {
+}
 
 Section::~Section() {}
 
@@ -74,7 +75,7 @@ Section::~Section() {}
 static inline bool isSysfs(const char* filename) { return strncmp(filename, "/sys/", 5) == 0; }
 
 FileSection::FileSection(int id, const char* filename, const int64_t timeoutMs)
-    : Section(id, timeoutMs, false), mFilename(filename) {
+    : Section(id, timeoutMs), mFilename(filename) {
     name = "file ";
     name += filename;
     mIsSysfs = isSysfs(filename);
@@ -236,8 +237,8 @@ WorkerThreadData::WorkerThreadData(const WorkerThreadSection* sec)
 WorkerThreadData::~WorkerThreadData() {}
 
 // ================================================================================
-WorkerThreadSection::WorkerThreadSection(int id, const int64_t timeoutMs, bool userdebugAndEngOnly)
-    : Section(id, timeoutMs, userdebugAndEngOnly) {}
+WorkerThreadSection::WorkerThreadSection(int id, const int64_t timeoutMs)
+    : Section(id, timeoutMs) {}
 
 WorkerThreadSection::~WorkerThreadSection() {}
 
@@ -425,8 +426,8 @@ status_t CommandSection::Execute(ReportWriter* writer) const {
 }
 
 // ================================================================================
-DumpsysSection::DumpsysSection(int id, bool userdebugAndEngOnly, const char* service, ...)
-    : WorkerThreadSection(id, REMOTE_CALL_TIMEOUT_MS, userdebugAndEngOnly), mService(service) {
+DumpsysSection::DumpsysSection(int id, const char* service, ...)
+    : WorkerThreadSection(id, REMOTE_CALL_TIMEOUT_MS), mService(service) {
     name = "dumpsys ";
     name += service;
 

diff --git a/cmds/incidentd/src/Section.h b/cmds/incidentd/src/Section.h
index f89824c..cfe7e16 100644 (file)
--- a/cmds/incidentd/src/Section.h
+++ b/cmds/incidentd/src/Section.h
@@ -40,10 +40,9 @@ class Section {
 public:
     const int id;
     const int64_t timeoutMs;  // each section must have a timeout
-    const bool userdebugAndEngOnly;
     String8 name;
 
-    Section(int id, int64_t timeoutMs = REMOTE_CALL_TIMEOUT_MS, bool userdebugAndEngOnly = false);
+    Section(int id, int64_t timeoutMs = REMOTE_CALL_TIMEOUT_MS);
     virtual ~Section();
 
     virtual status_t Execute(ReportWriter* writer) const = 0;
@@ -85,8 +84,7 @@ private:
  */
 class WorkerThreadSection : public Section {
 public:
-    WorkerThreadSection(int id, int64_t timeoutMs = REMOTE_CALL_TIMEOUT_MS,
-                        bool userdebugAndEngOnly = false);
+    WorkerThreadSection(int id, int64_t timeoutMs = REMOTE_CALL_TIMEOUT_MS);
     virtual ~WorkerThreadSection();
 
     virtual status_t Execute(ReportWriter* writer) const;
@@ -116,7 +114,7 @@ private:
  */
 class DumpsysSection : public WorkerThreadSection {
 public:
-    DumpsysSection(int id, bool userdebugAndEngOnly, const char* service, ...);
+    DumpsysSection(int id, const char* service, ...);
     virtual ~DumpsysSection();
 
     virtual status_t BlockingCall(int pipeWriteFd) const;
@@ -127,6 +125,21 @@ private:
 };
 
 /**
+ * Section that calls dumpsys on a system service.
+ */
+class SystemPropertyDumpsysSection : public WorkerThreadSection {
+public:
+    SystemPropertyDumpsysSection(int id, const char* service, ...);
+    virtual ~SystemPropertyDumpsysSection();
+
+    virtual status_t BlockingCall(int pipeWriteFd) const;
+
+private:
+    String16 mService;
+    Vector<String16> mArgs;
+};
+
+/**
  * Section that reads from logd.
  */
 class LogSection : public WorkerThreadSection {

diff --git a/tools/incident_section_gen/main.cpp b/tools/incident_section_gen/main.cpp
index f6c9c0e..3b3fe19 100644 (file)
--- a/tools/incident_section_gen/main.cpp
+++ b/tools/incident_section_gen/main.cpp
@@ -408,10 +408,16 @@ static bool generateSectionListCpp(Descriptor const* descriptor) {
     for (int i=0; i<descriptor->field_count(); i++) {
         const FieldDescriptor* field = descriptor->field(i);
 
-        if (field->type() != FieldDescriptor::TYPE_MESSAGE && field->type() != FieldDescriptor::TYPE_STRING) {
+        if (field->type() != FieldDescriptor::TYPE_MESSAGE
+                && field->type() != FieldDescriptor::TYPE_STRING) {
             continue;
         }
+
         const SectionFlags s = getSectionFlags(field);
+        if (s.userdebug_and_eng_only()) {
+            printf("#if ALLOW_RESTRICTED_SECTIONS\n");
+        }
+
         switch (s.type()) {
             case SECTION_NONE:
                 continue;
@@ -424,8 +430,7 @@ static bool generateSectionListCpp(Descriptor const* descriptor) {
                 printf(" NULL),\n");
                 break;
             case SECTION_DUMPSYS:
-                printf("    new DumpsysSection(%d, %s,", field->number(),
-                       s.userdebug_and_eng_only() ? "true" : "false");
+                printf("    new DumpsysSection(%d, ", field->number());
                 splitAndPrint(s.args());
                 printf(" NULL),\n");
                 break;
@@ -438,9 +443,13 @@ static bool generateSectionListCpp(Descriptor const* descriptor) {
                 printf(" NULL),\n");
                 break;
             case SECTION_TOMBSTONE:
-                printf("    new TombstoneSection(%d, \"%s\"),\n", field->number(), s.args().c_str());
+                printf("    new TombstoneSection(%d, \"%s\"),\n", field->number(),
+                        s.args().c_str());
                 break;
         }
+        if (s.userdebug_and_eng_only()) {
+            printf("#endif\n");
+        }
     }
     printf("    NULL };\n");