Merge "dumpstate: save background traces when a BR is captured."

4 files changed, 98 insertions, 2 deletions

diff --git a/cmds/dumpstate/DumpstateUtil.cpp b/cmds/dumpstate/DumpstateUtil.cpp
index eeaa5a3de0..c833d0e6bd 100644
--- a/cmds/dumpstate/DumpstateUtil.cpp
+++ b/cmds/dumpstate/DumpstateUtil.cpp
@@ -124,6 +124,12 @@ CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::Re
     return *this;
 }
 
+CommandOptions::CommandOptionsBuilder&
+CommandOptions::CommandOptionsBuilder::CloseAllFileDescriptorsOnExec() {
+    values.close_all_fds_on_exec_ = true;
+    return *this;
+}
+
 CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::Log(
     const std::string& message) {
     values.logging_message_ = message;
@@ -137,6 +143,7 @@ CommandOptions CommandOptions::CommandOptionsBuilder::Build() {
 CommandOptions::CommandOptionsValues::CommandOptionsValues(int64_t timeout_ms)
     : timeout_ms_(timeout_ms),
       always_(false),
+      close_all_fds_on_exec_(false),
       account_mode_(DONT_DROP_ROOT),
       output_mode_(NORMAL_OUTPUT),
       logging_message_("") {
@@ -157,6 +164,10 @@ bool CommandOptions::Always() const {
     return values.always_;
 }
 
+bool CommandOptions::ShouldCloseAllFileDescriptorsOnExec() const {
+    return values.close_all_fds_on_exec_;
+}
+
 PrivilegeMode CommandOptions::PrivilegeMode() const {
     return values.account_mode_;
 }
@@ -277,7 +288,8 @@ int RunCommandToFd(int fd, const std::string& title, const std::vector<std::stri
         MYLOGI(logging_message.c_str(), command_string.c_str());
     }
 
-    bool silent = (options.OutputMode() == REDIRECT_TO_STDERR);
+    bool silent = (options.OutputMode() == REDIRECT_TO_STDERR ||
+                   options.ShouldCloseAllFileDescriptorsOnExec());
     bool redirecting_to_fd = STDOUT_FILENO != fd;
 
     if (PropertiesHelper::IsDryRun() && !options.Always()) {
@@ -314,7 +326,27 @@ int RunCommandToFd(int fd, const std::string& title, const std::vector<std::stri
             return -1;
         }
 
-        if (silent) {
+        if (options.ShouldCloseAllFileDescriptorsOnExec()) {
+            int devnull_fd = TEMP_FAILURE_RETRY(open("/dev/null", O_RDONLY));
+            TEMP_FAILURE_RETRY(dup2(devnull_fd, STDIN_FILENO));
+            close(devnull_fd);
+            devnull_fd = TEMP_FAILURE_RETRY(open("/dev/null", O_WRONLY));
+            TEMP_FAILURE_RETRY(dup2(devnull_fd, STDOUT_FILENO));
+            TEMP_FAILURE_RETRY(dup2(devnull_fd, STDERR_FILENO));
+            close(devnull_fd);
+            // This is to avoid leaking FDs that, accidentally, have not been
+            // marked as O_CLOEXEC. Leaking FDs across exec can cause failures
+            // when execing a process that has a SELinux auto_trans rule.
+            // Here we assume that the dumpstate process didn't open more than
+            // 1000 FDs. In theory we could iterate through /proc/self/fd/, but
+            // doing that in a fork-safe way is too complex and not worth it
+            // (opendir()/readdir() do heap allocations and take locks).
+            for (int i = 0; i < 1000; i++) {
+                if (i != STDIN_FILENO && i!= STDOUT_FILENO && i != STDERR_FILENO) {
+                    close(i);
+                }
+            }
+        } else if (silent) {
             // Redirects stdout to stderr
             TEMP_FAILURE_RETRY(dup2(STDERR_FILENO, STDOUT_FILENO));
         } else if (redirecting_to_fd) {
diff --git a/cmds/dumpstate/DumpstateUtil.h b/cmds/dumpstate/DumpstateUtil.h
index b099443e32..b00c46e0db 100644
--- a/cmds/dumpstate/DumpstateUtil.h
+++ b/cmds/dumpstate/DumpstateUtil.h
@@ -80,6 +80,7 @@ class CommandOptions {
 
         int64_t timeout_ms_;
         bool always_;
+        bool close_all_fds_on_exec_;
         PrivilegeMode account_mode_;
         OutputMode output_mode_;
         std::string logging_message_;
@@ -112,6 +113,13 @@ class CommandOptions {
         CommandOptionsBuilder& DropRoot();
         /* Sets the command's OutputMode as `REDIRECT_TO_STDERR` */
         CommandOptionsBuilder& RedirectStderr();
+        /* Closes all file descriptors before exec-ing the target process. This
+         * includes also stdio pipes, which are dup-ed on /dev/null. It prevents
+         * leaking opened FDs to the target process, which in turn can hit
+         * selinux denials in presence of auto_trans rules.
+         */
+        CommandOptionsBuilder& CloseAllFileDescriptorsOnExec();
+
         /* When not empty, logs a message before executing the command.
          * Must contain a `%s`, which will be replaced by the full command line, and end on `\n`. */
         CommandOptionsBuilder& Log(const std::string& message);
@@ -130,6 +138,8 @@ class CommandOptions {
     int64_t TimeoutInMs() const;
     /* Checks whether the command should always be run, even on dry-run mode. */
     bool Always() const;
+    /* Checks whether all FDs should be closed prior to the exec() calls. */
+    bool ShouldCloseAllFileDescriptorsOnExec() const;
     /** Gets the PrivilegeMode of the command. */
     PrivilegeMode PrivilegeMode() const;
     /** Gets the OutputMode of the command. */
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index a374c864c5..fbb0a18174 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -175,6 +175,7 @@ void add_mountinfo();
 #define SNAPSHOTCTL_LOG_DIR "/data/misc/snapshotctl_log"
 #define LINKERCONFIG_DIR "/linkerconfig"
 #define PACKAGE_DEX_USE_LIST "/data/system/package-dex-usage.list"
+#define SYSTEM_TRACE_SNAPSHOT "/data/misc/perfetto-traces/bugreport/systrace.pftrace"
 
 // TODO(narayan): Since this information has to be kept in sync
 // with tombstoned, we should just put it in a common header.
@@ -1053,6 +1054,24 @@ static void DumpIncidentReport() {
     }
 }
 
+static void MaybeAddSystemTraceToZip() {
+    // This function copies into the .zip the system trace that was snapshotted
+    // by the early call to MaybeSnapshotSystemTrace(), if any background
+    // tracing was happening.
+    if (!ds.IsZipping()) {
+        MYLOGD("Not dumping system trace because it's not a zipped bugreport\n");
+        return;
+    }
+    if (!ds.has_system_trace_) {
+        // No background trace was happening at the time dumpstate was invoked.
+        return;
+    }
+    ds.AddZipEntry(
+            ZIP_ROOT_DIR + SYSTEM_TRACE_SNAPSHOT,
+            SYSTEM_TRACE_SNAPSHOT);
+    android::os::UnlinkAndLogOnError(SYSTEM_TRACE_SNAPSHOT);
+}
+
 static void DumpVisibleWindowViews() {
     if (!ds.IsZipping()) {
         MYLOGD("Not dumping visible views because it's not a zipped bugreport\n");
@@ -1649,6 +1668,8 @@ static Dumpstate::RunStatus dumpstate() {
 
     AddAnrTraceFiles();
 
+    MaybeAddSystemTraceToZip();
+
     // NOTE: tombstones are always added as separate entries in the zip archive
     // and are not interspersed with the main report.
     const bool tombstones_dumped = AddDumps(ds.tombstone_data_.begin(), ds.tombstone_data_.end(),
@@ -2886,6 +2907,13 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid,
         RunDumpsysCritical();
     }
     MaybeTakeEarlyScreenshot();
+
+    if (!is_dumpstate_restricted) {
+        // Snapshot the system trace now (if running) to avoid that dumpstate's
+        // own activity pushes out interesting data from the trace ring buffer.
+        // The trace file is added to the zip by MaybeAddSystemTraceToZip().
+        MaybeSnapshotSystemTrace();
+    }
     onUiIntensiveBugreportDumpsFinished(calling_uid);
     MaybeCheckUserConsent(calling_uid, calling_package);
     if (options_->telephony_only) {
@@ -2976,6 +3004,26 @@ void Dumpstate::MaybeTakeEarlyScreenshot() {
     TakeScreenshot();
 }
 
+void Dumpstate::MaybeSnapshotSystemTrace() {
+    // If a background system trace is happening and is marked as "suitable for
+    // bugreport" (i.e. bugreport_score > 0 in the trace config), this command
+    // will stop it and serialize into SYSTEM_TRACE_SNAPSHOT. In the (likely)
+    // case that no trace is ongoing, this command is a no-op.
+    // Note: this should not be enqueued as we need to freeze the trace before
+    // dumpstate starts. Otherwise the trace ring buffers will contain mostly
+    // the dumpstate's own activity which is irrelevant.
+    int res = RunCommand(
+        "SERIALIZE PERFETTO TRACE",
+        {"perfetto", "--save-for-bugreport"},
+        CommandOptions::WithTimeout(10)
+            .DropRoot()
+            .CloseAllFileDescriptorsOnExec()
+            .Build());
+    has_system_trace_ = res == 0;
+    // MaybeAddSystemTraceToZip() will take care of copying the trace in the zip
+    // file in the later stages.
+}
+
 void Dumpstate::onUiIntensiveBugreportDumpsFinished(int32_t calling_uid) {
     if (calling_uid == AID_SHELL || !CalledByApi()) {
         return;
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 255243f7e3..f83968b59e 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -458,6 +458,11 @@ class Dumpstate {
     // Whether it should take an screenshot earlier in the process.
     bool do_early_screenshot_ = false;
 
+    // This is set to true when the trace snapshot request in the early call to
+    // MaybeSnapshotSystemTrace(). When this is true, the later stages of
+    // dumpstate will append the trace to the zip archive.
+    bool has_system_trace_ = false;
+
     std::unique_ptr<Progress> progress_;
 
     // When set, defines a socket file-descriptor use to report progress to bugreportz
@@ -543,6 +548,7 @@ class Dumpstate {
     RunStatus DumpstateDefaultAfterCritical();
 
     void MaybeTakeEarlyScreenshot();
+    void MaybeSnapshotSystemTrace();
 
     void onUiIntensiveBugreportDumpsFinished(int32_t calling_uid);