Trace Redaction - Remove comm value from new task events
When a new thread starts, a new task event is triggered. This event
contains the threads pid and the threads name (comm value). All comm
values outside of the target package should be removed.
Bug: 318576499
Change-Id: I76bd50645e2e507fa9d88e71747da6f95f8d1655
diff --git a/Android.bp b/Android.bp
index 928eee1..c136ed5 100644
--- a/Android.bp
+++ b/Android.bp
@@ -12870,6 +12870,7 @@
"src/trace_redaction/prune_package_list.cc",
"src/trace_redaction/redact_ftrace_event.cc",
"src/trace_redaction/redact_sched_switch.cc",
+ "src/trace_redaction/redact_task_newtask.cc",
"src/trace_redaction/scrub_ftrace_events.cc",
"src/trace_redaction/scrub_process_stats.cc",
"src/trace_redaction/scrub_process_trees.cc",
@@ -12893,6 +12894,7 @@
"src/trace_redaction/proto_util_unittest.cc",
"src/trace_redaction/prune_package_list_unittest.cc",
"src/trace_redaction/redact_sched_switch_unittest.cc",
+ "src/trace_redaction/redact_task_newtask_unittest.cc",
],
}
diff --git a/src/trace_redaction/BUILD.gn b/src/trace_redaction/BUILD.gn
index 55e159d..bf254c9 100644
--- a/src/trace_redaction/BUILD.gn
+++ b/src/trace_redaction/BUILD.gn
@@ -56,6 +56,8 @@
"redact_ftrace_event.h",
"redact_sched_switch.cc",
"redact_sched_switch.h",
+ "redact_task_newtask.cc",
+ "redact_task_newtask.h",
"scrub_ftrace_events.cc",
"scrub_ftrace_events.h",
"scrub_process_stats.cc",
@@ -123,6 +125,7 @@
"proto_util_unittest.cc",
"prune_package_list_unittest.cc",
"redact_sched_switch_unittest.cc",
+ "redact_task_newtask_unittest.cc",
]
deps = [
":trace_redaction",
diff --git a/src/trace_redaction/main.cc b/src/trace_redaction/main.cc
index e964fc1..a8a1cdc 100644
--- a/src/trace_redaction/main.cc
+++ b/src/trace_redaction/main.cc
@@ -28,6 +28,7 @@
#include "src/trace_redaction/prune_package_list.h"
#include "src/trace_redaction/redact_ftrace_event.h"
#include "src/trace_redaction/redact_sched_switch.h"
+#include "src/trace_redaction/redact_task_newtask.h"
#include "src/trace_redaction/scrub_ftrace_events.h"
#include "src/trace_redaction/scrub_process_stats.h"
#include "src/trace_redaction/scrub_process_trees.h"
@@ -70,6 +71,7 @@
auto* redact_ftrace_events = redactor.emplace_transform<RedactFtraceEvent>();
redact_ftrace_events->emplace_back<RedactSchedSwitch>();
+ redact_ftrace_events->emplace_back<RedactTaskNewTask>();
Context context;
context.package_name = package_name;
diff --git a/src/trace_redaction/redact_task_newtask.cc b/src/trace_redaction/redact_task_newtask.cc
new file mode 100644
index 0000000..ed50fbb
--- /dev/null
+++ b/src/trace_redaction/redact_task_newtask.cc
@@ -0,0 +1,105 @@
+/*
+ * 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/redact_task_newtask.h"
+
+#include <string>
+
+#include "src/trace_redaction/proto_util.h"
+
+#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h"
+#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
+#include "protos/perfetto/trace/ftrace/task.pbzero.h"
+
+namespace perfetto::trace_redaction {
+
+// Redact sched switch trace events in an ftrace event bundle:
+//
+// event {
+// timestamp: 6702094133317685
+// pid: 6167
+// task_newtask {
+// pid: 7972
+// comm: "adbd"
+// clone_flags: 4001536
+// oom_score_adj: -1000
+// }
+// }
+//
+// In the above message, it should be noted that "event.pid" will never be
+// equal to "event.task_newtask.pid" (a thread cannot start itself).
+
+// TODO(vaage): How does this primitive (and others like it) work when we're
+// merging threads? Remame events are already dropped. New task and proces free
+// events won't matter the timeline is created. Can these events be dropped?
+
+RedactTaskNewTask::RedactTaskNewTask()
+ : FtraceEventRedaction(
+ protos::pbzero::FtraceEvent::kTaskNewtaskFieldNumber) {}
+
+base::Status RedactTaskNewTask::Redact(
+ const Context& context,
+ const protos::pbzero::FtraceEvent::Decoder& event,
+ protozero::ConstBytes bytes,
+ protos::pbzero::FtraceEvent* event_message) const {
+ if (!context.package_uid.has_value()) {
+ return base::Status("RedactTaskNewTask: missing package uid");
+ }
+
+ if (!context.timeline) {
+ return base::Status("RedactTaskNewTask: missing timeline");
+ }
+
+ // There must be a pid. If not, the message is meaningless and can be dropped.
+ if (!event.has_timestamp()) {
+ return base::OkStatus();
+ }
+
+ protozero::ProtoDecoder new_task(bytes);
+
+ auto pid = new_task.FindField(
+ protos::pbzero::TaskNewtaskFtraceEvent::kPidFieldNumber);
+
+ if (!pid.valid()) {
+ return base::OkStatus();
+ }
+
+ // Avoid making the message until we know that we have prev and next pids.
+ auto* new_task_message = event_message->set_task_newtask();
+
+ auto slice = context.timeline->Search(event.timestamp(), pid.as_int32());
+
+ for (auto field = new_task.ReadField(); field.valid();
+ field = new_task.ReadField()) {
+ if (field.id() ==
+ protos::pbzero::TaskNewtaskFtraceEvent::kCommFieldNumber) {
+ if (slice.uid == context.package_uid) {
+ proto_util::AppendField(field, new_task_message);
+ } else {
+ // Perfetto view (ui.perfetto.dev) crashes if the comm value is missing.
+ // To work around this, the comm value is replaced with an empty string.
+ // This appears to work.
+ new_task_message->set_comm("");
+ }
+ } else {
+ proto_util::AppendField(field, new_task_message);
+ }
+ }
+
+ return base::OkStatus();
+}
+
+} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/redact_task_newtask.h b/src/trace_redaction/redact_task_newtask.h
new file mode 100644
index 0000000..b4fd830
--- /dev/null
+++ b/src/trace_redaction/redact_task_newtask.h
@@ -0,0 +1,40 @@
+/*
+ * 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_REDACT_TASK_NEWTASK_H_
+#define SRC_TRACE_REDACTION_REDACT_TASK_NEWTASK_H_
+
+#include "src/trace_redaction/redact_ftrace_event.h"
+#include "src/trace_redaction/trace_redaction_framework.h"
+
+namespace perfetto::trace_redaction {
+
+// Goes through ftrace events and conditonally removes the comm values from
+// task_newtask events.
+class RedactTaskNewTask : public FtraceEventRedaction {
+ public:
+ RedactTaskNewTask();
+
+ base::Status Redact(
+ const Context& context,
+ const protos::pbzero::FtraceEvent::Decoder& event,
+ protozero::ConstBytes bytes,
+ protos::pbzero::FtraceEvent* event_message) const override;
+};
+
+} // namespace perfetto::trace_redaction
+
+#endif // SRC_TRACE_REDACTION_REDACT_TASK_NEWTASK_H_
diff --git a/src/trace_redaction/redact_task_newtask_unittest.cc b/src/trace_redaction/redact_task_newtask_unittest.cc
new file mode 100644
index 0000000..d09f044
--- /dev/null
+++ b/src/trace_redaction/redact_task_newtask_unittest.cc
@@ -0,0 +1,156 @@
+/*
+ * 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/redact_task_newtask.h"
+#include "perfetto/protozero/scattered_heap_buffer.h"
+#include "test/gtest_and_gmock.h"
+
+#include "protos/perfetto/trace/ftrace/ftrace_event.gen.h"
+#include "protos/perfetto/trace/ftrace/task.gen.h"
+#include "protos/perfetto/trace/ftrace/task.pbzero.h"
+
+namespace perfetto::trace_redaction {
+
+namespace {
+constexpr uint64_t kUidA = 1;
+constexpr uint64_t kUidB = 2;
+
+constexpr int32_t kNoParent = 10;
+constexpr int32_t kPidA = 11;
+constexpr int32_t kPidB = 12;
+
+constexpr std::string_view kCommA = "comm-a";
+
+} // namespace
+
+// Tests which nested messages and fields are removed.
+class RedactTaskNewTaskTest : public testing::Test {
+ protected:
+ void SetUp() override {
+ timeline_ = std::make_unique<ProcessThreadTimeline>();
+ timeline_->Append(
+ ProcessThreadTimeline::Event::Open(0, kPidA, kNoParent, kUidA));
+ timeline_->Append(
+ ProcessThreadTimeline::Event::Open(0, kPidB, kNoParent, kUidB));
+ timeline_->Sort();
+
+ // This test breaks the rules for task_newtask and the timeline. The
+ // timeline will report the task existing before the new task event. This
+ // should not happen in the field, but it makes the test more robust.
+ protozero::HeapBuffered<protos::pbzero::FtraceEvent> event;
+ event->set_timestamp(123456789);
+ event->set_pid(kPidA);
+
+ auto* new_task = event->set_task_newtask();
+ new_task->set_comm(kCommA.data(), kCommA.size());
+ new_task->set_pid(kPidA);
+
+ event_string_ = event.SerializeAsString();
+ }
+
+ const std::string& event_string() const { return event_string_; }
+
+ std::unique_ptr<ProcessThreadTimeline> timeline() {
+ return std::move(timeline_);
+ }
+
+ private:
+ std::string event_string_;
+
+ std::unique_ptr<ProcessThreadTimeline> timeline_;
+};
+
+TEST_F(RedactTaskNewTaskTest, RejectMissingPackageUid) {
+ RedactTaskNewTask redact;
+
+ Context context;
+ context.timeline = std::make_unique<ProcessThreadTimeline>();
+
+ protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
+ protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
+
+ auto result =
+ redact.Redact(context, event_decoder, event_decoder.task_newtask(),
+ event_message.get());
+ ASSERT_FALSE(result.ok());
+}
+
+TEST_F(RedactTaskNewTaskTest, RejectMissingTimeline) {
+ RedactTaskNewTask redact;
+
+ Context context;
+ context.package_uid = kUidA;
+
+ protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
+ protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
+
+ auto result =
+ redact.Redact(context, event_decoder, event_decoder.task_newtask(),
+ event_message.get());
+ ASSERT_FALSE(result.ok());
+}
+
+TEST_F(RedactTaskNewTaskTest, PidInPackageKeepsComm) {
+ RedactTaskNewTask redact;
+
+ // Because Uid A is the target, when Pid A starts (new task event), it should
+ // keep its comm value.
+ Context context;
+ context.package_uid = kUidA;
+ context.timeline = timeline();
+
+ protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
+ protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
+
+ auto result =
+ redact.Redact(context, event_decoder, event_decoder.task_newtask(),
+ event_message.get());
+ ASSERT_TRUE(result.ok());
+
+ protos::gen::FtraceEvent redacted_event;
+ redacted_event.ParseFromString(event_message.SerializeAsString());
+
+ ASSERT_TRUE(redacted_event.has_task_newtask());
+ ASSERT_TRUE(redacted_event.task_newtask().has_comm());
+ ASSERT_EQ(redacted_event.task_newtask().comm(), kCommA);
+}
+
+TEST_F(RedactTaskNewTaskTest, PidOutsidePackageLosesComm) {
+ RedactTaskNewTask redact;
+
+ // Because Uid B is the target, when Pid A starts (new task event), it should
+ // lose its comm value.
+ Context context;
+ context.package_uid = kUidB;
+ context.timeline = timeline();
+
+ protos::pbzero::FtraceEvent::Decoder event_decoder(event_string());
+ protozero::HeapBuffered<protos::pbzero::FtraceEvent> event_message;
+
+ auto result =
+ redact.Redact(context, event_decoder, event_decoder.task_newtask(),
+ event_message.get());
+ ASSERT_TRUE(result.ok());
+
+ protos::gen::FtraceEvent redacted_event;
+ redacted_event.ParseFromString(event_message.SerializeAsString());
+
+ ASSERT_TRUE(redacted_event.has_task_newtask());
+ ASSERT_TRUE(redacted_event.task_newtask().has_comm());
+ ASSERT_TRUE(redacted_event.task_newtask().comm().empty());
+}
+
+} // namespace perfetto::trace_redaction