From 8e8c109350f4cd636a7bc9dee154e8a295538681 Mon Sep 17 00:00:00 2001 From: Alan Stokes Date: Fri, 6 Apr 2018 11:59:38 +0100 Subject: [PATCH] Installd doesn't need to create cgroup files. cgroupfs doesn't allow files to be created, so this can't be needed. Also remove redundant neverallow and dontaudit rules. These are now more broadly handled by domain.te. Bug: 74182216 Test: Denials remain silenced. Change-Id: If7eb0e59f567695d987272a2fd36dbc251516e9f --- private/init.te | 10 ---------- private/zygote.te | 5 ----- public/domain.te | 14 +++++++------- public/init.te | 5 ----- public/installd.te | 1 - 5 files changed, 7 insertions(+), 28 deletions(-) diff --git a/private/init.te b/private/init.te index 50b1c94c0..e9959d3d2 100644 --- a/private/init.te +++ b/private/init.te @@ -20,13 +20,3 @@ domain_trans(init, { rootfs toolbox_exec }, modprobe) userdebug_or_eng(` domain_auto_trans(init, logcat_exec, logpersist) ') - -# Creating files on sysfs is impossible so this isn't a threat -# Sometimes we have to write to non-existent files to avoid conditional -# init behavior. See b/35303861 for an example. -dontaudit init sysfs:dir write; - -# Suppress false positives when using O_CREAT -# to open a file that already exists. -# There's a neverallow rule for this in domain.te -dontaudit init cgroup:file create; diff --git a/private/zygote.te b/private/zygote.te index ab707f155..4ea401dce 100644 --- a/private/zygote.te +++ b/private/zygote.te @@ -134,8 +134,3 @@ neverallow zygote { # Do not allow access to Bluetooth-related system properties and files neverallow zygote bluetooth_prop:file create_file_perms; - -# Suppress false positives when using O_CREAT -# to open a file that already exists. -# There's a neverallow rule for this in domain.te -dontaudit zygote cgroup:file create; diff --git a/public/domain.te b/public/domain.te index 8ff0cbab6..8cae3ca56 100644 --- a/public/domain.te +++ b/public/domain.te @@ -1328,23 +1328,23 @@ neverallow { } self:capability dac_override; neverallow { domain -traced_probes } self:capability dac_read_search; -# If an already existing file is opened with O_CREATE, the kernel might generate +# If an already existing file is opened with O_CREAT, the kernel might generate # a false report of a create denial. Silence these denials and make sure that # inappropriate permissions are not granted. + +# These filesystems don't allow files or directories to be created, so the permission +# to do so should never be granted. neverallow domain { proc_type sysfs_type }:dir { add_name create link remove_name rename reparent rmdir write }; -# cgroupfs directories can be created, but not files within them -# TODO(b/74182216): Remove the installd allow when we're sure it's not used -neverallow { - domain - -installd -} cgroup:file create; +# cgroupfs directories can be created, but not files within them. +neverallow domain cgroup:file create; dontaudit domain proc_type:dir write; dontaudit domain sysfs_type:dir write; +dontaudit domain cgroup:file create; # These are only needed in permissive mode - in enforcing mode the # directory write check fails and so these are never attempted. diff --git a/public/init.te b/public/init.te index 254d8e080..c34e02842 100644 --- a/public/init.te +++ b/public/init.te @@ -326,11 +326,6 @@ allow init { # Allow init to write to vibrator/trigger allow init sysfs_vibrator:file w_file_perms; -# Creating files on sysfs is impossible so this isn't a threat. -# We may write to a non-existent file to avoid conditional -# init behavior. -dontaudit init sysfs_vibrator:dir write; - # init chmod/chown access to /sys files. allow init { sysfs_android_usb diff --git a/public/installd.te b/public/installd.te index fad4562ad..6aba962dd 100644 --- a/public/installd.te +++ b/public/installd.te @@ -19,7 +19,6 @@ allow installd apk_tmp_file:dir { relabelfrom create_dir_perms }; allow installd oemfs:dir r_dir_perms; allow installd oemfs:file r_file_perms; allow installd cgroup:dir create_dir_perms; -allow installd cgroup:{ file lnk_file } create_file_perms; allow installd mnt_expand_file:dir { search getattr }; # Check validity of SELinux context before use. selinux_check_context(installd)