traced: allow cross-session cloning for bugreports
Prior changed introduced generalized support for CloneSession
and rewrote the --save-for-bugreport feature on top of that.
Unfortunately we didn't notice that the CloneSession feature
is very strict on UID checking, enforcing that source and target
consumer UID match. This is good in general but not desirable
(And a regression) for the case of bugreports, where the two
uids differ.
In this case bugreport_score acts as an "I am fine exporting
this trace to other uids".
Bug: 286908237
Bug: 260112703
(cherry picked from https://android-review.googlesource.com/q/commit:5457722190e8cad7dddbedc71b8957ab45402a6c)
Merged-In: I99bd22aef0bfa402cb24559b8515c5bdf0b48ba9
Change-Id: I99bd22aef0bfa402cb24559b8515c5bdf0b48ba9
diff --git a/protos/perfetto/config/perfetto_config.proto b/protos/perfetto/config/perfetto_config.proto
index 85f1de7..5fbcf8d 100644
--- a/protos/perfetto/config/perfetto_config.proto
+++ b/protos/perfetto/config/perfetto_config.proto
@@ -2935,13 +2935,18 @@
// trace ends to notify it about the trace readiness.
optional bool notify_traceur = 16;
+ // This field was introduced in Android S.
// Android-only. If set to a value > 0, marks the trace session as a candidate
// for being attached to a bugreport. This field effectively acts as a z-index
// for bugreports. When Android's dumpstate runs perfetto
// --save-for-bugreport, traced will pick the tracing session with the highest
- // score (score <= 0 is ignored), will steal its contents, save the trace into
- // a known path and stop prematurely.
- // This field was introduced in Android S.
+ // score (score <= 0 is ignored) and:
+ // On Android S, T: will steal its contents, save the trace into
+ // a known path and stop prematurely.
+ // On Android U+: will create a read-only snapshot and save that into a known
+ // path, without stoppin the original tracing session.
+ // When this field is set the tracing session becomes eligible to be cloned
+ // by other UIDs.
optional int32 bugreport_score = 30;
// Triggers allow producers to start or stop the tracing session when an event
diff --git a/protos/perfetto/config/trace_config.proto b/protos/perfetto/config/trace_config.proto
index 4fe3ea9..67c5f3d 100644
--- a/protos/perfetto/config/trace_config.proto
+++ b/protos/perfetto/config/trace_config.proto
@@ -256,13 +256,18 @@
// trace ends to notify it about the trace readiness.
optional bool notify_traceur = 16;
+ // This field was introduced in Android S.
// Android-only. If set to a value > 0, marks the trace session as a candidate
// for being attached to a bugreport. This field effectively acts as a z-index
// for bugreports. When Android's dumpstate runs perfetto
// --save-for-bugreport, traced will pick the tracing session with the highest
- // score (score <= 0 is ignored), will steal its contents, save the trace into
- // a known path and stop prematurely.
- // This field was introduced in Android S.
+ // score (score <= 0 is ignored) and:
+ // On Android S, T: will steal its contents, save the trace into
+ // a known path and stop prematurely.
+ // On Android U+: will create a read-only snapshot and save that into a known
+ // path, without stoppin the original tracing session.
+ // When this field is set the tracing session becomes eligible to be cloned
+ // by other UIDs.
optional int32 bugreport_score = 30;
// Triggers allow producers to start or stop the tracing session when an event
diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto
index e886311..77ff041 100644
--- a/protos/perfetto/trace/perfetto_trace.proto
+++ b/protos/perfetto/trace/perfetto_trace.proto
@@ -2935,13 +2935,18 @@
// trace ends to notify it about the trace readiness.
optional bool notify_traceur = 16;
+ // This field was introduced in Android S.
// Android-only. If set to a value > 0, marks the trace session as a candidate
// for being attached to a bugreport. This field effectively acts as a z-index
// for bugreports. When Android's dumpstate runs perfetto
// --save-for-bugreport, traced will pick the tracing session with the highest
- // score (score <= 0 is ignored), will steal its contents, save the trace into
- // a known path and stop prematurely.
- // This field was introduced in Android S.
+ // score (score <= 0 is ignored) and:
+ // On Android S, T: will steal its contents, save the trace into
+ // a known path and stop prematurely.
+ // On Android U+: will create a read-only snapshot and save that into a known
+ // path, without stoppin the original tracing session.
+ // When this field is set the tracing session becomes eligible to be cloned
+ // by other UIDs.
optional int32 bugreport_score = 30;
// Triggers allow producers to start or stop the tracing session when an event
diff --git a/src/tracing/core/tracing_service_impl.cc b/src/tracing/core/tracing_service_impl.cc
index b32a658..7c687f5 100644
--- a/src/tracing/core/tracing_service_impl.cc
+++ b/src/tracing/core/tracing_service_impl.cc
@@ -3580,8 +3580,13 @@
"The consumer is already attached to another tracing session");
}
- if (src->consumer_uid != consumer->uid_ && consumer->uid_ != 0)
+ // Skip the UID check for sessions marked with a bugreport_score > 0.
+ // Those sessions, by design, can be stolen by any other consumer for the
+ // sake of creating snapshots for bugreports.
+ if (src->config.bugreport_score() <= 0 &&
+ src->consumer_uid != consumer->uid_ && consumer->uid_ != 0) {
return PERFETTO_SVC_ERR("Not allowed to clone a session from another UID");
+ }
// First clone all TraceBuffer(s). This can fail because of ENOMEM. If it
// happens bail out early before creating any session.
diff --git a/src/tracing/core/tracing_service_impl_unittest.cc b/src/tracing/core/tracing_service_impl_unittest.cc
index fb175e9..9b6b6ae 100644
--- a/src/tracing/core/tracing_service_impl_unittest.cc
+++ b/src/tracing/core/tracing_service_impl_unittest.cc
@@ -4529,6 +4529,64 @@
Property(&protos::gen::TraceUuid::lsb, Eq(clone_uuid.lsb()))))));
}
+// Test that a consumer cannot clone a session from a consumer with a different
+// uid (unless it's marked as eligible for bugreport, see next test).
+TEST_F(TracingServiceImplTest, CloneSessionAcrossUidDenied) {
+ // The consumer the creates the initial tracing session.
+ std::unique_ptr<MockConsumer> consumer = CreateMockConsumer();
+ consumer->Connect(svc.get());
+
+ // The consumer that clones it and reads back the data.
+ std::unique_ptr<MockConsumer> consumer2 = CreateMockConsumer();
+ consumer2->Connect(svc.get(), 1234);
+
+ TraceConfig trace_config;
+ trace_config.add_buffers()->set_size_kb(32);
+
+ consumer->EnableTracing(trace_config);
+ auto flush_request = consumer->Flush();
+ ASSERT_TRUE(flush_request.WaitForReply());
+
+ auto clone_done = task_runner.CreateCheckpoint("clone_done");
+ EXPECT_CALL(*consumer2, OnSessionCloned(_))
+ .WillOnce(Invoke([clone_done](const Consumer::OnSessionClonedArgs& args) {
+ clone_done();
+ ASSERT_FALSE(args.success);
+ ASSERT_TRUE(base::Contains(args.error, "session from another UID"));
+ }));
+ consumer2->CloneSession(1);
+ task_runner.RunUntilCheckpoint("clone_done");
+}
+
+// Test that a consumer can clone a session from a different uid if the trace is
+// marked as eligible for bugreport.
+TEST_F(TracingServiceImplTest, CloneSessionAcrossUidForBugreport) {
+ // The consumer the creates the initial tracing session.
+ std::unique_ptr<MockConsumer> consumer = CreateMockConsumer();
+ consumer->Connect(svc.get());
+
+ // The consumer that clones it and reads back the data.
+ std::unique_ptr<MockConsumer> consumer2 = CreateMockConsumer();
+ consumer2->Connect(svc.get(), 1234);
+
+ TraceConfig trace_config;
+ trace_config.add_buffers()->set_size_kb(32);
+ trace_config.set_bugreport_score(1);
+
+ consumer->EnableTracing(trace_config);
+ auto flush_request = consumer->Flush();
+ ASSERT_TRUE(flush_request.WaitForReply());
+
+ auto clone_done = task_runner.CreateCheckpoint("clone_done");
+ EXPECT_CALL(*consumer2, OnSessionCloned(_))
+ .WillOnce(Invoke([clone_done](const Consumer::OnSessionClonedArgs& args) {
+ clone_done();
+ ASSERT_TRUE(args.success);
+ }));
+ consumer2->CloneSession(1);
+ task_runner.RunUntilCheckpoint("clone_done");
+}
+
TEST_F(TracingServiceImplTest, InvalidBufferSizes) {
std::unique_ptr<MockConsumer> consumer = CreateMockConsumer();
consumer->Connect(svc.get());
