Revert^3 "Trace Redaction - Reject invalid traces"
This reverts commit 719510ee9ccdf29748623245a56b315bade35c32.
Reason for revert: The test calling system trace is providing an invalid trace, resulting in the test failing now that the trace is verified before redacting.
Change-Id: Id4c849e2c24421258be22b5861bbc7c430c527fc
diff --git a/Android.bp b/Android.bp
index 9aa955d..54b1f20 100644
--- a/Android.bp
+++ b/Android.bp
@@ -13589,7 +13589,6 @@
"src/trace_redaction/suspend_resume.cc",
"src/trace_redaction/trace_redaction_framework.cc",
"src/trace_redaction/trace_redactor.cc",
- "src/trace_redaction/verify_integrity.cc",
],
}
diff --git a/src/trace_redaction/BUILD.gn b/src/trace_redaction/BUILD.gn
index ee876b0..b4aecb6 100644
--- a/src/trace_redaction/BUILD.gn
+++ b/src/trace_redaction/BUILD.gn
@@ -81,8 +81,6 @@
"trace_redaction_framework.h",
"trace_redactor.cc",
"trace_redactor.h",
- "verify_integrity.cc",
- "verify_integrity.h",
]
deps = [
"../../gn:default_deps",
diff --git a/src/trace_redaction/main.cc b/src/trace_redaction/main.cc
index 798d1a0..78bf3d0 100644
--- a/src/trace_redaction/main.cc
+++ b/src/trace_redaction/main.cc
@@ -39,7 +39,6 @@
#include "src/trace_redaction/suspend_resume.h"
#include "src/trace_redaction/trace_redaction_framework.h"
#include "src/trace_redaction/trace_redactor.h"
-#include "src/trace_redaction/verify_integrity.h"
namespace perfetto::trace_redaction {
@@ -49,12 +48,6 @@
std::string_view package_name) {
TraceRedactor redactor;
- // VerifyIntegrity breaks the CollectPrimitive pattern. Instead of writing to
- // the context, its job is to read trace packets and return errors if any
- // packet does not look "correct". This primitive is added first in an effort
- // to detect and react to bad input before other collectors run.
- redactor.emplace_collect<VerifyIntegrity>();
-
// Add all collectors.
redactor.emplace_collect<FindPackageUid>();
redactor.emplace_collect<CollectTimelineEvents>();
diff --git a/src/trace_redaction/verify_integrity.cc b/src/trace_redaction/verify_integrity.cc
deleted file mode 100644
index a2d58ce..0000000
--- a/src/trace_redaction/verify_integrity.cc
+++ /dev/null
@@ -1,103 +0,0 @@
-/*
- * Copyright (C) 2024 The Android Open Source Projectf
- *
- * 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/verify_integrity.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_packet.pbzero.h"
-#include "src/trace_processor/util/status_macros.h"
-
-namespace perfetto::trace_redaction {
-namespace {
-// All constants are from
-// "system/core/libcutils/include/private/android_filesystem_config.h"
-//
-// AID 1000 == system (those are probably frame_timeline packets you will see
-// those on) AID 9999 is nobody == traced/traced_probes
-constexpr int32_t kAidSystem = 1000;
-constexpr int32_t kAidNobody = 9999;
-} // namespace
-
-base::Status VerifyIntegrity::Collect(
- const protos::pbzero::TracePacket::Decoder& packet,
- Context*) const {
- if (!packet.has_trusted_uid()) {
- return base::ErrStatus(
- "VerifyIntegrity: missing field (TracePacket.trusted_uid).");
- }
-
- if (packet.trusted_uid() != kAidSystem &&
- packet.trusted_uid() != kAidNobody) {
- return base::ErrStatus(
- "VerifyIntegrity: invalid field value (TracePacket.trusted_uid).");
- }
-
- if (packet.has_ftrace_events()) {
- protos::pbzero::FtraceEventBundle::Decoder ftrace_events(
- packet.ftrace_events());
-
- // The other clocks in ftrace are only used on very old kernel versions. No
- // device with V should have such an old version. As a failsafe though,
- // check that the ftrace_clock field is unset to ensure no invalid
- // timestamps get by.
- if (ftrace_events.has_ftrace_clock()) {
- return base::ErrStatus(
- "VerifyIntegrity: unexpected field "
- "(FtraceEventBundle::kFtraceClockFieldNumber).");
- }
-
- // Every ftrace event bundle should have a CPU field. This is necessary for
- // switch/waking redaction to work.
- if (!ftrace_events.has_cpu()) {
- return base::ErrStatus(
- "VerifyIntegrity: missing field "
- "(FtraceEventBundle::kCpuFieldNumber).");
- }
-
- RETURN_IF_ERROR(VerifyFtraceEventsTime(ftrace_events));
- }
-
- return base::OkStatus();
-}
-
-base::Status VerifyIntegrity::VerifyFtraceEventsTime(
- const protos::pbzero::FtraceEventBundle::Decoder& bundle) const {
- // If a bundle has ftrace events, the events will contain the time stamps.
- // However, there are no ftrace events, the timestamp will be in the bundle.
- if (!bundle.has_event() && !bundle.has_ftrace_timestamp()) {
- return base::ErrStatus(
- "VerifyIntegrity: missing field "
- "(FtraceEventBundle::kFtraceTimestampFieldNumber).");
- }
-
- for (auto event_buffer = bundle.event(); event_buffer; ++event_buffer) {
- protos::pbzero::FtraceEvent::Decoder event(*event_buffer);
-
- if (!protozero::ProtoDecoder(*event_buffer)
- .FindField(protos::pbzero::FtraceEvent::kTimestampFieldNumber)
- .valid()) {
- return base::ErrStatus(
- "VerifyIntegrity: missing field "
- "(FtraceEvent::kTimestampFieldNumber)");
- }
- }
-
- return base::OkStatus();
-}
-
-} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/verify_integrity.h b/src/trace_redaction/verify_integrity.h
deleted file mode 100644
index 6548ce5..0000000
--- a/src/trace_redaction/verify_integrity.h
+++ /dev/null
@@ -1,42 +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 "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h"
-#include "src/trace_redaction/trace_redaction_framework.h"
-
-#include "protos/perfetto/trace/trace_packet.pbzero.h"
-
-#ifndef SRC_TRACE_REDACTION_VERIFY_INTEGRITY_H_
-#define SRC_TRACE_REDACTION_VERIFY_INTEGRITY_H_
-
-namespace perfetto::trace_redaction {
-
-// This breaks the normal collect primitive pattern. Rather than collecting
-// information, it looks at packets and returns an error if the packet violates
-// any requirements.
-class VerifyIntegrity : public CollectPrimitive {
- public:
- base::Status Collect(const protos::pbzero::TracePacket::Decoder& packet,
- Context* context) const override;
-
- private:
- base::Status VerifyFtraceEventsTime(
- const protos::pbzero::FtraceEventBundle::Decoder& bundle) const;
-};
-
-} // namespace perfetto::trace_redaction
-
-#endif // SRC_TRACE_REDACTION_VERIFY_INTEGRITY_H_
