Skip to content

Commit d198bdf

Browse files
Tashar02backslashxx
authored andcommitted
kernel: selinux: rules: Fix illegal RCU lock usage in apply_kernelsu_rules() (tiann#2646)
When kernel is compiled with CONFIG_DEBUG_ATOMIC_SLEEP enabled, it prints the following splat in dmesg during post boot: [ 6.739169] init: Opening SELinux policy [ 6.751520] init: Loading SELinux policy [ 6.894684] SELinux: policy capability network_peer_controls=1 [ 6.894688] SELinux: policy capability open_perms=1 [ 6.894690] SELinux: policy capability extended_socket_class=1 [ 6.894691] SELinux: policy capability always_check_network=0 [ 6.894693] SELinux: policy capability cgroup_seclabel=0 [ 6.894695] SELinux: policy capability nnp_nosuid_transition=1 [ 7.214323] selinux: SELinux: Loaded file context from: [ 7.214332] selinux: /system/etc/selinux/plat_file_contexts [ 7.214339] selinux: /system_ext/etc/selinux/system_ext_file_contexts [ 7.214345] selinux: /product/etc/selinux/product_file_contexts [ 7.214350] selinux: /vendor/etc/selinux/vendor_file_contexts [ 7.214356] selinux: /odm/etc/selinux/odm_file_contexts [ 7.216398] KernelSU: /system/bin/init argc: 2 [ 7.216401] KernelSU: /system/bin/init first arg: second_stage [ 7.216403] KernelSU: /system/bin/init second_stage executed [ 7.216506] BUG: sleeping function called from invalid context at security/selinux/ss/hashtab.c:47 [ 7.216512] in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 1, name: init [ 7.216516] preempt_count: 0, expected: 0 [ 7.216518] RCU nest depth: 1, expected: 0 [ 7.216524] CPU: 6 PID: 1 Comm: init Not tainted 5.4.289-Scarlet-v2.0-beta3 #1 [ 7.216526] Hardware name: redwood based Qualcomm Technologies, Inc. SM7325 (DT) [ 7.216528] Call trace: [ 7.216536] dump_backtrace+0x0/0x210 [ 7.216539] show_stack+0x14/0x20 [ 7.216544] dump_stack+0x9c/0xec [ 7.216548] __might_resched+0x1f0/0x210 [ 7.216552] hashtab_insert+0x38/0x230 [ 7.216557] add_type+0xd4/0x2e0 [ 7.216559] ksu_type+0x24/0x60 [ 7.216562] apply_kernelsu_rules+0xa8/0x650 [ 7.216565] ksu_handle_execveat_ksud+0x2a8/0x460 [ 7.216568] ksu_handle_execveat+0x2c/0x60 [ 7.216571] __arm64_sys_execve+0xe8/0xf0 [ 7.216574] el0_svc_common+0xf4/0x1a0 [ 7.216577] do_el0_svc+0x2c/0x40 [ 7.216579] el0_sync_handler+0x18c/0x200 [ 7.216582] el0_sync+0x140/0x180 This is because apply_kernelsu_rules() uses rcu_read_lock() to protect SELinux policy modifications. However, cond_resched() from hashtab_insert() at security/selinux/ss/hashtab.c is internally called and it sleeps which is illegal under an RCU read-side critical section. While replacing it with a spinlock would suppress the warning, this is fundamentally incorrect because sleeping is illegal while holding a spinlock and spinlock would turn off preemption which isn't an ideal solution since it intentionally turns off rescheduling, and can lead to deadlocks. Instead, replace the RCU lock with a mutex lock. Mutex lock allows sleeping when necessary, which is appropriate here because apply_kernelsu_rules() runs in process context, not in atomic or interrupt context. As apply_kernelsu_rules() is invoked only once during post boot (SYSTEM_RUNNING), the mutex lock does not introduce any major runtime performance regression and provides correct synchronization. Fixes: tiann#2637 Signed-off-by: Tashfin Shakeer Rhythm <[email protected]>
1 parent fd0ffb9 commit d198bdf

File tree

1 file changed

+11
-6
lines changed

1 file changed

+11
-6
lines changed

kernel/selinux/rules.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,19 @@ static struct policydb *get_policydb(void)
3636
return db;
3737
}
3838

39+
static DEFINE_MUTEX(ksu_rules);
40+
3941
void apply_kernelsu_rules()
4042
{
43+
struct policydb *db;
44+
4145
if (!getenforce()) {
4246
pr_info("SELinux permissive or disabled, apply rules!\n");
4347
}
4448

45-
rcu_read_lock();
46-
struct policydb *db = get_policydb();
49+
mutex_lock(&ksu_rules);
50+
51+
db = get_policydb();
4752

4853
ksu_permissive(db, KERNEL_SU_DOMAIN);
4954
ksu_typeattribute(db, KERNEL_SU_DOMAIN, "mlstrustedsubject");
@@ -130,11 +135,11 @@ void apply_kernelsu_rules()
130135
// Allow all binder transactions
131136
ksu_allow(db, ALL, KERNEL_SU_DOMAIN, "binder", ALL);
132137

133-
// Allow system server kill su process
134-
ksu_allow(db, "system_server", KERNEL_SU_DOMAIN, "process", "getpgid");
135-
ksu_allow(db, "system_server", KERNEL_SU_DOMAIN, "process", "sigkill");
138+
// Allow system server kill su process
139+
ksu_allow(db, "system_server", KERNEL_SU_DOMAIN, "process", "getpgid");
140+
ksu_allow(db, "system_server", KERNEL_SU_DOMAIN, "process", "sigkill");
136141

137-
rcu_read_unlock();
142+
mutex_unlock(&ksu_rules);
138143
}
139144

140145
#define MAX_SEPOL_LEN 128

0 commit comments

Comments
 (0)