From b57e3c1d9943249f0e7383cb54dd29ed8c416a78 Mon Sep 17 00:00:00 2001 From: Qais Yousef Date: Fri, 25 Nov 2022 15:23:17 +0000 Subject: [PATCH] ANDROID: sched/uclamp: Don't enable uclamp_is_used static key by in-kernel requests We do have now in-kernel users of uclamp to implement inheritance. The static_branch_enable() path unconditionally holds the cpus_read_lock() which might_sleep(). The path in binder that implements inheritance happens from in_atomic() context which leads to a splat like this one: [ 147.529960] BUG: sleeping function called from invalid context at include/linux/percpu-rwsem.h:56 [ 147.530196] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 2586, name: RenderThread [ 147.530410] INFO: lockdep is turned off. [ 147.530518] Preemption disabled at: [ 147.530521] [] binder_proc_transaction+0x78/0x41c [ 147.530793] CPU: 8 PID: 2586 Comm: RenderThread Tainted: G S W O 5.15.76-android14-5-00086-gc01afe5d262f #1 [ 147.531214] Call trace: [ 147.531288] dump_backtrace+0xe8/0x134 [ 147.531444] show_stack+0x1c/0x4c [ 147.531598] dump_stack_lvl+0x74/0x94 [ 147.531766] dump_stack+0x14/0x3c [ 147.531920] ___might_sleep+0x210/0x230 [ 147.532094] __might_sleep+0x54/0x84 [ 147.532259] cpus_read_lock+0x2c/0x160 [ 147.532429] static_key_enable+0x1c/0x34 [ 147.532608] __sched_setscheduler+0x2a8/0x99c [ 147.532802] sched_setattr_nocheck+0x1c/0x24 [ 147.532994] binder_do_set_priority+0x31c/0x4a4 [ 147.533195] binder_transaction_priority+0x200/0x3f4 [ 147.533413] binder_proc_transaction+0x220/0x41c [ 147.533618] binder_transaction+0x1df0/0x234c [ 147.533812] binder_thread_write+0xd84/0x2398 [ 147.534007] binder_ioctl_write_read+0x19c/0xb28 [ 147.534212] binder_ioctl+0x344/0x1a3c [ 147.534382] __arm64_sys_ioctl+0x94/0xc8 [ 147.534561] invoke_syscall+0x44/0xf8 [ 147.534729] el0_svc_common+0xc8/0x10c [ 147.534900] do_el0_svc+0x20/0x28 [ 147.535053] el0_svc+0x58/0xe0 [ 147.535198] el0t_64_sync_handler+0x7c/0xe4 [ 147.535386] el0t_64_sync+0x188/0x18c Prevent enabling the lock for !user initiated sched_setattr() operations. Generally we don't expect in-kernel uclamp users. Bug: 259145692 Signed-off-by: Qais Yousef Change-Id: Iac5be139b5ffd39f5e1c0431ce253133d81b98cf --- kernel/sched/core.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 8c1911fc5664..773175f6df5d 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1880,7 +1880,7 @@ done: #endif static int uclamp_validate(struct task_struct *p, - const struct sched_attr *attr) + const struct sched_attr *attr, bool user) { int util_min = p->uclamp_req[UCLAMP_MIN].value; int util_max = p->uclamp_req[UCLAMP_MAX].value; @@ -1905,11 +1905,19 @@ static int uclamp_validate(struct task_struct *p, /* * We have valid uclamp attributes; make sure uclamp is enabled. * - * We need to do that here, because enabling static branches is a - * blocking operation which obviously cannot be done while holding + * We need to do that here, because enabling static branches is + * a blocking operation which obviously cannot be done while holding * scheduler locks. + * + * We only enable the static key if this was initiated by user space + * request. There should be no in-kernel users of uclamp except to + * implement things like inheritance like in binder. These in-kernel + * callers can rightfully be called be sometimes in_atomic() context + * which is invalid context to enable the key in. The enabling path + * unconditionally holds the cpus_read_lock() which might_sleep(). */ - static_branch_enable(&sched_uclamp_used); + if (user) + static_branch_enable(&sched_uclamp_used); return 0; } @@ -2050,7 +2058,7 @@ static void __init init_uclamp(void) static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p) { } static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p) { } static inline int uclamp_validate(struct task_struct *p, - const struct sched_attr *attr) + const struct sched_attr *attr, bool user) { return -EOPNOTSUPP; } @@ -7649,7 +7657,7 @@ recheck: /* Update task specific "requested" clamps */ if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP) { - retval = uclamp_validate(p, attr); + retval = uclamp_validate(p, attr, user); if (retval) return retval; }