From 7baf725ea63c9db5a360ba3d4516c30e2ad18d4a Mon Sep 17 00:00:00 2001
From: Steven Moreland <smoreland@google.com>
Date: Fri, 25 May 2018 16:23:37 -0700
Subject: [PATCH] mediacodec->mediacodec+hal_omx{,_server,_client}

(breaks vendor blobs, will have to be regenerated
after this CL)

This moves mediacodec to vendor so it is replaced with
hal_omx_server. The main benefit of this is that someone
can create their own implementation of mediacodec without
having to alter the one in the tree. mediacodec is still
seccomp enforced by CTS tests.

Fixes: 36375899
Test: (sanity) YouTube
Test: (sanity) camera pics + video
Test: check for denials
Change-Id: I31f91b7ad6cd0a891a1681ff3b9af82ab400ce5e
---
 private/app_neverallows.te   |  1 +
 private/bug_map              |  1 -
 private/compat/26.0/26.0.cil |  2 ++
 private/compat/27.0/27.0.cil |  2 ++
 private/incidentd.te         |  2 +-
 private/mediaserver.te       |  6 +---
 private/system_server.te     | 12 +++----
 public/app.te                | 12 +++----
 public/attributes            |  1 +
 public/bufferhubd.te         |  6 ++--
 public/domain.te             |  2 +-
 public/dumpstate.te          |  2 +-
 public/hal_omx.te            | 54 ++++++++++++++++++++++++++++
 public/mediacodec.te         | 70 ------------------------------------
 public/mediadrmserver.te     |  4 ++-
 public/mediaserver.te        |  5 ---
 vendor/bug_map               |  1 +
 vendor/hal_drm_default.te    |  2 +-
 vendor/hal_omx.te            |  1 -
 vendor/mediacodec.te         | 19 ++++++++++
 20 files changed, 99 insertions(+), 106 deletions(-)
 create mode 100644 public/hal_omx.te
 delete mode 100644 public/mediacodec.te
 delete mode 100644 vendor/hal_omx.te
 create mode 100644 vendor/mediacodec.te

