From 7cda44f49f8b128f6a4673174220b4825024f654 Mon Sep 17 00:00:00 2001
From: Alex Klyubin <klyubin@google.com>
Date: Tue, 21 Mar 2017 14:28:53 -0700
Subject: [PATCH] Mark all clients of Allocator HAL

This change associates all domains which are clients of Allocator HAL
with hal_allocator_client and the, required for all HAL client
domains, halclientdomain.

This enables this commit to remove the now unnecessary hwallocator_use
macro because its binder_call(..., hal_allocator_server) is covered by
binder_call(hal_allocator_client, hal_allocator_server) added in this
commit.

Unfortunately apps, except isolated app, are clients of Allocator HAL
as well. This makes it hard to use the hal_client_domain(...,
hal_allocator) macro because it translates into "typeattribute" which
currently does not support being provided with a set of types, such as
{ appdomain -isolated_app }. As a workaround, hopefully until
typeattribute is improved, this commit expresses the necessary
association operation in CIL. private/technical_debt.cil introduced by
this commit is appended into the platform policy CIL file, thus
ensuring that the hack has effect on the final monolithic policy.

P. S. This change also removes Allocator HAL access from isolated_app.
Isolated app shouldn't have access to this HAL anyway.

Test: Google Play Music plays back radios
Test: Google Camera records video with sound and that video is then
      successfully played back with sound
Test: YouTube app plays back clips with sound
Test: YouTube in Chrome plays back clips with sound
Bug: 34170079
Change-Id: Id00bba6fde83e7cf04fb58bc1c353c2f66333f92
---
 Android.mk                  | 12 +++++++++++-
 private/app.te              |  1 -
 private/audioserver.te      |  3 +--
 private/system_server.te    |  2 +-
 private/technical_debt.cil  | 13 +++++++++++++
 public/hal_allocator.te     |  2 ++
 public/hal_audio.te         |  6 ------
 public/mediacodec.te        |  5 +----
 public/mediaserver.te       |  4 +---
 public/te_macros            |  8 --------
 vendor/hal_audio_default.te |  2 ++
 11 files changed, 32 insertions(+), 26 deletions(-)
 create mode 100644 private/technical_debt.cil
 create mode 100644 public/hal_allocator.te

diff --git a/Android.mk b/Android.mk
index 976d61b5a..44ba23d24 100644
--- a/Android.mk
+++ b/Android.mk
@@ -124,6 +124,11 @@ sepolicy_build_files := security_classes \
                         genfs_contexts \
                         port_contexts
 
+# CIL files which contain workarounds for current limitation of human-readable
+# module policy language. These files are appended to the CIL files produced
+# from module language files.
+sepolicy_build_cil_workaround_files := technical_debt.cil
+
 my_target_arch := $(TARGET_ARCH)
 ifneq (,$(filter mips mips64,$(TARGET_ARCH)))
   my_target_arch := mips
@@ -250,9 +255,13 @@ $(PLAT_PUBLIC_POLICY) $(PLAT_PRIVATE_POLICY))
 	$(hide) sed '/dontaudit/d' $@ > $@.dontaudit
 
 plat_policy_nvr := $(intermediates)/plat_policy_nvr.cil
-$(plat_policy_nvr): $(plat_policy.conf) $(HOST_OUT_EXECUTABLES)/checkpolicy
+$(plat_policy_nvr): PRIVATE_ADDITIONAL_CIL_FILES := \
+  $(call build_policy, $(sepolicy_build_cil_workaround_files), $(PLAT_PRIVATE_POLICY))
+$(plat_policy_nvr): $(plat_policy.conf) $(HOST_OUT_EXECUTABLES)/checkpolicy \
+  $(call build_policy, $(sepolicy_build_cil_workaround_files), $(PLAT_PRIVATE_POLICY))
 	@mkdir -p $(dir $@)
 	$(hide) $(HOST_OUT_EXECUTABLES)/checkpolicy -M -C -c $(POLICYVERS) -o $@ $<
+	$(hide) cat $(PRIVATE_ADDITIONAL_CIL_FILES) >> $@
 
 $(LOCAL_BUILT_MODULE): PRIVATE_CIL_FILES := $(plat_policy_nvr)
 $(LOCAL_BUILT_MODULE): $(HOST_OUT_EXECUTABLES)/secilc $(plat_policy_nvr)
@@ -1114,6 +1123,7 @@ plat_policy_nvr :=
 plat_pub_policy.cil :=
 reqd_policy_mask.cil :=
 sepolicy_build_files :=
+sepolicy_build_cil_workaround_files :=
 with_asan :=
 
 include $(call all-makefiles-under,$(LOCAL_PATH))
diff --git a/private/app.te b/private/app.te
index c5943ddb6..81de403aa 100644
--- a/private/app.te
+++ b/private/app.te
@@ -155,7 +155,6 @@ binder_call(appdomain, ephemeral_app)
 # hidl access for mediacodec
 # TODO(b/34454312): only allow getting and talking to mediacodec service
 hwbinder_use(appdomain)
-hwallocator_use(appdomain)
 
 # Already connected, unnamed sockets being passed over some other IPC
 # hence no sock_file or connectto permission. This appears to be how
