Trace Redaction - Filter suspend resume events
This starts a new pattern for some primitives. Rather than having
a single primitive build the allowlist, this allows multiple
primitives to contribute to the allowlist.
For this change, there is:
1. a collect filter that adds the ftrace event id
2. a ftrace event filter than filters suspend-resume events
As long as item 2 is registered, item 1 can be optionally added to
enable/disable suspend-resume events.
Change-Id: I41d3e10a381e4497d0aa5ba9756c0c3d941a653b
diff --git a/Android.bp b/Android.bp
index 1683b53..3c0b263 100644
--- a/Android.bp
+++ b/Android.bp
@@ -12879,6 +12879,7 @@
"src/trace_redaction/scrub_process_stats.cc",
"src/trace_redaction/scrub_process_trees.cc",
"src/trace_redaction/scrub_trace_packet.cc",
+ "src/trace_redaction/suspend_resume.cc",
"src/trace_redaction/trace_redaction_framework.cc",
"src/trace_redaction/trace_redactor.cc",
],
@@ -12900,6 +12901,7 @@
"src/trace_redaction/redact_process_free_unittest.cc",
"src/trace_redaction/redact_sched_switch_unittest.cc",
"src/trace_redaction/redact_task_newtask_unittest.cc",
+ "src/trace_redaction/suspend_resume_unittest.cc",
],
}
diff --git a/src/trace_redaction/BUILD.gn b/src/trace_redaction/BUILD.gn
index 3dc9cdf..8bfffdd 100644
--- a/src/trace_redaction/BUILD.gn
+++ b/src/trace_redaction/BUILD.gn
@@ -68,6 +68,8 @@
"scrub_process_trees.h",
"scrub_trace_packet.cc",
"scrub_trace_packet.h",
+ "suspend_resume.cc",
+ "suspend_resume.h",
"trace_redaction_framework.cc",
"trace_redaction_framework.h",
"trace_redactor.cc",
@@ -130,6 +132,7 @@
"redact_process_free_unittest.cc",
"redact_sched_switch_unittest.cc",
"redact_task_newtask_unittest.cc",
+ "suspend_resume_unittest.cc",
]
deps = [
":trace_redaction",
diff --git a/src/trace_redaction/main.cc b/src/trace_redaction/main.cc
index 3fa0080..656fdef 100644
--- a/src/trace_redaction/main.cc
+++ b/src/trace_redaction/main.cc
@@ -34,6 +34,7 @@
#include "src/trace_redaction/scrub_process_stats.h"
#include "src/trace_redaction/scrub_process_trees.h"
#include "src/trace_redaction/scrub_trace_packet.h"
+#include "src/trace_redaction/suspend_resume.h"
#include "src/trace_redaction/trace_redaction_framework.h"
#include "src/trace_redaction/trace_redactor.h"
@@ -51,6 +52,7 @@
// Add all builders.
redactor.emplace_build<PopulateAllowlists>();
+ redactor.emplace_build<AllowSuspendResume>();
redactor.emplace_build<OptimizeTimeline>();
// Add all transforms.
@@ -62,6 +64,7 @@
scrub_ftrace_events->emplace_back<FilterPrintEvents>();
scrub_ftrace_events->emplace_back<FilterSchedWakingEvents>();
scrub_ftrace_events->emplace_back<FilterTaskRename>();
+ scrub_ftrace_events->emplace_back<FilterSuspendResume>();
// Scrub packets and ftrace events first as they will remove the largest
// chucks of data from the trace. This will reduce the amount of data that the
diff --git a/src/trace_redaction/populate_allow_lists.cc b/src/trace_redaction/populate_allow_lists.cc
index 01da4a0..380a152 100644
--- a/src/trace_redaction/populate_allow_lists.cc
+++ b/src/trace_redaction/populate_allow_lists.cc
@@ -25,10 +25,6 @@
namespace perfetto::trace_redaction {
base::Status PopulateAllowlists::Build(Context* context) const {
- if (!context->trace_packet_allow_list.empty()) {
- return base::ErrStatus("PopulateAllowlists: allow-list should be empty.");
- }
-
// TRACE PACKET NOTES
//
// protos::pbzero::TracePacket::kAndroidSystemPropertyFieldNumber
@@ -38,7 +34,7 @@
// difficult. Because this packet's value has no measurable, the safest
// option to drop the whole packet.
- context->trace_packet_allow_list = {
+ std::initializer_list<uint> trace_packets = {
protos::pbzero::TracePacket::kProcessTreeFieldNumber,
protos::pbzero::TracePacket::kProcessStatsFieldNumber,
protos::pbzero::TracePacket::kClockSnapshotFieldNumber,
@@ -59,13 +55,16 @@
protos::pbzero::TracePacket::kPackagesListFieldNumber,
};
- context->ftrace_packet_allow_list = {
+ for (auto item : trace_packets) {
+ context->trace_packet_allow_list.insert(item);
+ }
+
+ std::initializer_list<uint> ftrace_events = {
protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber,
protos::pbzero::FtraceEvent::kCpuFrequencyFieldNumber,
protos::pbzero::FtraceEvent::kCpuIdleFieldNumber,
protos::pbzero::FtraceEvent::kSchedBlockedReasonFieldNumber,
protos::pbzero::FtraceEvent::kSchedWakingFieldNumber,
- protos::pbzero::FtraceEvent::kSuspendResumeFieldNumber,
protos::pbzero::FtraceEvent::kTaskNewtaskFieldNumber,
protos::pbzero::FtraceEvent::kTaskRenameFieldNumber,
protos::pbzero::FtraceEvent::kSchedProcessFreeFieldNumber,
@@ -80,21 +79,9 @@
protos::pbzero::FtraceEvent::kPrintFieldNumber,
};
- // TODO: Some ftrace fields should be retained, but they carry too much risk
- // without additional redaction. This list should be configured in a build
- // primitive so that they can be optionally included.
- //
- // protos::pbzero::FtraceEvent::kPrintFieldNumber,
- //
- // TODO: Some fields will create new packets (e.g. binder calls may create
- // new spans. This is currently not supported (generated packets still
- // need to be redacted).
- //
- // protos::pbzero::FtraceEvent::kBinderTransactionFieldNumber,
- // protos::pbzero::FtraceEvent::kBinderTransactionReceivedFieldNumber,
- // protos::pbzero::FtraceEvent::kBinderSetPriorityFieldNumber,
- // protos::pbzero::FtraceEvent::kBinderLockedFieldNumber,
- // protos::pbzero::FtraceEvent::kBinderUnlockFieldNumber,
+ for (auto item : ftrace_events) {
+ context->ftrace_packet_allow_list.insert(item);
+ }
return base::OkStatus();
}
diff --git a/src/trace_redaction/suspend_resume.cc b/src/trace_redaction/suspend_resume.cc
new file mode 100644
index 0000000..243446f
--- /dev/null
+++ b/src/trace_redaction/suspend_resume.cc
@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_redaction/suspend_resume.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
+#include "protos/perfetto/trace/ftrace/power.pbzero.h"
+
+namespace perfetto::trace_redaction {
+
+base::Status AllowSuspendResume::Build(Context* context) const {
+ context->ftrace_packet_allow_list.insert(
+ protos::pbzero::FtraceEvent::kSuspendResumeFieldNumber);
+
+ // Values are taken from "suspend_period.textproto".
+ context->suspend_result_allow_list.insert("syscore_suspend");
+ context->suspend_result_allow_list.insert("syscore_resume");
+ context->suspend_result_allow_list.insert("timekeeping_freeze");
+
+ return base::OkStatus();
+}
+
+base::Status FilterSuspendResume::VerifyContext(const Context&) const {
+ // FilterSuspendResume could check if kSuspendResumeFieldNumber is present in
+ // ftrace_packet_allow_list and there are values in the
+ // suspend_result_allow_list, but would make it hard to enable/disable
+ // suspend-resume redaction.
+ return base::OkStatus();
+}
+
+// The ftrace event is passed in.
+bool FilterSuspendResume::KeepEvent(const Context& context,
+ protozero::ConstBytes bytes) const {
+ protozero::ProtoDecoder event_decoder(bytes);
+
+ auto suspend_resume = event_decoder.FindField(
+ protos::pbzero::FtraceEvent::kSuspendResumeFieldNumber);
+
+ // It's not a suspend-resume event, defer the decision to another filter.
+ if (!suspend_resume.valid()) {
+ return true;
+ }
+
+ protozero::ProtoDecoder suspend_resume_decoder(suspend_resume.as_bytes());
+
+ auto action = suspend_resume_decoder.FindField(
+ protos::pbzero::SuspendResumeFtraceEvent::kActionFieldNumber);
+
+ return !action.valid() ||
+ context.suspend_result_allow_list.count(action.as_std_string());
+}
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/suspend_resume.h b/src/trace_redaction/suspend_resume.h
new file mode 100644
index 0000000..09f7bd8
--- /dev/null
+++ b/src/trace_redaction/suspend_resume.h
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SRC_TRACE_REDACTION_SUSPEND_RESUME_H_
+#define SRC_TRACE_REDACTION_SUSPEND_RESUME_H_
+
+#include "src/trace_redaction/scrub_ftrace_events.h"
+#include "src/trace_redaction/trace_redaction_framework.h"
+
+namespace perfetto::trace_redaction {
+
+// Updates allowlists to include suspend-resume events and which events to allow
+// through.
+class AllowSuspendResume : public BuildPrimitive {
+ public:
+ base::Status Build(Context* context) const override;
+};
+
+// Filters ftrace events based on the suspend-resume event.
+class FilterSuspendResume : public FtraceEventFilter {
+ public:
+ base::Status VerifyContext(const Context& context) const override;
+
+ bool KeepEvent(const Context& context,
+ protozero::ConstBytes bytes) const override;
+};
+
+} // namespace perfetto::trace_redaction
+
+#endif // SRC_TRACE_REDACTION_SUSPEND_RESUME_H_
diff --git a/src/trace_redaction/suspend_resume_unittest.cc b/src/trace_redaction/suspend_resume_unittest.cc
new file mode 100644
index 0000000..7cf498a
--- /dev/null
+++ b/src/trace_redaction/suspend_resume_unittest.cc
@@ -0,0 +1,149 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_redaction/suspend_resume.h"
+#include "src/base/test/status_matchers.h"
+#include "test/gtest_and_gmock.h"
+
+#include "protos/perfetto/trace/ftrace/ftrace.gen.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event.gen.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
+#include "protos/perfetto/trace/ftrace/power.gen.h"
+
+namespace perfetto::trace_redaction {
+
+TEST(AllowSuspendResumeTest, UpdatesTracePacketAllowlist) {
+ Context context;
+
+ // Start with a non-empty allow-list item.
+ context.ftrace_packet_allow_list.insert(
+ protos::pbzero::FtraceEvent::kPrintFieldNumber);
+
+ ASSERT_EQ(context.ftrace_packet_allow_list.size(), 1u);
+
+ AllowSuspendResume allow;
+ auto status = allow.Build(&context);
+ ASSERT_OK(status) << status.message();
+
+ // Print should still be present. The allowlist should have been updated, not
+ // replaced.
+ ASSERT_EQ(context.ftrace_packet_allow_list.count(
+ protos::pbzero::FtraceEvent::kPrintFieldNumber),
+ 1u);
+
+ ASSERT_EQ(context.ftrace_packet_allow_list.count(
+ protos::pbzero::FtraceEvent::kSuspendResumeFieldNumber),
+ 1u);
+}
+
+TEST(AllowSuspendResumeTest, UpdatesSuspendResumeAllowlist) {
+ Context context;
+
+ ASSERT_TRUE(context.suspend_result_allow_list.empty());
+
+ AllowSuspendResume allow;
+ auto status = allow.Build(&context);
+ ASSERT_OK(status) << status.message();
+
+ ASSERT_FALSE(context.suspend_result_allow_list.empty());
+}
+
+class SuspendResumeTest : public testing::Test {
+ protected:
+ void SetUp() {
+ AllowSuspendResume allow;
+ ASSERT_OK(allow.Build(&context_));
+ }
+
+ protos::gen::FtraceEvent CreateSuspendResumeEvent(
+ const std::string* action) const {
+ protos::gen::FtraceEvent event;
+ event.set_timestamp(1234);
+ event.set_pid(0);
+
+ auto* suspend_resume = event.mutable_suspend_resume();
+
+ if (action) {
+ suspend_resume->set_action(*action);
+ }
+
+ return event;
+ }
+
+ protos::gen::FtraceEvent CreateOtherEvent() const {
+ protos::gen::FtraceEvent event;
+ event.set_timestamp(1234);
+ event.set_pid(0);
+
+ auto* print = event.mutable_print();
+ print->set_buf("This is a message");
+
+ return event;
+ }
+
+ const Context& context() const { return context_; }
+
+ private:
+ Context context_;
+};
+
+// The suspend-resume filter is not responsible for non-suspend-resume events.
+// It should assume that another filter will handle it and it should just allow
+// those events through
+TEST_F(SuspendResumeTest, AcceptsOtherEvents) {
+ auto event = CreateOtherEvent();
+ auto event_array = event.SerializeAsArray();
+ protozero::ConstBytes event_bytes{event_array.data(), event_array.size()};
+
+ FilterSuspendResume filter;
+ ASSERT_TRUE(filter.KeepEvent(context(), event_bytes));
+}
+
+TEST_F(SuspendResumeTest, AcceptsEventsWithNoName) {
+ auto event = CreateSuspendResumeEvent(nullptr);
+ auto event_array = event.SerializeAsArray();
+ protozero::ConstBytes event_bytes{event_array.data(), event_array.size()};
+
+ Context context;
+
+ FilterSuspendResume filter;
+ ASSERT_TRUE(filter.KeepEvent(context, event_bytes));
+}
+
+TEST_F(SuspendResumeTest, AcceptsEventsWithValidName) {
+ // This value is from "src/trace_redaction/suspend_resume.cc".
+ std::string name = "syscore_suspend";
+
+ auto event = CreateSuspendResumeEvent(&name);
+ auto event_array = event.SerializeAsArray();
+ protozero::ConstBytes event_bytes{event_array.data(), event_array.size()};
+
+ FilterSuspendResume filter;
+ ASSERT_TRUE(filter.KeepEvent(context(), event_bytes));
+}
+
+TEST_F(SuspendResumeTest, RejectsEventsWithInvalidName) {
+ std::string name = "hello world";
+
+ auto event = CreateSuspendResumeEvent(&name);
+ auto event_array = event.SerializeAsArray();
+ protozero::ConstBytes event_bytes{event_array.data(), event_array.size()};
+
+ FilterSuspendResume filter;
+ ASSERT_FALSE(filter.KeepEvent(context(), event_bytes));
+}
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/trace_redaction_framework.h b/src/trace_redaction/trace_redaction_framework.h
index c1c7e45..c76a651 100644
--- a/src/trace_redaction/trace_redaction_framework.h
+++ b/src/trace_redaction/trace_redaction_framework.h
@@ -159,6 +159,17 @@
// ftrace event
base::FlatSet<uint32_t> ftrace_packet_allow_list;
+ // message SuspendResumeFtraceEvent {
+ // optional string action = 1 [(datapol.semantic_type) = ST_NOT_REQUIRED];
+ // optional int32 val = 2;
+ // optional uint32 start = 3 [(datapol.semantic_type) = ST_NOT_REQUIRED];
+ // }
+ //
+ // The "action" in SuspendResumeFtraceEvent is a free-form string. There are
+ // some know and expected values. Those values are stored here and all events
+ // who's action value is not found here, the ftrace event will be dropped.
+ base::FlatSet<std::string> suspend_result_allow_list;
+
// The timeline is a query-focused data structure that connects a pid to a
// uid at specific point in time.
//
