From 941ba723baceac19151560e8a1d2830b9be6493c Mon Sep 17 00:00:00 2001
From: Ryan Savitski <rsavitski@google.com>
Date: Thu, 2 Feb 2023 14:24:45 +0000
Subject: [PATCH] sepolicy: rework perfetto producer/profiler rules for "user"
 builds

This patch:
* allows for heap and perf profiling of all processes on the system
  (minus undumpable and otherwise incompatible domains). For apps, the
  rest of the platform will still perform checks based on
  profileable/debuggable manifest flags. For native processes, the
  profilers will check that the process runs as an allowlisted UID.
* allows for all apps (=appdomain) to act as perfetto tracing data
  writers (=perfetto_producer) for the ART java heap graph plugin
  (perfetto_hprof).
* allows for system_server to act a perfetto_producer for java heap
  graphs.

Bug: 247858731
Change-Id: I792ec1812d94b4fa9a8688ed74f2f62f6a7f33a6
---
 private/app.te                  |  8 +++--
 private/app_zygote.te           | 11 ++----
 private/crash_dump.te           | 15 ++++----
 private/domain.te               | 61 ++++++++++++++++++---------------
 private/ephemeral_app.te        |  8 -----
 private/heapprofd.te            | 29 +++++++---------
 private/isolated_app.te         |  9 -----
 private/isolated_compute_app.te |  9 -----
 private/platform_app.te         |  3 --
 private/priv_app.te             |  8 -----
 private/sdk_sandbox.te          |  8 -----
 private/system_app.te           |  3 --
 private/system_server.te        |  4 ++-
 private/traced_perf.te          |  9 ++++-
 private/untrusted_app_all.te    |  8 -----
 public/hal_configstore.te       |  2 --
 16 files changed, 72 insertions(+), 123 deletions(-)

diff --git a/private/app.te b/private/app.te
index c8f455b6d..ff30fe5f2 100644
--- a/private/app.te
+++ b/private/app.te
@@ -58,8 +58,6 @@ get_prop(appdomain, camera2_extensions_prop)
 # Allow to ro.camerax.extensions.enabled
 get_prop(appdomain, camerax_extensions_prop)
 