diff --git a/private/audioserver.te b/private/audioserver.te
index a6253f244..61ccefcd4 100644
--- a/private/audioserver.te
+++ b/private/audioserver.te
@@ -12,10 +12,9 @@ binder_call(audioserver, binderservicedomain)
 binder_call(audioserver, appdomain)
 binder_service(audioserver)
 
+hal_client_domain(audioserver, hal_allocator)
 hal_client_domain(audioserver, hal_audio)
 
-allow audioserver system_file:dir r_dir_perms;
-
 userdebug_or_eng(`
   # used for TEE sink - pcm capture for debug.
   allow audioserver media_data_file:dir create_dir_perms;
diff --git a/private/system_server.te b/private/system_server.te
index da1c62550..af1e91818 100644
--- a/private/system_server.te
+++ b/private/system_server.te
@@ -168,7 +168,7 @@ binder_service(system_server)
 
 # Perform HwBinder IPC.
 hwbinder_use(system_server)
-hwallocator_use(system_server)
+hal_client_domain(system_server, hal_allocator)
 binder_call(system_server, hal_contexthub)
 hal_client_domain(system_server, hal_contexthub)
 hal_client_domain(system_server, hal_fingerprint)
diff --git a/private/technical_debt.cil b/private/technical_debt.cil
new file mode 100644
index 000000000..2d9ec8bca
--- /dev/null
+++ b/private/technical_debt.cil
@@ -0,0 +1,13 @@
+; THIS IS A WORKAROUND for the current limitations of the module policy language
+; This should be used sparingly until we figure out a saner way to achieve the
+; stuff below, for example, by improving typeattribute statement of module
+; language.
+;
+; NOTE: This file has no effect on recovery policy.
+
+; Apps, except isolated apps, are clients of Allocator HAL
+; Unfortunately, we can't currently express this in module policy language:
+;     typeattribute { appdomain -isolated_app } hal_allocator_client;
+;     typeattribute hal_allocator_client halclientdomain;
+(typeattributeset hal_allocator_client ((and (appdomain) ((not (isolated_app))))))
+(typeattributeset halclientdomain (hal_allocator_client))
diff --git a/public/hal_allocator.te b/public/hal_allocator.te
new file mode 100644
index 000000000..b444593ba
--- /dev/null
+++ b/public/hal_allocator.te
@@ -0,0 +1,2 @@
+# HwBinder IPC from client to server
+binder_call(hal_allocator_client, hal_allocator_server)
diff --git a/public/hal_audio.te b/public/hal_audio.te
index a195c9363..3531944a0 100644
--- a/public/hal_audio.te
+++ b/public/hal_audio.te
@@ -2,14 +2,8 @@
 binder_call(hal_audio_client, hal_audio_server)
 binder_call(hal_audio_server, hal_audio_client)
 
-# Both client and the server need to use hwallocator
-hwallocator_use(hal_audio_client)
-hwallocator_use(hal_audio_server)
-
 allow hal_audio ion_device:chr_file r_file_perms;
 
-allow hal_audio system_file:dir { open read };
-
 userdebug_or_eng(`
   # used for pcm capture for debug.
   allow hal_audio audiohal_data_file:dir create_dir_perms;
diff --git a/public/mediacodec.te b/public/mediacodec.te
index f0e7e9a3d..469c8bab5 100644
--- a/public/mediacodec.te
+++ b/public/mediacodec.te
@@ -25,10 +25,7 @@ allow mediacodec hal_camera:fd use;
 
 crash_dump_fallback(mediacodec)
 
-# hidl access
-hwbinder_use(mediacodec)
-hwallocator_use(mediacodec)
-allow mediacodec system_file:dir { open read };
+hal_client_domain(mediacodec, hal_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
diff --git a/public/mediaserver.te b/public/mediaserver.te
index 46140b3ac..93f154805 100644
--- a/public/mediaserver.te
+++ b/public/mediaserver.te
@@ -136,9 +136,7 @@ allow mediaserver hal_camera:fd use;
 
 allow mediaserver system_server:fd use;
 
-# hidl access
-hwbinder_use(mediaserver)
-hwallocator_use(mediaserver)
+hal_client_domain(mediaserver, hal_allocator)
 
 ###
 ### neverallow rules
diff --git a/public/te_macros b/public/te_macros
index 52f2e1b61..57a038a23 100644
--- a/public/te_macros
+++ b/public/te_macros
@@ -327,14 +327,6 @@ define(`binder_service', `
 typeattribute $1 binderservicedomain;
 ')
 
-#####################################
-# hwallocator_use(domain)
-# Allow a domain to use Hidl shared memory
-define(`hwallocator_use', `
-# Call into the allocator hal
-binder_call($1, hal_allocator_server);
-')
-
 #####################################
 # wakelock_use(domain)
 # Allow domain to manage wake locks
diff --git a/vendor/hal_audio_default.te b/vendor/hal_audio_default.te
index 4811f4da3..04ef7aa4b 100644
--- a/vendor/hal_audio_default.te
+++ b/vendor/hal_audio_default.te
@@ -3,3 +3,5 @@ hal_server_domain(hal_audio_default, hal_audio)
 
 type hal_audio_default_exec, exec_type, file_type;
 init_daemon_domain(hal_audio_default)
+
+hal_client_domain(hal_audio_default, hal_allocator)
-- 
GitLab