Merge "dumpstate: save background traces when a BR is captured." am: 8592b080ec am: 069ec16537 am: 2d69bdaa92
Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/1516517
MUST ONLY BE SUBMITTED BY AUTOMERGER
Change-Id: I729de4f188a3b2464178f366d06e60ae6851a878
diff --git a/cmds/dumpstate/DumpstateUtil.cpp b/cmds/dumpstate/DumpstateUtil.cpp
index eeaa5a3..c833d0e 100644
--- a/cmds/dumpstate/DumpstateUtil.cpp
+++ b/cmds/dumpstate/DumpstateUtil.cpp
@@ -124,6 +124,12 @@
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::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 @@
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 @@
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 @@
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 b099443..b00c46e 100644
--- a/cmds/dumpstate/DumpstateUtil.h
+++ b/cmds/dumpstate/DumpstateUtil.h
@@ -80,6 +80,7 @@
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 @@
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 @@
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 6cb8d83..4a4a510 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -175,6 +175,7 @@
#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 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");
@@ -1650,6 +1669,8 @@
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(),
@@ -2887,6 +2908,13 @@
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) {
@@ -2977,6 +3005,26 @@
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 255243f..f83968b 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -458,6 +458,11 @@
// 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 @@
RunStatus DumpstateDefaultAfterCritical();
void MaybeTakeEarlyScreenshot();
+ void MaybeSnapshotSystemTrace();
void onUiIntensiveBugreportDumpsFinished(int32_t calling_uid);