-userdebug_or_eng(`perfetto_producer({ appdomain })')
-
 # Prevent apps from causing presubmit failures.
 # Apps can cause selinux denials by accessing CE storage
 # and/or external storage. In either case, the selinux denial is
@@ -358,6 +356,12 @@ allow appdomain user_profile_root_file:dir search;
 allow appdomain user_profile_data_file:dir w_dir_perms;
 allow appdomain user_profile_data_file:file create_file_perms;
 
+# Allow writing performance tracing data into the perfetto traced daemon.
+# Needed for java heap graph ART plugin (perfetto_hprof).
+# The perfetto profiling daemon will check for the specific application's
+# opt-in/opt-out.
+perfetto_producer(appdomain)
+
 # Send heap dumps to system_server via an already open file descriptor
 # % adb shell am set-watch-heap com.android.systemui 1048576
 # % adb shell dumpsys procstats --start-testing
diff --git a/private/app_zygote.te b/private/app_zygote.te
index 8aa288e4d..6552d6309 100644
--- a/private/app_zygote.te
+++ b/private/app_zygote.te
@@ -142,18 +142,15 @@ neverallow app_zygote domain:{
   alg_socket nfc_socket vsock_socket kcm_socket qipcrtr_socket smc_socket
 } *;
 
-# Only allow app_zygote to talk to the logd socket, and
-# su/heapprofd/traced_perf on eng/userdebug. This is because
-# cap_setuid/cap_setgid allow to forge uid/gid in SCM_CREDENTIALS.
-# Think twice before changing.
+# Only allow app_zygote to talk to the logd socket, and su on eng/userdebug.
+# This is because cap_setuid/cap_setgid allow to forge uid/gid in
+# SCM_CREDENTIALS. Think twice before changing.
 neverallow app_zygote {
   domain
   -app_zygote
   -logd
   -system_server
   userdebug_or_eng(`-su')
-  userdebug_or_eng(`-heapprofd')
-  userdebug_or_eng(`-traced_perf')
 }:unix_dgram_socket *;
 
 neverallow app_zygote {
@@ -161,8 +158,6 @@ neverallow app_zygote {
   -app_zygote
   -prng_seeder
   userdebug_or_eng(`-su')
-  userdebug_or_eng(`-heapprofd')
-  userdebug_or_eng(`-traced_perf')
 }:unix_stream_socket *;
 
 # Never allow ptrace
diff --git a/private/crash_dump.te b/private/crash_dump.te
index 31f01284b..bc6020e9f 100644
--- a/private/crash_dump.te
+++ b/private/crash_dump.te
@@ -30,13 +30,16 @@ userdebug_or_eng(`
   }:process { ptrace signal sigchld sigstop sigkill };
 ')
 
+# Read ART APEX data directory
+allow crash_dump apex_art_data_file:dir { getattr search };
+allow crash_dump apex_art_data_file:file r_file_perms;
+
 ###
 ### neverallow assertions
 ###
 
-# ptrace neverallow assertions are spread throughout the other policy
-# files, so we avoid adding redundant assertions here
-
+# sigchld not explicitly forbidden since it's part of the
+# domain-transition-on-exec macros, and is by itself not sensitive
 neverallow crash_dump {
   apexd
   userdebug_or_eng(`-apexd')
@@ -54,11 +57,7 @@ neverallow crash_dump {
   vendor_init
   vold
   userdebug_or_eng(`-vold')
-}:process { signal sigstop sigkill };
+}:process { ptrace signal sigstop sigkill };
 
 neverallow crash_dump self:process ptrace;
 neverallow crash_dump gpu_device:chr_file *;
-
-# Read ART APEX data directory
-allow crash_dump apex_art_data_file:dir { getattr search };
-allow crash_dump apex_art_data_file:file r_file_perms;
diff --git a/private/domain.te b/private/domain.te
index 953ddce1c..d9bcc6fd6 100644
--- a/private/domain.te
+++ b/private/domain.te
@@ -12,44 +12,49 @@ allow domain crash_dump:process sigchld;
 # heap profiling, as initialization will fail if it does not have the
 # necessary SELinux permissions.
 get_prop(domain, heapprofd_prop);
-# Allow heap profiling on debug builds.
-userdebug_or_eng(`can_profile_heap({
-  domain
-  -bpfloader
-  -init
-  -kernel
-  -keystore
-  -llkd
-  -logd
-  -logpersist
-  -recovery
-  -recovery_persist
-  -recovery_refresh
-  -ueventd
-  -vendor_init
-  -vold
-})')
 
-# As above, allow perf profiling most processes on debug builds.
-# zygote is excluded as system-wide profiling could end up with it
-# (unexpectedly) holding an open fd across a fork.
-userdebug_or_eng(`can_profile_perf({
+# See private/crash_dump.te
+define(`dumpable_domain',`{
   domain
+  -apexd
   -bpfloader
+  -crash_dump
+  -crosvm # TODO(b/236672526): Remove exception for crosvm
+  -diced
   -init
   -kernel
   -keystore
   -llkd
   -logd
+  -ueventd
+  -vendor_init
+  -vold
+}')
+
+# Allow heap profiling by heapprofd.
+# Zygotes are excluded due to potential issues with holding open file
+# descriptors or other state across forks. Other exclusions conflict with
+# neverallows, and are not considered important to profile.
+can_profile_heap({
+  dumpable_domain
+  -app_zygote
+  -hal_configstore
   -logpersist
   -recovery
   -recovery_persist
   -recovery_refresh
-  -ueventd
-  -vendor_init
-  -vold
+  -webview_zygote
   -zygote
-})')
+})
+
+# Allow profiling using perf_event_open by traced_perf.
+can_profile_perf({
+  dumpable_domain
+  -app_zygote
+  -hal_configstore
+  -webview_zygote
+  -zygote
+})
 
 # Everyone can access the IncFS list of features.
 r_dir_file(domain, sysfs_fs_incfs_features);
@@ -556,9 +561,9 @@ full_treble_only(`
     userdebug_or_eng(`-su') # communications with su are permitted only on userdebug or eng builds
     -init
     -tombstoned # linker to tombstoned
-    userdebug_or_eng(`-heapprofd')
-    userdebug_or_eng(`-traced')
-    userdebug_or_eng(`-traced_perf')
+    -heapprofd
+    -traced
+    -traced_perf
   });
 ')
 
diff --git a/private/ephemeral_app.te b/private/ephemeral_app.te
index 3b916e236..9f2b1d524 100644
--- a/private/ephemeral_app.te
+++ b/private/ephemeral_app.te
@@ -45,14 +45,6 @@ allow ephemeral_app drmserver_service:service_manager find;
 allow ephemeral_app radio_service:service_manager find;
 allow ephemeral_app ephemeral_app_api_service:service_manager find;
 
-# Write app-specific trace data to the Perfetto traced damon. This requires
-# connecting to its producer socket and obtaining a (per-process) tmpfs fd.
-perfetto_producer(ephemeral_app)
-
-# Allow profiling if the app opts in by being marked profileable/debuggable.
-can_profile_heap(ephemeral_app)
-can_profile_perf(ephemeral_app)
-
 # allow ephemeral apps to use UDP sockets provided by the system server but not
 # modify them other than to connect
 allow ephemeral_app system_server:udp_socket {
diff --git a/private/heapprofd.te b/private/heapprofd.te
index 36d2938be..1b41823c1 100644
--- a/private/heapprofd.te
+++ b/private/heapprofd.te
@@ -1,14 +1,4 @@
 # Android heap profiling daemon. go/heapprofd.
-#
-# On user builds, this daemon is responsible for receiving the initial
-# profiling configuration, finding matching target processes (if profiling by
-# process name), and sending the activation signal to them (+ setting system
-# properties for new processes to start profiling from startup). When profiling
-# is triggered in a process, it spawns a private heapprofd subprocess (in its
-# own SELinux domain), which will exclusively handle profiling of its parent.
-#
-# On debug builds, this central daemon performs profiling for all target
-# processes (which talk directly to this daemon).
 type heapprofd_exec, exec_type, file_type, system_file_type;
 type heapprofd_tmpfs, file_type;
 
@@ -56,23 +46,28 @@ allow heapprofd self:global_capability_class_set dac_read_search;
 # For checking profileability.
 allow heapprofd packages_list_file:file r_file_perms;
 
-# This is going to happen on user but is benign because central heapprofd
-# does not actually need these permission.
-# If the dac_read_search capability check is rejected, the kernel then tries
-# to perform a dac_override capability check, so we need to dontaudit that
-# as well.
-dontaudit heapprofd self:global_capability_class_set { dac_read_search dac_override };
-
+# Never allow profiling privileged or otherwise incompatible domains.
+# Corresponding allow-rule is in private/domain.te.
 never_profile_heap(`{
+  apexd
+  app_zygote
   bpfloader
+  diced
+  hal_configstore
   init
   kernel
   keystore
   llkd
   logd
+  logpersist
+  recovery
+  recovery_persist
+  recovery_refresh
   ueventd
   vendor_init
   vold
+  webview_zygote
+  zygote
 }')
 
 full_treble_only(`
diff --git a/private/isolated_app.te b/private/isolated_app.te
index 7230844c9..9d0fd73a1 100644
--- a/private/isolated_app.te
+++ b/private/isolated_app.te
@@ -34,12 +34,3 @@ allow isolated_app webview_zygote:process sigchld;
 allow isolated_app webview_zygote:unix_dgram_socket write;
 # Read system properties managed by webview_zygote.
 allow isolated_app webview_zygote_tmpfs:file read;
-
-# Write app-specific trace data to the Perfetto traced damon. This requires
-# connecting to its producer socket and obtaining a (per-process) tmpfs fd.
-perfetto_producer(isolated_app)
-
-# Allow profiling if the main app has been marked as profileable or
-# debuggable.
-can_profile_heap(isolated_app)
-can_profile_perf(isolated_app)
diff --git a/private/isolated_compute_app.te b/private/isolated_compute_app.te
index 2c6d570fe..536261fd6 100644
--- a/private/isolated_compute_app.te
+++ b/private/isolated_compute_app.te
@@ -25,15 +25,6 @@ allow isolated_compute_app speech_recognition_service:service_manager find;
 hal_client_domain(isolated_compute_app, hal_allocator)
 hwbinder_use(isolated_compute_app)
 
-# Write app-specific trace data to the Perfetto traced damon. This requires
-# connecting to its producer socket and obtaining a (per-process) tmpfs fd.
-perfetto_producer(isolated_compute_app)
-
-# Allow profiling if the main app has been marked as profileable or
-# debuggable.
-can_profile_heap(isolated_compute_app)
-can_profile_perf(isolated_compute_app)
-
 #####
 ##### Neverallow
 #####
diff --git a/private/platform_app.te b/private/platform_app.te
index 46abb1600..5d16d858b 100644
--- a/private/platform_app.te
+++ b/private/platform_app.te
@@ -115,9 +115,6 @@ allow platform_app app_data_file:lnk_file create_file_perms;
 # suppress denials caused by debugfs_tracing
 dontaudit platform_app debugfs_tracing:file rw_file_perms;
 
-# Allow platform apps to act as Perfetto producers.
-perfetto_producer(platform_app)
-
 # Allow platform apps to create VMs
 virtualizationservice_use(platform_app)
 
diff --git a/private/priv_app.te b/private/priv_app.te
index 8c965fce1..cfd872153 100644
--- a/private/priv_app.te
+++ b/private/priv_app.te
@@ -126,20 +126,12 @@ allow priv_app preloads_media_file:dir r_dir_perms;
 
 read_runtime_log_tags(priv_app)
 
-# Write app-specific trace data to the Perfetto traced damon. This requires
-# connecting to its producer socket and obtaining a (per-process) tmpfs fd.
-perfetto_producer(priv_app)
-
 # Allow priv_apps to request and collect incident reports.
 # (Also requires DUMP and PACKAGE_USAGE_STATS permissions)
 allow priv_app incident_service:service_manager find;
 binder_call(priv_app, incidentd)
 allow priv_app incidentd:fifo_file { read write };
 
-# Allow profiling if the app opts in by being marked profileable/debuggable.
-can_profile_heap(priv_app)
-can_profile_perf(priv_app)
-
 # Allow priv_apps to check whether Dynamic System Update is enabled
 get_prop(priv_app, dynamic_system_prop)
 
diff --git a/private/sdk_sandbox.te b/private/sdk_sandbox.te
index 6ebfa0aee..cfcf2a4ff 100644
--- a/private/sdk_sandbox.te
+++ b/private/sdk_sandbox.te
@@ -218,14 +218,6 @@ allow sdk_sandbox system_linker_exec:file execute_no_trans;
 allow sdk_sandbox shell_data_file:file r_file_perms;
 allow sdk_sandbox shell_data_file:dir r_dir_perms;
 
-# Write app-specific trace data to the Perfetto traced damon. This requires
-# connecting to its producer socket and obtaining a (per-process) tmpfs fd.
-perfetto_producer(sdk_sandbox)
-
-# Allow profiling if the app opts in by being marked profileable/debuggable.
-can_profile_heap(sdk_sandbox)
-can_profile_perf(sdk_sandbox)
-
 # allow sdk sandbox to use UDP sockets provided by the system server but not
 # modify them other than to connect
 allow sdk_sandbox system_server:udp_socket {
diff --git a/private/system_app.te b/private/system_app.te
index 9116058c5..e2bec305e 100644
--- a/private/system_app.te
+++ b/private/system_app.te
@@ -180,9 +180,6 @@ get_prop(system_app, oem_unlock_prop)
 # Settings app reads ro.usb.uvc.enabled
 get_prop(system_app, usb_uvc_enabled_prop)
 
-# Allow system apps to act as Perfetto producers.
-perfetto_producer(system_app)
-
 ###
 ### Neverallow rules
 ###
diff --git a/private/system_server.te b/private/system_server.te
index 62185fc91..4e5b2e8de 100644
--- a/private/system_server.te
+++ b/private/system_server.te
@@ -421,7 +421,9 @@ allow system_server mediaserver:udp_socket rw_socket_perms;
 allow system_server mediadrmserver:tcp_socket rw_socket_perms;
 allow system_server mediadrmserver:udp_socket rw_socket_perms;
 
-userdebug_or_eng(`perfetto_producer({ system_server })')
+# Write trace data to the Perfetto traced daemon. This requires connecting to
+# its producer socket and obtaining a (per-process) tmpfs fd.
+perfetto_producer(system_server)
 
 # Get file context
 allow system_server file_contexts_file:file r_file_perms;
diff --git a/private/traced_perf.te b/private/traced_perf.te
index 811bf483f..080b6feaa 100644
--- a/private/traced_perf.te
+++ b/private/traced_perf.te
@@ -60,9 +60,14 @@ dontaudit traced_perf domain:process signal;
 # Never allow access to app data files
 neverallow traced_perf { app_data_file privapp_data_file system_app_data_file }:file *;
 
-# Never allow profiling highly privileged processes.
+# Never allow profiling privileged or otherwise incompatible domains.
+# Corresponding allow-rule is in private/domain.te.
 never_profile_perf(`{
+  apexd
+  app_zygote
   bpfloader
+  diced
+  hal_configstore
   init
   kernel
   keystore
@@ -71,4 +76,6 @@ never_profile_perf(`{
   ueventd
   vendor_init
   vold
+  webview_zygote
+  zygote
 }')
diff --git a/private/untrusted_app_all.te b/private/untrusted_app_all.te
index 8c7fe7a58..f666cc8c1 100644
--- a/private/untrusted_app_all.te
+++ b/private/untrusted_app_all.te
@@ -129,14 +129,6 @@ allow untrusted_app_all vendor_app_file:dir { open getattr read search };
 allow untrusted_app_all vendor_app_file:file { r_file_perms execute };
 allow untrusted_app_all vendor_app_file:lnk_file { open getattr read };
 
-# Write app-specific trace data to the Perfetto traced damon. This requires
-# connecting to its producer socket and obtaining a (per-process) tmpfs fd.
-perfetto_producer(untrusted_app_all)
-
-# Allow profiling if the app opts in by being marked profileable/debuggable.
-can_profile_heap(untrusted_app_all)
-can_profile_perf(untrusted_app_all)
-
 # allow untrusted apps to use UDP sockets provided by the system server but not
 # modify them other than to connect
 allow untrusted_app_all system_server:udp_socket {
diff --git a/public/hal_configstore.te b/public/hal_configstore.te
index 7d4d150c3..8867a8de8 100644
--- a/public/hal_configstore.te
+++ b/public/hal_configstore.te
@@ -34,8 +34,6 @@ neverallow hal_configstore_server {
   -prng_seeder
   userdebug_or_eng(`-su')
   -tombstoned
-  userdebug_or_eng(`-heapprofd')
-  userdebug_or_eng(`-traced_perf')
 }:{ unix_dgram_socket unix_stream_socket } *;
 
 # Should never need access to anything on /data