diff --git a/private/app_neverallows.te b/private/app_neverallows.te
index 819408ac3..e71d565c0 100644
--- a/private/app_neverallows.te
+++ b/private/app_neverallows.te
@@ -253,6 +253,7 @@ full_treble_only(`
     -hal_graphics_allocator_server
     -hal_cas_server
     -hal_neuralnetworks_server
+    -hal_omx_server
     -binder_in_vendor_violators # TODO(b/35870313): Remove once all violations are gone
     -untrusted_app_visible_halserver
   }:binder { call transfer };
diff --git a/private/bug_map b/private/bug_map
index 5c551c83d..6f78f4a45 100644
--- a/private/bug_map
+++ b/private/bug_map
@@ -34,7 +34,6 @@ profman apk_data_file dir 77922323
 radio statsdw_socket sock_file 78456764
 statsd hal_health_default binder 77919007
 storaged storaged capability 77634061
-surfaceflinger mediacodec binder 77924251
 system_server crash_dump process 73128755
 system_server logd_socket sock_file 64734187
 system_server sdcardfs file 77856826
diff --git a/private/compat/26.0/26.0.cil b/private/compat/26.0/26.0.cil
index ee53d77cf..5696d9584 100644
--- a/private/compat/26.0/26.0.cil
+++ b/private/compat/26.0/26.0.cil
@@ -7,6 +7,8 @@
 (type asan_reboot_prop)
 (type log_device)
 (type mediacasserver_service)
+(type mediacodec)
+(type mediacodec_exec)
 (type qtaguid_proc)
 (type reboot_data_file)
 (type tracing_shell_writable)
diff --git a/private/compat/27.0/27.0.cil b/private/compat/27.0/27.0.cil
index 9f661b230..4bc428cbc 100644
--- a/private/compat/27.0/27.0.cil
+++ b/private/compat/27.0/27.0.cil
@@ -1,5 +1,7 @@
 ;; types removed from current policy
 (type qtaguid_proc)
+(type mediacodec)
+(type mediacodec_exec)
 (type reboot_data_file)
 (type rild)
 (type webview_zygote_socket)
diff --git a/private/incidentd.te b/private/incidentd.te
index 6b248f181..6fab126e5 100644
--- a/private/incidentd.te
+++ b/private/incidentd.te
@@ -84,9 +84,9 @@ allow incidentd {
   hal_bluetooth_server
   hal_camera_server
   hal_graphics_composer_server
+  hal_omx_server
   hal_sensors_server
   hal_vr_server
-  mediacodec # TODO(b/36375899): hal_omx_server
 }:process signal;
 
 # Allow incidentd to make binder calls to any binder service
diff --git a/private/mediaserver.te b/private/mediaserver.te
index a5fa9e10e..4c30bc027 100644
--- a/private/mediaserver.te
+++ b/private/mediaserver.te
@@ -4,8 +4,4 @@ init_daemon_domain(mediaserver)
 
 # allocate and use graphic buffers
 hal_client_domain(mediaserver, hal_graphics_allocator)
-
-# TODO(b/36375899): Remove this once OMX HAL is attributized and mediaserver is marked as a client
-# of OMX HAL.
-allow mediaserver hal_codec2_hwservice:hwservice_manager find;
-allow mediaserver hal_omx_hwservice:hwservice_manager find;
+hal_client_domain(mediaserver, hal_omx)
diff --git a/private/system_server.te b/private/system_server.te
index de6ad7b78..f74159e96 100644
--- a/private/system_server.te
+++ b/private/system_server.te
@@ -105,7 +105,7 @@ allow system_server appdomain:process { getsched setsched };
 allow system_server audioserver:process { getsched setsched };
 allow system_server hal_audio:process { getsched setsched };
 allow system_server hal_bluetooth:process { getsched setsched };
-allow system_server mediacodec:process { getsched setsched };
+allow system_server hal_omx_server:process { getsched setsched };
 allow system_server cameraserver:process { getsched setsched };
 allow system_server hal_camera:process { getsched setsched };
 allow system_server mediaserver:process { getsched setsched };
@@ -114,9 +114,9 @@ allow system_server bootanim:process { getsched setsched };
 # Allow system_server to write to /proc/<pid>/timerslack_ns
 allow system_server appdomain:file w_file_perms;
 allow system_server audioserver:file w_file_perms;
-allow system_server mediacodec:file w_file_perms;
 allow system_server cameraserver:file w_file_perms;
 allow system_server hal_audio_server:file w_file_perms;
+allow system_server hal_omx_server:file w_file_perms;
 
 # Read /proc/pid data for all domains. This is used by ProcessCpuTracker
 # within system_server to keep track of memory and CPU usage for
@@ -201,9 +201,7 @@ hal_client_domain(system_server, hal_light)
 hal_client_domain(system_server, hal_memtrack)
 hal_client_domain(system_server, hal_neuralnetworks)
 hal_client_domain(system_server, hal_oemlock)
-allow system_server hal_codec2_hwservice:hwservice_manager find;
-allow system_server hal_omx_hwservice:hwservice_manager find;
-allow system_server hidl_token_hwservice:hwservice_manager find;
+hal_client_domain(system_server, hal_omx)
 hal_client_domain(system_server, hal_power)
 hal_client_domain(system_server, hal_sensors)
 hal_client_domain(system_server, hal_tetheroffload)
@@ -220,8 +218,6 @@ hal_client_domain(system_server, hal_wifi_hostapd)
 hal_client_domain(system_server, hal_wifi_offload)
 hal_client_domain(system_server, hal_wifi_supplicant)
 
-binder_call(system_server, mediacodec)
-
 # Talk with graphics composer fences
 allow system_server hal_graphics_composer:fd use;
 
@@ -261,9 +257,9 @@ allow system_server {
   hal_bluetooth_server
   hal_camera_server
   hal_graphics_composer_server
+  hal_omx_server
   hal_sensors_server
   hal_vr_server
-  mediacodec # TODO(b/36375899): hal_omx_server
 }:process { signal };
 
 # Use sockets received over binder from various services.
diff --git a/public/app.te b/public/app.te
index 4ebf4803e..35c200859 100644
--- a/public/app.te
+++ b/public/app.te
@@ -219,12 +219,14 @@ binder_call(appdomain, appdomain)
 # Perform binder IPC to ephemeral apps.
 binder_call(appdomain, ephemeral_app)
 
-# TODO(b/36375899): Replace this with hal_client_domain once mediacodec is properly attributized
-# as OMX HAL
+# TODO(b/80317992): use hal_client_domain on individual domains or have tests
+#     that the required individual permissions are all granted
 hwbinder_use({ appdomain  -isolated_app })
 allow { appdomain -isolated_app } hal_codec2_hwservice:hwservice_manager find;
 allow { appdomain -isolated_app } hal_omx_hwservice:hwservice_manager find;
 allow { appdomain -isolated_app } hidl_token_hwservice:hwservice_manager find;
+get_prop({ appdomain -isolated_app }, hwservicemanager_prop);
+binder_call({ appdomain -isolated_app }, hal_omx_server)
 
 # Talk with graphics composer fences
 allow appdomain hal_graphics_composer:fd use;
@@ -307,12 +309,6 @@ allowxperm { appdomain -bluetooth } self:{ rawip_socket tcp_socket udp_socket }
 
 allow { appdomain -isolated_app } ion_device:chr_file r_file_perms;
 
-# TODO(b/36375899) replace with hal_client_domain for mediacodec (hal_omx)
-get_prop({ appdomain -isolated_app }, hwservicemanager_prop);
-
-# Allow app access to mediacodec (IOMX HAL)
-binder_call({ appdomain -isolated_app }, mediacodec)
-
 # Allow AAudio apps to use shared memory file descriptors from the HAL
 allow { appdomain -isolated_app } hal_audio:fd use;
 
diff --git a/public/attributes b/public/attributes
index f83394379..f64576624 100644
--- a/public/attributes
+++ b/public/attributes
@@ -277,6 +277,7 @@ hal_attribute(memtrack);
 hal_attribute(neuralnetworks);
 hal_attribute(nfc);
 hal_attribute(oemlock);
+hal_attribute(omx);
 hal_attribute(power);
 hal_attribute(secure_element);
 hal_attribute(sensors);
diff --git a/public/bufferhubd.te b/public/bufferhubd.te
index 274c2716b..580462c08 100644
--- a/public/bufferhubd.te
+++ b/public/bufferhubd.te
@@ -13,8 +13,8 @@ allow bufferhubd gpu_device:chr_file rw_file_perms;
 # Access /dev/ion
 allow bufferhubd ion_device:chr_file r_file_perms;
 
-# Receive sync fence FDs from mediacodec. Note that mediacodec never directly
+# Receive sync fence FDs from hal_omx_server. Note that hal_omx_server never directly
 # connects to bufferhubd via PDX. Instead, a VR app acts as a bridge between
-# those two: it talks to mediacodec via Binder and talks to bufferhubd via PDX.
+# those two: it talks to hal_omx_server via Binder and talks to bufferhubd via PDX.
 # Thus, there is no need to use pdx_client macro.
-allow bufferhubd mediacodec:fd use;
+allow bufferhubd hal_omx_server:fd use;
diff --git a/public/domain.te b/public/domain.te
index 0f977294a..2f93e42f4 100644
--- a/public/domain.te
+++ b/public/domain.te
@@ -1077,7 +1077,7 @@ neverallow {
   -system_server
 
   # Processes that can't exec crash_dump
-  -mediacodec
+  -hal_omx_server
   -mediaextractor
 } tombstoned_crash_socket:unix_stream_socket connectto;
 
diff --git a/public/dumpstate.te b/public/dumpstate.te
index 8379cf858..f6c750730 100644
--- a/public/dumpstate.te
+++ b/public/dumpstate.te
@@ -75,9 +75,9 @@ allow dumpstate {
   hal_camera_server
   hal_drm_server
   hal_graphics_composer_server
+  hal_omx_server
   hal_sensors_server
   hal_vr_server
-  mediacodec # TODO(b/36375899): hal_omx_server
 }:process signal;
 
 # Connect to tombstoned to intercept dumps.
diff --git a/public/hal_omx.te b/public/hal_omx.te
new file mode 100644
index 000000000..cf036900e
--- /dev/null
+++ b/public/hal_omx.te
@@ -0,0 +1,54 @@
+# applies all permissions to hal_omx NOT hal_omx_server
+# since OMX must always be in its own process.
+
+add_hwservice(hal_omx_server, hal_codec2_hwservice)
+add_hwservice(hal_omx_server, hal_omx_hwservice)
+
+# can route /dev/binder traffic to /dev/vndbinder
+vndbinder_use(hal_omx_server)
+
+binder_call(hal_omx_server, binderservicedomain)
+binder_call(hal_omx_server, { appdomain -isolated_app })
+
+# Allow hal_omx_server access to composer sync fences
+allow hal_omx_server hal_graphics_composer:fd use;
+
+allow hal_omx_server gpu_device:chr_file rw_file_perms;
+allow hal_omx_server video_device:chr_file rw_file_perms;
+allow hal_omx_server video_device:dir search;
+allow hal_omx_server ion_device:chr_file rw_file_perms;
+allow hal_omx_server hal_camera:fd use;
+
+crash_dump_fallback(hal_omx_server)
+
+# Recieve gralloc buffer FDs from bufferhubd. Note that hal_omx_server never
+# directly connects to bufferhubd via PDX. Instead, a VR app acts as a bridge
+# between those two: it talks to hal_omx_server via Binder and talks to bufferhubd
+# via PDX. Thus, there is no need to use pdx_client macro.
+allow hal_omx_server bufferhubd:fd use;
+
+allow hal_omx_client hal_codec2_hwservice:hwservice_manager find;
+allow hal_omx_client hal_omx_hwservice:hwservice_manager find;
+allow hal_omx_client hidl_token_hwservice:hwservice_manager find;
+
+binder_call(hal_omx_client, hal_omx_server)
+
+###
+### neverallow rules
+###
+
+# hal_omx_server should never execute any executable without a
+# domain transition
+neverallow hal_omx_server { file_type fs_type }:file execute_no_trans;
+
+# The goal of the mediaserver split is to place media processing code into
+# restrictive sandboxes with limited responsibilities and thus limited
+# permissions. Example: Audioserver is only responsible for controlling audio
+# hardware and processing audio content. Cameraserver does the same for camera
+# hardware/content. Etc.
+#
+# Media processing code is inherently risky and thus should have limited
+# permissions and be isolated from the rest of the system and network.
+# Lengthier explanation here:
+# https://android-developers.googleblog.com/2016/05/hardening-media-stack.html
+neverallow hal_omx_server domain:{ tcp_socket udp_socket rawip_socket } *;
diff --git a/public/mediacodec.te b/public/mediacodec.te
deleted file mode 100644
index e5b4a7d35..000000000
--- a/public/mediacodec.te
+++ /dev/null
@@ -1,70 +0,0 @@
-# mediacodec - audio and video codecs live here
-type mediacodec, domain;
-type mediacodec_exec, exec_type, vendor_file_type, file_type;
-
-typeattribute mediacodec mlstrustedsubject;
-
-# TODO(b/36375899) attributize this domain appropriately as hal_omx
-# and use macro hal_server_domain
-get_prop(mediacodec, hwservicemanager_prop)
-
-# can route /dev/binder traffic to /dev/vndbinder
-vndbinder_use(mediacodec)
-
-not_full_treble(`
-    # on legacy devices, continue to allow /dev/binder traffic
-    binder_use(mediacodec)
-    binder_service(mediacodec)
-    add_service(mediacodec, mediacodec_service)
-    allow mediacodec mediametrics_service:service_manager find;
-    allow mediacodec surfaceflinger_service:service_manager find;
-')
-binder_call(mediacodec, binderservicedomain)
-binder_call(mediacodec, appdomain)
-
-# Allow mediacodec access to composer sync fences
-allow mediacodec hal_graphics_composer:fd use;
-
-allow mediacodec gpu_device:chr_file rw_file_perms;
-allow mediacodec video_device:chr_file rw_file_perms;
-allow mediacodec video_device:dir search;
-allow mediacodec ion_device:chr_file rw_file_perms;
-allow mediacodec hal_camera:fd use;
-
-crash_dump_fallback(mediacodec)
-
-add_hwservice(mediacodec, hal_codec2_hwservice)
-add_hwservice(mediacodec, hal_omx_hwservice)
-
-hal_client_domain(mediacodec, hal_allocator)
-
-hal_client_domain(mediacodec, hal_cas)
-
-# allocate and use graphic buffers
-hal_client_domain(mediacodec, hal_graphics_allocator)
-
-# Recieve gralloc buffer FDs from bufferhubd. Note that mediacodec never
-# directly connects to bufferhubd via PDX. Instead, a VR app acts as a bridge
-# between those two: it talks to mediacodec via Binder and talks to bufferhubd
-# via PDX. Thus, there is no need to use pdx_client macro.
-allow mediacodec bufferhubd:fd use;
-
-###
-### neverallow rules
-###
-
-# mediacodec should never execute any executable without a
-# domain transition
-neverallow mediacodec { file_type fs_type }:file execute_no_trans;
-
-# The goal of the mediaserver split is to place media processing code into
-# restrictive sandboxes with limited responsibilities and thus limited
-# permissions. Example: Audioserver is only responsible for controlling audio
-# hardware and processing audio content. Cameraserver does the same for camera
-# hardware/content. Etc.
-#
-# Media processing code is inherently risky and thus should have limited
-# permissions and be isolated from the rest of the system and network.
-# Lengthier explanation here:
-# https://android-developers.googleblog.com/2016/05/hardening-media-stack.html
-neverallow mediacodec domain:{ tcp_socket udp_socket rawip_socket } *;
diff --git a/public/mediadrmserver.te b/public/mediadrmserver.te
index 123cb29a5..059be7be9 100644
--- a/public/mediadrmserver.te
+++ b/public/mediadrmserver.te
@@ -18,7 +18,9 @@ allow mediadrmserver processinfo_service:service_manager find;
 allow mediadrmserver surfaceflinger_service:service_manager find;
 allow mediadrmserver system_file:dir r_dir_perms;
 
-binder_call(mediadrmserver, mediacodec)
+# TODO(b/80317992): remove
+binder_call(mediadrmserver, hal_omx_server)
+
 ###
 ### neverallow rules
 ###
diff --git a/public/mediaserver.te b/public/mediaserver.te
index 4032a7623..9e00fbbf8 100644
--- a/public/mediaserver.te
+++ b/public/mediaserver.te
@@ -4,9 +4,6 @@ type mediaserver_exec, exec_type, file_type;
 
 typeattribute mediaserver mlstrustedsubject;
 
-# TODO(b/36375899): replace with hal_client_domain macro on hal_omx
-typeattribute mediaserver halclientdomain;
-
 net_domain(mediaserver)
 
 r_dir_file(mediaserver, sdcard_type)
@@ -135,8 +132,6 @@ allow mediaserver system_server:fd use;
 
 hal_client_domain(mediaserver, hal_allocator)
 
-binder_call(mediaserver, mediacodec)
-
 ###
 ### neverallow rules
 ###
diff --git a/vendor/bug_map b/vendor/bug_map
index e69de29bb..ef56ca608 100644
--- a/vendor/bug_map
+++ b/vendor/bug_map
@@ -0,0 +1 @@
+surfaceflinger mediacodec binder 77924251
diff --git a/vendor/hal_drm_default.te b/vendor/hal_drm_default.te
index 0dac07596..5bcbe9ac2 100644
--- a/vendor/hal_drm_default.te
+++ b/vendor/hal_drm_default.te
@@ -4,7 +4,7 @@ hal_server_domain(hal_drm_default, hal_drm)
 type hal_drm_default_exec, exec_type, vendor_file_type, file_type;
 init_daemon_domain(hal_drm_default)
 
-allow hal_drm_default mediacodec:fd use;
+allow hal_drm_default hal_omx_server:fd use;
 allow hal_drm_default { appdomain -isolated_app }:fd use;
 
 allow hal_drm_default hal_allocator_server:fd use;
diff --git a/vendor/hal_omx.te b/vendor/hal_omx.te
deleted file mode 100644
index fdb4aca59..000000000
--- a/vendor/hal_omx.te
+++ /dev/null
@@ -1 +0,0 @@
-init_daemon_domain(mediacodec)
diff --git a/vendor/mediacodec.te b/vendor/mediacodec.te
new file mode 100644
index 000000000..a235145cc
--- /dev/null
+++ b/vendor/mediacodec.te
@@ -0,0 +1,19 @@
+type mediacodec, domain, mlstrustedsubject;
+type mediacodec_exec, exec_type, vendor_file_type, file_type;
+
+init_daemon_domain(mediacodec)
+
+not_full_treble(`
+    # on legacy devices, continue to allow /dev/binder traffic
+    binder_use(mediacodec)
+    binder_service(mediacodec)
+    add_service(mediacodec, mediacodec_service)
+    allow mediacodec mediametrics_service:service_manager find;
+    allow mediacodec surfaceflinger_service:service_manager find;
+')
+
+hal_server_domain(mediacodec, hal_omx)
+
+hal_client_domain(mediacodec, hal_allocator)
+hal_client_domain(mediacodec, hal_cas)
+hal_client_domain(mediacodec, hal_graphics_allocator)
-- 
GitLab