Trace Redaction - Broadphase Filtering
Previously, whole packets were dropped when they did not pass the
allow-list check. This meant important information abount
packet-to-packet timing and data-loss was removed.
This CL changes all broadphase filtering code to use a mask. This
is applied at the packet level and at the ftrace event packet.
This has shown important differences when viewing traces as additional
CPU time used by the target process increases.
A side effect of this change is non-target timeline information has
also showed up. An additional change will be needed to address the
timeline information.
Change-Id: I0d042b48a0c9cc4086e1c07f3072807593ba99dc
diff --git a/Android.bp b/Android.bp
index 09433f2..53cabd1 100644
--- a/Android.bp
+++ b/Android.bp
@@ -13603,10 +13603,10 @@
filegroup {
name: "perfetto_src_trace_redaction_trace_redaction",
srcs: [
+ "src/trace_redaction/broadphase_packet_filter.cc",
"src/trace_redaction/collect_frame_cookies.cc",
"src/trace_redaction/collect_system_info.cc",
"src/trace_redaction/collect_timeline_events.cc",
- "src/trace_redaction/filter_packet_using_allowlist.cc",
"src/trace_redaction/filtering.cc",
"src/trace_redaction/find_package_uid.cc",
"src/trace_redaction/merge_threads.cc",
@@ -13631,11 +13631,10 @@
filegroup {
name: "perfetto_src_trace_redaction_unittests",
srcs: [
+ "src/trace_redaction/broadphase_packet_filter_unittest.cc",
"src/trace_redaction/collect_frame_cookies_unittest.cc",
"src/trace_redaction/collect_system_info_unittest.cc",
"src/trace_redaction/collect_timeline_events_unittest.cc",
- "src/trace_redaction/filter_ftrace_using_allowlist_unittest.cc",
- "src/trace_redaction/filter_packet_using_allowlist_unittest.cc",
"src/trace_redaction/filter_sched_waking_events_unittest.cc",
"src/trace_redaction/find_package_uid_unittest.cc",
"src/trace_redaction/process_thread_timeline_unittest.cc",
diff --git a/src/trace_redaction/BUILD.gn b/src/trace_redaction/BUILD.gn
index 71b51b3..72df74b 100644
--- a/src/trace_redaction/BUILD.gn
+++ b/src/trace_redaction/BUILD.gn
@@ -28,14 +28,14 @@
source_set("trace_redaction") {
sources = [
+ "broadphase_packet_filter.cc",
+ "broadphase_packet_filter.h",
"collect_frame_cookies.cc",
"collect_frame_cookies.h",
"collect_system_info.cc",
"collect_system_info.h",
"collect_timeline_events.cc",
"collect_timeline_events.h",
- "filter_packet_using_allowlist.cc",
- "filter_packet_using_allowlist.h",
"filtering.cc",
"filtering.h",
"find_package_uid.cc",
@@ -91,14 +91,13 @@
source_set("integrationtests") {
testonly = true
sources = [
- "filter_ftrace_using_allowlist_integrationtest.cc",
+ "boardphase_packet_filter_integrationtest.cc",
"filter_sched_waking_events_integrationtest.cc",
"filter_task_rename_integrationtest.cc",
"process_thread_timeline_integrationtest.cc",
"prune_package_list_integrationtest.cc",
"redact_process_trees_integrationtest.cc",
"redact_sched_events_integrationtest.cc",
- "scrub_ftrace_events_integrationtest.cc",
"scrub_process_stats_integrationtest.cc",
"trace_redaction_integration_fixture.cc",
"trace_redaction_integration_fixture.h",
@@ -122,11 +121,10 @@
perfetto_unittest_source_set("unittests") {
testonly = true
sources = [
+ "broadphase_packet_filter_unittest.cc",
"collect_frame_cookies_unittest.cc",
"collect_system_info_unittest.cc",
"collect_timeline_events_unittest.cc",
- "filter_ftrace_using_allowlist_unittest.cc",
- "filter_packet_using_allowlist_unittest.cc",
"filter_sched_waking_events_unittest.cc",
"find_package_uid_unittest.cc",
"process_thread_timeline_unittest.cc",
diff --git a/src/trace_redaction/boardphase_packet_filter_integrationtest.cc b/src/trace_redaction/boardphase_packet_filter_integrationtest.cc
new file mode 100644
index 0000000..7c3d2fc
--- /dev/null
+++ b/src/trace_redaction/boardphase_packet_filter_integrationtest.cc
@@ -0,0 +1,132 @@
+/*
+ * 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 <string>
+
+#include "src/base/test/status_matchers.h"
+#include "src/trace_redaction/broadphase_packet_filter.h"
+#include "src/trace_redaction/populate_allow_lists.h"
+#include "src/trace_redaction/trace_redaction_integration_fixture.h"
+#include "test/gtest_and_gmock.h"
+
+#include "protos/perfetto/trace/trace.pbzero.h"
+#include "protos/perfetto/trace/trace_packet.pbzero.h"
+
+namespace perfetto::trace_redaction {
+
+class BroadphasePacketFilterIntegrationTest
+ : public testing::Test,
+ protected TraceRedactionIntegrationFixure {
+ protected:
+ void SetUp() override {
+ trace_redactor()->emplace_build<PopulateAllowlists>();
+ trace_redactor()->emplace_transform<BroadphasePacketFilter>();
+ }
+
+ Context::TracePacketMask ScanPacketFields(const std::string& trace) {
+ protos::pbzero::Trace::Decoder trace_decoder(trace);
+
+ Context::TracePacketMask mask;
+
+ for (auto packet = trace_decoder.packet(); packet; ++packet) {
+ protozero::ProtoDecoder decoder(*packet);
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ PERFETTO_DCHECK(field.id() < mask.size());
+ mask.set(field.id());
+ }
+ }
+
+ return mask;
+ }
+
+ Context::FtraceEventMask ScanFtraceEventFields(const std::string& buffer) {
+ protos::pbzero::Trace::Decoder trace(buffer);
+
+ Context::FtraceEventMask mask;
+
+ for (auto packet = trace.packet(); packet; ++packet) {
+ protos::pbzero::TracePacket::Decoder decoder(*packet);
+
+ if (decoder.has_ftrace_events()) {
+ mask |= CopyEventFields(decoder.ftrace_events());
+ }
+ }
+
+ return mask;
+ }
+
+ private:
+ Context::FtraceEventMask CopyEventFields(protozero::ConstBytes bytes) {
+ protozero::ProtoDecoder decoder(bytes);
+
+ Context::FtraceEventMask mask;
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ PERFETTO_DCHECK(field.id() < mask.size());
+ mask.set(field.id());
+ }
+
+ return mask;
+ }
+};
+
+// To avoid being fragile, this test checks that some included fields passed
+// through redaction and checks that no excluded fields passed through
+// redaction.
+TEST_F(BroadphasePacketFilterIntegrationTest, OnlyKeepsIncludedPacketFields) {
+ ASSERT_OK(Redact());
+
+ auto trace = LoadRedacted();
+ ASSERT_OK(trace);
+
+ auto include_mask = context()->packet_mask;
+ auto exclude_mask = ~include_mask;
+
+ auto fields = ScanPacketFields(*trace);
+
+ ASSERT_TRUE(fields.any());
+ ASSERT_TRUE(include_mask.any());
+
+ ASSERT_TRUE((fields & include_mask).any());
+ ASSERT_FALSE((fields & exclude_mask).any());
+}
+
+// To avoid being fragile, this test checks that some included fields passed
+// through redaction and checks that no excluded fields passed through
+// redaction.
+TEST_F(BroadphasePacketFilterIntegrationTest,
+ OnlyKeepsIncludedFtraceEventFields) {
+ ASSERT_OK(Redact());
+
+ auto trace = LoadRedacted();
+ ASSERT_OK(trace);
+
+ auto include_mask = context()->ftrace_mask;
+ auto exclude_mask = ~include_mask;
+
+ auto fields = ScanFtraceEventFields(*trace);
+
+ ASSERT_TRUE(fields.any());
+ ASSERT_TRUE(include_mask.any());
+
+ ASSERT_TRUE((fields & include_mask).any());
+ ASSERT_FALSE((fields & exclude_mask).any());
+}
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/broadphase_packet_filter.cc b/src/trace_redaction/broadphase_packet_filter.cc
new file mode 100644
index 0000000..3f009f0
--- /dev/null
+++ b/src/trace_redaction/broadphase_packet_filter.cc
@@ -0,0 +1,99 @@
+/*
+ * 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/broadphase_packet_filter.h"
+
+#include "perfetto/base/status.h"
+#include "perfetto/protozero/scattered_heap_buffer.h"
+#include "src/trace_redaction/proto_util.h"
+#include "src/trace_redaction/trace_redaction_framework.h"
+
+#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
+
+namespace perfetto::trace_redaction {
+
+base::Status BroadphasePacketFilter::Transform(const Context& context,
+ std::string* packet) const {
+ if (context.packet_mask.none()) {
+ return base::ErrStatus("FilterTracePacketFields: empty packet mask.");
+ }
+
+ if (context.ftrace_mask.none()) {
+ return base::ErrStatus("FilterTracePacketFields: empty ftrace mask.");
+ }
+
+ if (!packet || packet->empty()) {
+ return base::ErrStatus("FilterTracePacketFields: missing packet.");
+ }
+
+ protozero::HeapBuffered<protos::pbzero::TracePacket> message;
+
+ protozero::ProtoDecoder decoder(*packet);
+
+ const auto& mask = context.packet_mask;
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ if (mask.test(field.id()) &&
+ field.id() == protos::pbzero::TracePacket::kFtraceEventsFieldNumber) {
+ OnFtraceEvents(context, field.as_bytes(), message->set_ftrace_events());
+ } else if (mask.test(field.id())) {
+ proto_util::AppendField(field, message.get());
+ }
+ }
+
+ packet->assign(message.SerializeAsString());
+ return base::OkStatus();
+}
+
+void BroadphasePacketFilter::OnFtraceEvents(
+ const Context& context,
+ protozero::ConstBytes bytes,
+ protos::pbzero::FtraceEventBundle* message) const {
+ PERFETTO_DCHECK(message);
+
+ protozero::ProtoDecoder decoder(bytes);
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ if (field.id() == protos::pbzero::FtraceEventBundle::kEventFieldNumber) {
+ OnFtraceEvent(context, field.as_bytes(), message->add_event());
+ } else {
+ proto_util::AppendField(field, message);
+ }
+ }
+}
+
+void BroadphasePacketFilter::OnFtraceEvent(
+ const Context& context,
+ protozero::ConstBytes bytes,
+ protos::pbzero::FtraceEvent* message) const {
+ PERFETTO_DCHECK(message);
+
+ protozero::ProtoDecoder decoder(bytes);
+
+ const auto& mask = context.ftrace_mask;
+
+ for (auto field = decoder.ReadField(); field.valid();
+ field = decoder.ReadField()) {
+ if (mask.test(field.id())) {
+ proto_util::AppendField(field, message);
+ }
+ }
+}
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/broadphase_packet_filter.h b/src/trace_redaction/broadphase_packet_filter.h
new file mode 100644
index 0000000..68f18e3
--- /dev/null
+++ b/src/trace_redaction/broadphase_packet_filter.h
@@ -0,0 +1,55 @@
+/*
+ * 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_BROADPHASE_PACKET_FILTER_H_
+#define SRC_TRACE_REDACTION_BROADPHASE_PACKET_FILTER_H_
+
+#include "src/trace_redaction/trace_redaction_framework.h"
+
+#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
+
+namespace perfetto::trace_redaction {
+
+// Execute a broad-phase filter here and defer a narrow-phase filter via other
+// primitives.
+//
+// The concepts of broad-phase and narrow-phase are borrowed from the graphics
+// space where a cheap operations removes large chunks of information
+// (broad-phase) so that less information goes through the more operations
+// (narrow-phase).
+//
+// Here, the broad-phase operation is a filter that removes high-level fields
+// from trace packets so that other primitives (narrow-phase operations) have
+// fewer fields to read and write.
+class BroadphasePacketFilter : public TransformPrimitive {
+ public:
+ base::Status Transform(const Context& context,
+ std::string* packet) const override;
+
+ private:
+ void OnFtraceEvents(const Context& context,
+ protozero::ConstBytes bytes,
+ protos::pbzero::FtraceEventBundle* message) const;
+
+ void OnFtraceEvent(const Context& context,
+ protozero::ConstBytes bytes,
+ protos::pbzero::FtraceEvent* message) const;
+};
+
+} // namespace perfetto::trace_redaction
+
+#endif // SRC_TRACE_REDACTION_BROADPHASE_PACKET_FILTER_H_
diff --git a/src/trace_redaction/broadphase_packet_filter_unittest.cc b/src/trace_redaction/broadphase_packet_filter_unittest.cc
new file mode 100644
index 0000000..7dfb7a7
--- /dev/null
+++ b/src/trace_redaction/broadphase_packet_filter_unittest.cc
@@ -0,0 +1,207 @@
+/*
+ * 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/broadphase_packet_filter.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_bundle.gen.h"
+#include "protos/perfetto/trace/trace_packet.gen.h"
+#include "src/base/test/status_matchers.h"
+#include "src/trace_redaction/trace_redaction_framework.h"
+#include "test/gtest_and_gmock.h"
+
+#include "protos/perfetto/trace/trace_packet.pbzero.h"
+
+namespace perfetto::trace_redaction {
+
+class BroadphasePacketFilterTest : public testing::Test {
+ protected:
+ BroadphasePacketFilter transform_;
+ Context context_;
+ protos::gen::TracePacket builder_;
+};
+
+TEST_F(BroadphasePacketFilterTest, ReturnErrorForEmptyMasks) {
+ auto buffer = builder_.SerializeAsString();
+ ASSERT_FALSE(transform_.Transform(context_, &buffer).ok());
+}
+
+TEST_F(BroadphasePacketFilterTest, ReturnErrorForEmptyPacketMask) {
+ // Set the ftrace mask to ensure the error is from the packet mask.
+ context_.ftrace_mask.set(0);
+
+ auto buffer = builder_.SerializeAsString();
+ ASSERT_FALSE(transform_.Transform(context_, &buffer).ok());
+}
+
+TEST_F(BroadphasePacketFilterTest, ReturnErrorForEmptyFtraceMask) {
+ // Set the ftrace mask to ensure the error is from the ftrace mask.
+ context_.packet_mask.set(0);
+
+ auto buffer = builder_.SerializeAsString();
+ ASSERT_FALSE(transform_.Transform(context_, &buffer).ok());
+}
+
+TEST_F(BroadphasePacketFilterTest, ReturnErrorForNullPacket) {
+ // Set the masks to ensure the error is from the ftrace mask.
+ context_.ftrace_mask.set(0);
+ context_.packet_mask.set(0);
+
+ ASSERT_FALSE(transform_.Transform(context_, nullptr).ok());
+}
+
+TEST_F(BroadphasePacketFilterTest, ReturnErrorForEmptyPacket) {
+ // Set the masks to ensure the error is from the ftrace mask.
+ context_.ftrace_mask.set(0);
+ context_.packet_mask.set(0);
+
+ std::string buffer;
+ ASSERT_FALSE(transform_.Transform(context_, &buffer).ok());
+}
+
+TEST_F(BroadphasePacketFilterTest, DropsPacketField) {
+ constexpr uint64_t kTime = 1000;
+
+ builder_.set_timestamp(kTime);
+ auto buffer = builder_.SerializeAsString();
+
+ // Both masks need some bit set.
+ context_.ftrace_mask.set(0);
+ context_.packet_mask.set(0);
+
+ ASSERT_OK(transform_.Transform(context_, &buffer));
+
+ protos::gen::TracePacket packet;
+ ASSERT_TRUE(packet.ParseFromString(buffer));
+
+ // The timestamp field should have been dropped.
+ ASSERT_FALSE(packet.has_timestamp());
+}
+
+TEST_F(BroadphasePacketFilterTest, KeepsPacketField) {
+ constexpr uint64_t kTime = 1000;
+
+ builder_.set_timestamp(kTime);
+ auto buffer = builder_.SerializeAsString();
+
+ // Both masks need some bit set.
+ context_.ftrace_mask.set(0);
+ context_.packet_mask.set(protos::pbzero::TracePacket::kTimestampFieldNumber);
+
+ ASSERT_OK(transform_.Transform(context_, &buffer));
+
+ protos::gen::TracePacket packet;
+ ASSERT_TRUE(packet.ParseFromString(buffer));
+
+ ASSERT_TRUE(packet.has_timestamp());
+ ASSERT_EQ(packet.timestamp(), kTime);
+}
+
+TEST_F(BroadphasePacketFilterTest, DropsAllFtraceEvents) {
+ constexpr uint64_t kTime = 1000;
+
+ builder_.mutable_ftrace_events()->add_event()->set_timestamp(kTime);
+ auto buffer = builder_.SerializeAsString();
+
+ // Both masks need some bit set.
+ context_.ftrace_mask.set(0);
+ context_.packet_mask.set(0);
+
+ ASSERT_OK(transform_.Transform(context_, &buffer));
+
+ protos::gen::TracePacket packet;
+ ASSERT_TRUE(packet.ParseFromString(buffer));
+
+ // Because kEvents was not set, all ftrace events will be dropped.
+ ASSERT_FALSE(packet.has_ftrace_events());
+}
+
+TEST_F(BroadphasePacketFilterTest, KeepFtraceEvents) {
+ constexpr uint64_t kTime = 1000;
+ constexpr int32_t kCpu = 3;
+
+ builder_.mutable_ftrace_events()->add_event()->set_timestamp(kTime);
+ builder_.mutable_ftrace_events()->set_cpu(kCpu);
+
+ auto buffer = builder_.SerializeAsString();
+
+ // Both masks need some bit set.
+ context_.ftrace_mask.set(0);
+ context_.packet_mask.set(
+ protos::pbzero::TracePacket::kFtraceEventsFieldNumber);
+
+ ASSERT_OK(transform_.Transform(context_, &buffer));
+
+ protos::gen::TracePacket packet;
+ ASSERT_TRUE(packet.ParseFromString(buffer));
+
+ // The bundle will be kept. Ignoring the events, the other fields should be
+ // copied over. To be simple, we're only checking one field (CPU).
+ ASSERT_TRUE(packet.has_ftrace_events());
+ ASSERT_TRUE(packet.ftrace_events().has_cpu());
+}
+
+TEST_F(BroadphasePacketFilterTest, KeepsFtraceEvent) {
+ constexpr uint64_t kTime = 1000;
+
+ auto* event = builder_.mutable_ftrace_events()->add_event();
+ event->set_timestamp(kTime);
+ event->mutable_print()->set_buf("hello world");
+ auto buffer = builder_.SerializeAsString();
+
+ // Both masks need some bit set.
+ context_.ftrace_mask.set(protos::pbzero::FtraceEvent::kPrintFieldNumber);
+ context_.packet_mask.set(
+ protos::pbzero::TracePacket::kFtraceEventsFieldNumber);
+
+ ASSERT_OK(transform_.Transform(context_, &buffer));
+
+ protos::gen::TracePacket packet;
+ ASSERT_TRUE(packet.ParseFromString(buffer));
+
+ // kFtraceEvents must be in the packet mask in order for the ftrace events to
+ // be searched.
+ ASSERT_TRUE(packet.has_ftrace_events());
+ ASSERT_EQ(packet.ftrace_events().event().size(), 1u);
+ ASSERT_TRUE(packet.ftrace_events().event().at(0).has_print());
+}
+
+TEST_F(BroadphasePacketFilterTest, DropsFtraceEvent) {
+ constexpr uint64_t kTime = 1000;
+
+ auto* event = builder_.mutable_ftrace_events()->add_event();
+ event->set_timestamp(kTime);
+ event->mutable_print()->set_buf("hello world");
+ auto buffer = builder_.SerializeAsString();
+
+ // Both masks need some bit set.
+ context_.ftrace_mask.set(0);
+ context_.packet_mask.set(
+ protos::pbzero::TracePacket::kFtraceEventsFieldNumber);
+
+ ASSERT_OK(transform_.Transform(context_, &buffer));
+
+ protos::gen::TracePacket packet;
+ ASSERT_TRUE(packet.ParseFromString(buffer));
+
+ // The ftrace events bundle will be copied. All the ftrace events will be
+ // copied, but the tasks in the events (e.g. print) will be removed.
+ ASSERT_TRUE(packet.has_ftrace_events());
+ ASSERT_EQ(packet.ftrace_events().event().size(), 1u);
+ ASSERT_FALSE(packet.ftrace_events().event().at(0).has_print());
+}
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/filter_ftrace_using_allowlist_integrationtest.cc b/src/trace_redaction/filter_ftrace_using_allowlist_integrationtest.cc
deleted file mode 100644
index 0fcc544..0000000
--- a/src/trace_redaction/filter_ftrace_using_allowlist_integrationtest.cc
+++ /dev/null
@@ -1,161 +0,0 @@
-/*
- * 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 <cstdint>
-#include <string>
-
-#include "src/base/test/status_matchers.h"
-#include "src/trace_redaction/populate_allow_lists.h"
-#include "src/trace_redaction/redact_ftrace_events.h"
-#include "src/trace_redaction/trace_redaction_integration_fixture.h"
-#include "test/gtest_and_gmock.h"
-
-#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
-#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
-#include "protos/perfetto/trace/trace.pbzero.h"
-#include "protos/perfetto/trace/trace_packet.pbzero.h"
-
-namespace perfetto::trace_redaction {
-
-class FilterFtraceUsingAllowlistTest
- : public testing::Test,
- protected TraceRedactionIntegrationFixure {
- protected:
- void SetUp() override {
- trace_redactor()->emplace_build<PopulateAllowlists>();
-
- auto redact = trace_redactor()->emplace_transform<RedactFtraceEvents>();
- redact->emplace_ftrace_filter<FilterFtracesUsingAllowlist>();
- redact->emplace_post_filter_modifier<DoNothing>();
- }
-
- // Parse the given buffer and gather field ids from across all events. This
- // will also include fields like timestamp.
- base::FlatSet<uint32_t> ParseEvents(std::string trace_buffer) {
- base::FlatSet<uint32_t> event_ids;
-
- protos::pbzero::Trace::Decoder trace_decoder(trace_buffer);
-
- for (auto packet = trace_decoder.packet(); packet; ++packet) {
- protos::pbzero::TracePacket::Decoder packet_decoder(*packet);
-
- if (!packet_decoder.has_ftrace_events()) {
- continue;
- }
-
- protos::pbzero::FtraceEventBundle::Decoder bundle_decoder(
- packet_decoder.ftrace_events());
-
- for (auto event = bundle_decoder.event(); event; ++event) {
- protozero::ProtoDecoder event_decoder(*event);
-
- for (auto field = event_decoder.ReadField(); field.valid();
- field = event_decoder.ReadField()) {
- event_ids.insert(field.id());
- }
- }
- }
-
- return event_ids;
- }
-};
-
-// This is not a test of FilterFtraceUsingAllowlist, but instead verifies of the
-// sample trace used in the test.
-TEST_F(FilterFtraceUsingAllowlistTest, TraceHasAllEvents) {
- auto trace = LoadOriginal();
- ASSERT_OK(trace) << trace->c_str();
-
- auto events = ParseEvents(std::move(trace.value()));
- ASSERT_EQ(events.size(), 14u);
-
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kCpuFrequencyFieldNumber));
- ASSERT_TRUE(events.count(protos::pbzero::FtraceEvent::kCpuIdleFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kOomScoreAdjUpdateFieldNumber));
- ASSERT_TRUE(events.count(protos::pbzero::FtraceEvent::kPidFieldNumber));
- ASSERT_TRUE(events.count(protos::pbzero::FtraceEvent::kPrintFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kSchedProcessExitFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kSchedProcessFreeFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kSchedWakeupFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kSchedWakeupNewFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kSchedWakingFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kTaskNewtaskFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kTaskRenameFieldNumber));
- ASSERT_TRUE(events.count(protos::pbzero::FtraceEvent::kTimestampFieldNumber));
-}
-
-TEST_F(FilterFtraceUsingAllowlistTest, RetainsAllowedEvents) {
- auto redacted = Redact();
- ASSERT_OK(redacted) << redacted.c_message();
-
- auto trace = LoadRedacted();
- ASSERT_OK(trace) << trace.status().c_message();
-
- auto events = ParseEvents(std::move(trace.value()));
-
- // These are not events, they are fields that exist alongside the event.
- ASSERT_TRUE(events.count(protos::pbzero::FtraceEvent::kPidFieldNumber));
- ASSERT_TRUE(events.count(protos::pbzero::FtraceEvent::kTimestampFieldNumber));
-
- // These are events.
- ASSERT_TRUE(events.count(protos::pbzero::FtraceEvent::kPrintFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kCpuFrequencyFieldNumber));
- ASSERT_TRUE(events.count(protos::pbzero::FtraceEvent::kCpuIdleFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kSchedProcessFreeFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kSchedWakingFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kTaskNewtaskFieldNumber));
- ASSERT_TRUE(
- events.count(protos::pbzero::FtraceEvent::kTaskRenameFieldNumber));
-}
-
-TEST_F(FilterFtraceUsingAllowlistTest, RemovesNotAllowedEvents) {
- auto redacted = Redact();
- ASSERT_OK(redacted) << redacted.c_message();
-
- auto trace = LoadRedacted();
- ASSERT_OK(trace) << trace.status().c_message();
-
- auto events = ParseEvents(std::move(trace.value()));
-
- // These are events.
- ASSERT_FALSE(
- events.count(protos::pbzero::FtraceEvent::kOomScoreAdjUpdateFieldNumber));
- ASSERT_FALSE(
- events.count(protos::pbzero::FtraceEvent::kSchedProcessExitFieldNumber));
- ASSERT_FALSE(
- events.count(protos::pbzero::FtraceEvent::kSchedWakeupFieldNumber));
- ASSERT_FALSE(
- events.count(protos::pbzero::FtraceEvent::kSchedWakeupNewFieldNumber));
-}
-
-} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/filter_ftrace_using_allowlist_unittest.cc b/src/trace_redaction/filter_ftrace_using_allowlist_unittest.cc
deleted file mode 100644
index e2958a3..0000000
--- a/src/trace_redaction/filter_ftrace_using_allowlist_unittest.cc
+++ /dev/null
@@ -1,246 +0,0 @@
-/*
- * 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/base/test/status_matchers.h"
-#include "src/trace_redaction/redact_ftrace_events.h"
-#include "test/gtest_and_gmock.h"
-
-#include "protos/perfetto/trace/ftrace/ftrace_event.gen.h"
-#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
-#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.gen.h"
-#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
-#include "protos/perfetto/trace/ftrace/power.gen.h"
-#include "protos/perfetto/trace/ftrace/task.gen.h"
-#include "protos/perfetto/trace/ps/process_tree.gen.h"
-#include "protos/perfetto/trace/trace_packet.gen.h"
-
-namespace perfetto::trace_redaction {
-namespace {
-constexpr uint64_t kTime = 10;
-
-constexpr int32_t kPidA = 7;
-constexpr int32_t kPidB = 8;
-constexpr int32_t kPidC = 9;
-
-constexpr int32_t kCpu = 3;
-} // namespace
-
-// Tests which nested messages and fields are removed.
-class FilterFtraceUsingAllowlistTest : public testing::Test {
- protected:
- void SetUp() override {
- transform_.emplace_ftrace_filter<FilterFtracesUsingAllowlist>();
- transform_.emplace_post_filter_modifier<DoNothing>();
-
- // RedactFtraceEvents expects ftrace events bundles to have a cpu value.
- // These tests don't rely on one, so set it here and forget about it.
- packet_.mutable_ftrace_events()->set_cpu(kCpu);
- }
-
- // task_rename should be in the allow-list.
- static void AddTaskRename(protos::gen::FtraceEventBundle* bundle,
- int32_t pid,
- const std::string& old_comm,
- const std::string& new_comm) {
- auto* e = bundle->add_event();
- e->set_pid(static_cast<uint32_t>(pid));
- e->set_timestamp(kTime);
- e->mutable_task_rename()->set_pid(pid);
- e->mutable_task_rename()->set_oldcomm(old_comm);
- e->mutable_task_rename()->set_newcomm(new_comm);
- }
-
- static void AddClockSetRate(protos::gen::FtraceEventBundle* bundle,
- int32_t pid,
- uint64_t cpu,
- const std::string& name,
- uint64_t state) {
- auto* e = bundle->add_event();
- e->set_pid(static_cast<uint32_t>(pid));
- e->set_timestamp(kTime);
- e->mutable_clock_set_rate()->set_cpu_id(cpu);
- e->mutable_clock_set_rate()->set_name(name);
- e->mutable_clock_set_rate()->set_state(state);
- }
-
- RedactFtraceEvents transform_;
-
- protos::gen::TracePacket packet_;
-};
-
-TEST_F(FilterFtraceUsingAllowlistTest, ReturnErrorForNullPacket) {
- Context context;
- ASSERT_FALSE(transform_.Transform(context, nullptr).ok());
-}
-
-TEST_F(FilterFtraceUsingAllowlistTest, ReturnErrorForEmptyPacket) {
- std::string packet_str = "";
-
- Context context;
- ASSERT_FALSE(transform_.Transform(context, &packet_str).ok());
-}
-
-TEST_F(FilterFtraceUsingAllowlistTest, IgnorePacketWithNoFtraceEvents) {
- auto* tree = packet_.mutable_process_tree();
-
- auto& process = tree->mutable_processes()->emplace_back();
- process.set_pid(1);
- process.set_ppid(2);
- process.set_uid(3);
-
- auto& thread = tree->mutable_threads()->emplace_back();
- thread.set_name("hello world");
- thread.set_tgid(1);
- thread.set_tid(135);
-
- auto original_packet = packet_.SerializeAsString();
- auto packet = original_packet;
-
- Context context;
- context.ftrace_packet_allow_list = {
- protos::pbzero::FtraceEvent::kTaskRenameFieldNumber};
-
- auto transform_status = transform_.Transform(context, &packet);
- ASSERT_OK(transform_status) << transform_status.c_message();
-
- // The packet doesn't have any ftrace events. It should not be affected by
- // this transform.
- ASSERT_EQ(original_packet, packet);
-}
-
-// There are some values in a ftrace event that sits behind the ftrace bundle.
-// These values should be retained.
-TEST_F(FilterFtraceUsingAllowlistTest, KeepsFtraceBundleSiblingValues) {
- auto* ftrace_events = packet_.mutable_ftrace_events();
-
- ftrace_events->set_cpu(kCpu);
- AddTaskRename(ftrace_events, kPidA, "old_comm", "new_comm_7");
- AddClockSetRate(ftrace_events, kPidA, kCpu, "cool cpu name", 1);
-
- auto original_packet = packet_.SerializeAsString();
- auto packet = original_packet;
-
- Context context;
- context.ftrace_packet_allow_list = {
- protos::pbzero::FtraceEvent::kTaskRenameFieldNumber};
-
- ASSERT_OK(transform_.Transform(context, &packet));
-
- protos::gen::TracePacket gen_packet;
- gen_packet.ParseFromString(packet);
-
- ASSERT_TRUE(gen_packet.has_ftrace_events());
- const auto& gen_events = gen_packet.ftrace_events();
-
- // Because the CPU sits beside the event list, and not inside the event list,
- // the CPU value should be retained.
- ASSERT_TRUE(gen_events.has_cpu());
- ASSERT_EQ(gen_events.cpu(), static_cast<uint32_t>(kCpu));
-
- // ClockSetRate should be dropped. Only TaskRename should remain.
- ASSERT_EQ(gen_events.event_size(), 1);
- ASSERT_FALSE(gen_events.event().front().has_clock_set_rate());
- ASSERT_TRUE(gen_events.event().front().has_task_rename());
-}
-
-TEST_F(FilterFtraceUsingAllowlistTest, KeepsAllowedEvents) {
- Context context;
- context.ftrace_packet_allow_list = {
- protos::pbzero::FtraceEvent::kTaskRenameFieldNumber,
- };
-
- AddTaskRename(packet_.mutable_ftrace_events(), kPidA, "old_comm",
- "new_comm_7");
- AddTaskRename(packet_.mutable_ftrace_events(), kPidB, "old_comm",
- "new_comm_8");
- AddTaskRename(packet_.mutable_ftrace_events(), kPidC, "old_comm",
- "new_comm_9");
-
- auto before_str = packet_.SerializeAsString();
- auto after_str = before_str;
-
- ASSERT_OK(transform_.Transform(context, &after_str));
-
- protos::gen::TracePacket after;
- after.ParseFromString(after_str);
-
- // Implementation detail: ScrubFtraceEvents may change entry order. The diff
- // must be order independent. Sort the events by pid, this will make it easier
- // to assert values.
- auto events = after.ftrace_events().event();
- std::sort(events.begin(), events.end(),
- [](const auto& l, const auto& r) { return l.pid() < r.pid(); });
-
- ASSERT_EQ(events.size(), 3u);
-
- ASSERT_TRUE(events[0].has_task_rename());
- ASSERT_EQ(events[0].task_rename().pid(), kPidA);
- ASSERT_EQ(events[0].task_rename().oldcomm(), "old_comm");
- ASSERT_EQ(events[0].task_rename().newcomm(), "new_comm_7");
-
- ASSERT_TRUE(events[1].has_task_rename());
- ASSERT_EQ(events[1].task_rename().pid(), kPidB);
- ASSERT_EQ(events[1].task_rename().oldcomm(), "old_comm");
- ASSERT_EQ(events[1].task_rename().newcomm(), "new_comm_8");
-
- ASSERT_TRUE(events[2].has_task_rename());
- ASSERT_EQ(events[2].task_rename().pid(), kPidC);
- ASSERT_EQ(events[2].task_rename().oldcomm(), "old_comm");
- ASSERT_EQ(events[2].task_rename().newcomm(), "new_comm_9");
-}
-
-// Only the specific non-allowed events should be removed from the event list.
-TEST_F(FilterFtraceUsingAllowlistTest, OnlyDropsNotAllowedEvents) {
- // AddTaskRename >> Keep
- // AddClockSetRate >> Drop
- AddTaskRename(packet_.mutable_ftrace_events(), kPidA, "old_comm",
- "new_comm_7");
- AddClockSetRate(packet_.mutable_ftrace_events(), kPidA, kCpu, "cool cpu name",
- 1);
- AddTaskRename(packet_.mutable_ftrace_events(), kPidB, "old_comm",
- "new_comm_8");
- AddTaskRename(packet_.mutable_ftrace_events(), kPidC, "old_comm",
- "new_comm_9");
- auto packet = packet_.SerializeAsString();
-
- Context context;
- context.ftrace_packet_allow_list = {
- protos::pbzero::FtraceEvent::kTaskRenameFieldNumber};
-
- ASSERT_OK(transform_.Transform(context, &packet));
-
- protos::gen::TracePacket modified_packet;
- ASSERT_TRUE(modified_packet.ParseFromString(packet));
-
- // Only the clock set rate event should have been removed (drop 1 of the 4
- // events).
- ASSERT_TRUE(modified_packet.has_ftrace_events());
- ASSERT_EQ(modified_packet.ftrace_events().event_size(), 3);
-
- // All ftrace events should be rename events.
- const auto& events = modified_packet.ftrace_events().event();
-
- ASSERT_TRUE(events.at(0).has_task_rename());
- ASSERT_EQ(events.at(0).task_rename().pid(), kPidA);
-
- ASSERT_TRUE(events.at(1).has_task_rename());
- ASSERT_EQ(events.at(1).task_rename().pid(), kPidB);
-
- ASSERT_TRUE(events.at(2).has_task_rename());
- ASSERT_EQ(events.at(2).task_rename().pid(), kPidC);
-}
-
-} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/filter_packet_using_allowlist.cc b/src/trace_redaction/filter_packet_using_allowlist.cc
deleted file mode 100644
index 3b714a4..0000000
--- a/src/trace_redaction/filter_packet_using_allowlist.cc
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * 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/filter_packet_using_allowlist.h"
-#include "perfetto/base/status.h"
-#include "perfetto/protozero/field.h"
-#include "src/trace_redaction/trace_redaction_framework.h"
-
-namespace perfetto::trace_redaction {
-
-base::Status FilterPacketUsingAllowlist::VerifyContext(
- const Context& context) const {
- if (context.trace_packet_allow_list.empty()) {
- return base::ErrStatus("FilterPacketUsingAllowlist: missing allow-list.");
- }
-
- return base::OkStatus();
-}
-
-bool FilterPacketUsingAllowlist::KeepField(
- const Context& context,
- const protozero::Field& field) const {
- PERFETTO_DCHECK(!context.trace_packet_allow_list.empty());
- return field.valid() && context.trace_packet_allow_list.count(field.id());
-}
-
-} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/filter_packet_using_allowlist.h b/src/trace_redaction/filter_packet_using_allowlist.h
deleted file mode 100644
index 9205e94..0000000
--- a/src/trace_redaction/filter_packet_using_allowlist.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * 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_FILTER_PACKET_USING_ALLOWLIST_H_
-#define SRC_TRACE_REDACTION_FILTER_PACKET_USING_ALLOWLIST_H_
-
-#include "src/trace_redaction/scrub_trace_packet.h"
-#include "src/trace_redaction/trace_redaction_framework.h"
-
-namespace perfetto::trace_redaction {
-
-// Since the number of allow-listed message types, and the allow-list is
-// small, the look-up can be considered constant time.
-//
-// There is a constant max number of fields in a packet. Given this limit and
-// the constant allow-list look-up, this primitive can be considered linear.
-class FilterPacketUsingAllowlist : public TracePacketFilter {
- public:
- base::Status VerifyContext(const Context& context) const override;
-
- bool KeepField(const Context& context,
- const protozero::Field& field) const override;
-};
-
-} // namespace perfetto::trace_redaction
-
-#endif // SRC_TRACE_REDACTION_FILTER_PACKET_USING_ALLOWLIST_H_
diff --git a/src/trace_redaction/filter_packet_using_allowlist_unittest.cc b/src/trace_redaction/filter_packet_using_allowlist_unittest.cc
deleted file mode 100644
index fad6879..0000000
--- a/src/trace_redaction/filter_packet_using_allowlist_unittest.cc
+++ /dev/null
@@ -1,72 +0,0 @@
-/*
- * 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/filter_packet_using_allowlist.h"
-#include "perfetto/protozero/scattered_heap_buffer.h"
-#include "test/gtest_and_gmock.h"
-
-#include "protos/perfetto/trace/trace_packet.pbzero.h"
-
-// TODO(vaage): These tests were used to test the filter-driver, but these tests
-// no longer do that. A new test suite should be created to test the driver code
-// with the different filters.
-namespace perfetto::trace_redaction {
-
-namespace {
-
-constexpr auto kJustSomeFieldId =
- protos::pbzero::TracePacket::kProcessTreeFieldNumber;
-
-} // namespace
-
-TEST(FilterPacketUsingAllowlistParamErrorTest, ReturnsErrorForEmptyAllowlist) {
- Context context;
-
- FilterPacketUsingAllowlist filter;
- auto status = filter.VerifyContext(context);
-
- ASSERT_FALSE(status.ok()) << status.message();
-}
-
-TEST(FilterPacketUsingAllowlistParamErrorTest, ReturnsFalseForInvalidField) {
- // Have something in the allow-list to avoid an error.
- Context context;
- context.trace_packet_allow_list.insert(kJustSomeFieldId);
-
- protozero::Field invalid = {};
- ASSERT_FALSE(invalid.valid());
-
- FilterPacketUsingAllowlist filter;
- ASSERT_FALSE(filter.KeepField(context, invalid));
-}
-
-TEST(FilterPacketUsingAllowlistParamErrorTest, ReturnsFalseForExcludedField) {
- Context context;
- context.trace_packet_allow_list.insert(kJustSomeFieldId);
-
- protozero::HeapBuffered<protos::pbzero::TracePacket> packet;
- packet->set_timestamp(123456789);
-
- auto buffer = packet.SerializeAsString();
-
- protozero::ProtoDecoder decoder(buffer);
- protozero::Field field = decoder.FindField(kJustSomeFieldId);
-
- FilterPacketUsingAllowlist filter;
- ASSERT_FALSE(filter.KeepField(context, field));
-}
-
-} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/main.cc b/src/trace_redaction/main.cc
index 51d5ead..9d2e5d5 100644
--- a/src/trace_redaction/main.cc
+++ b/src/trace_redaction/main.cc
@@ -16,10 +16,10 @@
#include "perfetto/base/logging.h"
#include "perfetto/base/status.h"
+#include "src/trace_redaction/broadphase_packet_filter.h"
#include "src/trace_redaction/collect_frame_cookies.h"
#include "src/trace_redaction/collect_system_info.h"
#include "src/trace_redaction/collect_timeline_events.h"
-#include "src/trace_redaction/filter_packet_using_allowlist.h"
#include "src/trace_redaction/find_package_uid.h"
#include "src/trace_redaction/merge_threads.h"
#include "src/trace_redaction/populate_allow_lists.h"
@@ -29,7 +29,6 @@
#include "src/trace_redaction/redact_process_trees.h"
#include "src/trace_redaction/redact_sched_events.h"
#include "src/trace_redaction/scrub_process_stats.h"
-#include "src/trace_redaction/scrub_trace_packet.h"
#include "src/trace_redaction/trace_redaction_framework.h"
#include "src/trace_redaction/trace_redactor.h"
#include "src/trace_redaction/verify_integrity.h"
@@ -55,20 +54,14 @@
redactor.emplace_collect<CollectSystemInfo>();
// Add all builders.
- redactor.emplace_build<PopulateAllowlists>();
redactor.emplace_build<ReduceFrameCookies>();
redactor.emplace_build<BuildSyntheticThreads>();
{
- auto* primitive = redactor.emplace_transform<ScrubTracePacket>();
- primitive->emplace_back<FilterPacketUsingAllowlist>();
- primitive->emplace_back<FilterFrameEvents>();
- }
-
- {
- auto* primitive = redactor.emplace_transform<RedactFtraceEvents>();
- primitive->emplace_ftrace_filter<FilterFtracesUsingAllowlist>();
- primitive->emplace_post_filter_modifier<DoNothing>();
+ // In order for BroadphasePacketFilter to work, something needs to populate
+ // the masks (i.e. PopulateAllowlists).
+ redactor.emplace_build<PopulateAllowlists>();
+ redactor.emplace_transform<BroadphasePacketFilter>();
}
{
diff --git a/src/trace_redaction/populate_allow_lists.cc b/src/trace_redaction/populate_allow_lists.cc
index 3af842a..2b8207d 100644
--- a/src/trace_redaction/populate_allow_lists.cc
+++ b/src/trace_redaction/populate_allow_lists.cc
@@ -25,64 +25,44 @@
namespace perfetto::trace_redaction {
base::Status PopulateAllowlists::Build(Context* context) const {
- // These fields are top-level fields that outside the "oneof data" field.
- std::initializer_list<uint32_t> required_trace_fields = {
+ auto& packet_mask = context->packet_mask;
- protos::pbzero::TracePacket::kTimestampFieldNumber,
- protos::pbzero::TracePacket::kTimestampClockIdFieldNumber,
- protos::pbzero::TracePacket::kTrustedUidFieldNumber,
- protos::pbzero::TracePacket::kTrustedPacketSequenceIdFieldNumber,
- protos::pbzero::TracePacket::kTrustedPidFieldNumber,
- protos::pbzero::TracePacket::kInternedDataFieldNumber,
- protos::pbzero::TracePacket::kSequenceFlagsFieldNumber,
+ // Top-level fields - fields outside of the "oneof data" field.
+ packet_mask.set(
+ protos::pbzero::TracePacket::kFirstPacketOnSequenceFieldNumber);
+ packet_mask.set(
+ protos::pbzero::TracePacket::kIncrementalStateClearedFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kInternedDataFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kMachineIdFieldNumber);
+ packet_mask.set(
+ protos::pbzero::TracePacket::kPreviousPacketDroppedFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kSequenceFlagsFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kTimestampClockIdFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kTimestampFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kTracePacketDefaultsFieldNumber);
+ packet_mask.set(
+ protos::pbzero::TracePacket::kTrustedPacketSequenceIdFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kTrustedPidFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kTrustedUidFieldNumber);
- // DEPRECATED. Moved to SequenceFlags::SEQ_INCREMENTAL_STATE_CLEARED. So
- // there is no reason to include it.
- //
- // protos::pbzero::TracePacket::incremental_state_cleared
-
- protos::pbzero::TracePacket::kTracePacketDefaultsFieldNumber,
- protos::pbzero::TracePacket::kPreviousPacketDroppedFieldNumber,
- protos::pbzero::TracePacket::kFirstPacketOnSequenceFieldNumber,
- protos::pbzero::TracePacket::kMachineIdFieldNumber,
- };
-
- for (auto item : required_trace_fields) {
- context->trace_packet_allow_list.insert(item);
- }
-
- // TRACE PACKET NOTES
- //
- // protos::pbzero::TracePacket::kAndroidSystemPropertyFieldNumber
- //
- // AndroidSystemProperty exposes a key-value pair structure with no
- // constraints around keys or values, making fine-grain redaction
- // difficult. Because this packet's value has no measurable, the safest
- // option to drop the whole packet.
- std::initializer_list<uint32_t> trace_packets = {
- protos::pbzero::TracePacket::kProcessTreeFieldNumber,
- protos::pbzero::TracePacket::kProcessStatsFieldNumber,
- protos::pbzero::TracePacket::kClockSnapshotFieldNumber,
- protos::pbzero::TracePacket::kSysStatsFieldNumber,
- protos::pbzero::TracePacket::kTraceConfigFieldNumber,
- protos::pbzero::TracePacket::kTraceStatsFieldNumber,
- protos::pbzero::TracePacket::kSystemInfoFieldNumber,
- protos::pbzero::TracePacket::kTriggerFieldNumber,
- protos::pbzero::TracePacket::kCpuInfoFieldNumber,
- protos::pbzero::TracePacket::kServiceEventFieldNumber,
- protos::pbzero::TracePacket::kInitialDisplayStateFieldNumber,
- protos::pbzero::TracePacket::kFrameTimelineEventFieldNumber,
- protos::pbzero::TracePacket::kSynchronizationMarkerFieldNumber,
- protos::pbzero::TracePacket::kFtraceEventsFieldNumber,
-
- // Keep the package list. There are some metrics and stdlib queries that
- // depend on the package list.
- protos::pbzero::TracePacket::kPackagesListFieldNumber,
- };
-
- for (auto item : trace_packets) {
- context->trace_packet_allow_list.insert(item);
- }
+ // Trace packet data (one-of field) - Every field here should also be modified
+ // by message-focused transform.
+ packet_mask.set(protos::pbzero::TracePacket::kClockSnapshotFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kCpuInfoFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kFrameTimelineEventFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kFtraceEventsFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kInitialDisplayStateFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kPackagesListFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kProcessStatsFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kProcessTreeFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kServiceEventFieldNumber);
+ packet_mask.set(
+ protos::pbzero::TracePacket::kSynchronizationMarkerFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kSysStatsFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kSystemInfoFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kTraceConfigFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kTraceStatsFieldNumber);
+ packet_mask.set(protos::pbzero::TracePacket::kTriggerFieldNumber);
// FTRACE EVENT NOTES
//
@@ -99,23 +79,22 @@
// TODO(vaage): kSchedBlockedReasonFieldNumber contains two pids, an outer
// and inner pid. A primitive is needed to further redact these events.
- std::initializer_list<uint32_t> ftrace_events = {
- protos::pbzero::FtraceEvent::kCpuFrequencyFieldNumber,
- protos::pbzero::FtraceEvent::kCpuIdleFieldNumber,
- protos::pbzero::FtraceEvent::kPrintFieldNumber,
- protos::pbzero::FtraceEvent::kRssStatFieldNumber,
- protos::pbzero::FtraceEvent::kRssStatThrottledFieldNumber,
- protos::pbzero::FtraceEvent::kSchedBlockedReasonFieldNumber,
- protos::pbzero::FtraceEvent::kSchedProcessFreeFieldNumber,
- protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber,
- protos::pbzero::FtraceEvent::kSchedWakingFieldNumber,
- protos::pbzero::FtraceEvent::kTaskNewtaskFieldNumber,
- protos::pbzero::FtraceEvent::kTaskRenameFieldNumber,
- };
+ auto& ftrace_masks = context->ftrace_mask;
- for (auto item : ftrace_events) {
- context->ftrace_packet_allow_list.insert(item);
- }
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kCommonFlagsFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kCpuFrequencyFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kCpuIdleFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kPidFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kPrintFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kRssStatFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kRssStatThrottledFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kSchedBlockedReasonFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kSchedProcessFreeFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kSchedWakingFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kTaskNewtaskFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kTaskRenameFieldNumber);
+ ftrace_masks.set(protos::pbzero::FtraceEvent::kTimestampFieldNumber);
return base::OkStatus();
}
diff --git a/src/trace_redaction/redact_ftrace_events.cc b/src/trace_redaction/redact_ftrace_events.cc
index 71af76b..123e122 100644
--- a/src/trace_redaction/redact_ftrace_events.cc
+++ b/src/trace_redaction/redact_ftrace_events.cc
@@ -31,21 +31,6 @@
FtraceEventFilter::~FtraceEventFilter() = default;
-bool FilterFtracesUsingAllowlist::Includes(const Context& context,
- protozero::Field event) const {
- PERFETTO_DCHECK(!context.ftrace_packet_allow_list.empty());
-
- protozero::ProtoDecoder decoder(event.as_bytes());
-
- for (auto it = decoder.ReadField(); it.valid(); it = decoder.ReadField()) {
- if (context.ftrace_packet_allow_list.count(it.id())) {
- return true;
- }
- }
-
- return false;
-}
-
bool FilterFtraceUsingSuspendResume::Includes(const Context&,
protozero::Field event) const {
// Values are taken from "suspend_period.textproto". These values would
diff --git a/src/trace_redaction/redact_ftrace_events.h b/src/trace_redaction/redact_ftrace_events.h
index 9ae65ff..f4006e9 100644
--- a/src/trace_redaction/redact_ftrace_events.h
+++ b/src/trace_redaction/redact_ftrace_events.h
@@ -26,11 +26,6 @@
namespace perfetto::trace_redaction {
-class FilterFtracesUsingAllowlist : public FtraceEventFilter {
- public:
- bool Includes(const Context& context, protozero::Field event) const override;
-};
-
class FilterFtraceUsingSuspendResume : public FtraceEventFilter {
public:
bool Includes(const Context& context, protozero::Field event) const override;
diff --git a/src/trace_redaction/scrub_ftrace_events_integrationtest.cc b/src/trace_redaction/scrub_ftrace_events_integrationtest.cc
deleted file mode 100644
index 7c5114a..0000000
--- a/src/trace_redaction/scrub_ftrace_events_integrationtest.cc
+++ /dev/null
@@ -1,112 +0,0 @@
-/*
- * 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 <vector>
-
-#include "src/base/test/status_matchers.h"
-#include "src/trace_redaction/redact_ftrace_events.h"
-#include "src/trace_redaction/trace_redaction_integration_fixture.h"
-#include "test/gtest_and_gmock.h"
-
-#include "protos/perfetto/trace//ftrace/ftrace_event.pbzero.h"
-#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
-#include "protos/perfetto/trace/trace.pbzero.h"
-#include "protos/perfetto/trace/trace_packet.pbzero.h"
-
-namespace perfetto::trace_redaction {
-
-// Runs ScrubFtraceEvents over an actual trace, verifying packet integrity when
-// fields are removed.
-class ScrubFtraceEventsIntegrationTest
- : public testing::Test,
- protected TraceRedactionIntegrationFixure {
- public:
- ScrubFtraceEventsIntegrationTest() = default;
- ~ScrubFtraceEventsIntegrationTest() override = default;
-
- protected:
- void SetUp() override {
- context()->ftrace_packet_allow_list.insert(
- protos::pbzero::FtraceEvent::kSchedSwitchFieldNumber);
-
- auto* redact = trace_redactor()->emplace_transform<RedactFtraceEvents>();
- redact->emplace_ftrace_filter<FilterFtracesUsingAllowlist>();
- redact->emplace_post_filter_modifier<DoNothing>();
- }
-
- // Gets spans for `event` messages that contain `sched_switch` messages.
- void FindAllEvents(const protos::pbzero::TracePacket::Decoder& packet,
- std::vector<protozero::ConstBytes>* events) const {
- if (!packet.has_ftrace_events()) {
- return;
- }
-
- protos::pbzero::FtraceEventBundle::Decoder ftrace_events(
- packet.ftrace_events());
-
- for (auto it = ftrace_events.event(); it; ++it) {
- events->push_back(*it);
- }
- }
-
- std::vector<protozero::ConstBytes> FindAllEvents(
- const std::string& data) const {
- protos::pbzero::Trace::Decoder decoder(data);
- std::vector<protozero::ConstBytes> events;
-
- for (auto it = decoder.packet(); it; ++it) {
- protos::pbzero::TracePacket::Decoder packet(*it);
- FindAllEvents(packet, &events);
- }
-
- return events;
- }
-
- static bool IsNotSwitchEvent(protozero::ConstBytes field) {
- protos::pbzero::FtraceEvent::Decoder event(field);
- return event.has_sched_switch();
- }
-};
-
-TEST_F(ScrubFtraceEventsIntegrationTest, FindsPackageAndFiltersPackageList) {
- ASSERT_OK(Redact());
-
- // Load unredacted trace - make sure there are non-allow-listed events.
- {
- auto raw = LoadOriginal();
- ASSERT_OK(raw);
-
- auto fields = FindAllEvents(*raw);
-
- // More than switch events should be found.
- auto it = std::find_if(fields.begin(), fields.end(), IsNotSwitchEvent);
- ASSERT_NE(it, fields.end());
- }
-
- // Load redacted trace - make sure there are only allow-listed events.
- {
- auto raw = LoadRedacted();
- ASSERT_OK(raw);
-
- auto field = FindAllEvents(*raw);
-
- // Only switch events should be found.
- auto it = std::find_if_not(field.begin(), field.end(), IsNotSwitchEvent);
- ASSERT_EQ(it, field.end());
- }
-}
-
-} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/trace_redaction_framework.h b/src/trace_redaction/trace_redaction_framework.h
index 86982a8..fc20f30 100644
--- a/src/trace_redaction/trace_redaction_framework.h
+++ b/src/trace_redaction/trace_redaction_framework.h
@@ -17,6 +17,7 @@
#ifndef SRC_TRACE_REDACTION_TRACE_REDACTION_FRAMEWORK_H_
#define SRC_TRACE_REDACTION_TRACE_REDACTION_FRAMEWORK_H_
+#include <bitset>
#include <cstdint>
#include <memory>
#include <optional>
@@ -257,9 +258,13 @@
// - protos::pbzero::TracePacket::kProcessStatsFieldNumber
// - protos::pbzero::TracePacket::kClockSnapshotFieldNumber
//
- // Because "data" is a "one of", if no field in "trace_packet_allow_list" can
- // be found, it packet should be removed.
- base::FlatSet<uint32_t> trace_packet_allow_list;
+ // If the mask is set to 0x00, all fields would be removed. This should not
+ // happen as some metadata provides context between packets.
+ //
+ // Important note, 128 is used because it's the first power-of-2 after
+ // TracePacket::kMaxFieldNumber.
+ using TracePacketMask = std::bitset<128>;
+ TracePacketMask packet_mask;
// Ftrace packets contain a "one of" entry called "event". Within the scope of
// a ftrace event, the event can be considered the payload and other other
@@ -298,7 +303,8 @@
//
// 3. In this example, a cpu_idle event populates the one-of slot in the
// ftrace event
- base::FlatSet<uint32_t> ftrace_packet_allow_list;
+ using FtraceEventMask = std::bitset<512>;
+ FtraceEventMask ftrace_mask;
// message SuspendResumeFtraceEvent {
// optional string action = 1 [(datapol.semantic_type) = ST_NOT_REQUIRED];
