[PATCH] sched/uclamp: Validate min/max pair under task_rq_lock()

[Hui Su]

sched_setattr() may update UCLAMP_MIN and UCLAMP_MAX independently. If two
calls validate against the same old pair before taking task_rq_lock(), both
can succeed and leave uclamp_req with min > max.

Check the task-dependent pair after task_rq_lock(), where updates are
serialized. Reject an inverted pair supplied together before
sched_uclamp_enable(). The uclamp static key remains enabled once set.
Keep scalar range checks and sched_uclamp_enable() before scheduler locks
because enabling uclamp may block.

Fixes: a509a7cd7974 ("sched/uclamp: Extend sched_setattr() to support utilization clamping")
Signed-off-by: Hui Su <sh_def@xxxxxxx>
---
Testing:
- PASS: x86_64 QEMU/KVM reproduced min=900/max=800 with both setters
  succeeding at iteration 20 before the fix; the candidate completed 100000
  rounds with exactly one setter returning `-EINVAL` per round, `bad_pair=0`,
  and `unexpected_errno=0`.
- PASS: x86_64 `make -j12 bzImage` with `CONFIG_UCLAMP_TASK=y`,
  `CONFIG_PROVE_LOCKING=y`, and `CONFIG_CPU_FREQ_GOV_SCHEDUTIL=y`.
---
 kernel/sched/syscalls.c | 51 ++++++++++++++++++++++++++++++++---------
 1 file changed, 40 insertions(+), 11 deletions(-)

diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index b215b0ead9a6..289559b0c571 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -315,27 +315,47 @@ static inline void __setscheduler_dl_pi(int newprio, int policy,
 
 #ifdef CONFIG_UCLAMP_TASK
 
-static int uclamp_validate(struct task_struct *p,
-			   const struct sched_attr *attr)
+static int uclamp_validate_pair(struct task_struct *p,
+			       const struct sched_attr *attr)
 {
-	int util_min = p->uclamp_req[UCLAMP_MIN].value;
-	int util_max = p->uclamp_req[UCLAMP_MAX].value;
+	int util_min;
+	int util_max;
 
-	if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) {
+	lockdep_assert_held(&p->pi_lock);
+
+	util_min = p->uclamp_req[UCLAMP_MIN].value;
+	util_max = p->uclamp_req[UCLAMP_MAX].value;
+
+	if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN)
 		util_min = attr->sched_util_min;
+	if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MAX)
+		util_max = attr->sched_util_max;
+
+	if (util_min != -1 && util_max != -1 && util_min > util_max)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int uclamp_validate(const struct sched_attr *attr)
+{
+	int util_min = attr->sched_util_min;
+	int util_max = attr->sched_util_max;
 
+	if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) {
 		if (util_min + 1 > SCHED_CAPACITY_SCALE + 1)
 			return -EINVAL;
 	}
 
 	if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MAX) {
-		util_max = attr->sched_util_max;
-
 		if (util_max + 1 > SCHED_CAPACITY_SCALE + 1)
 			return -EINVAL;
 	}
 
-	if (util_min != -1 && util_max != -1 && util_min > util_max)
+	/* Both clamps are request-local, so validate them before enabling uclamp. */
+	if ((attr->sched_flags & SCHED_FLAG_UTIL_CLAMP) ==
+	    SCHED_FLAG_UTIL_CLAMP &&
+	    util_min != -1 && util_max != -1 && util_min > util_max)
 		return -EINVAL;
 
 	/*
@@ -418,11 +438,15 @@ static void __setscheduler_uclamp(struct task_struct *p,
 
 #else /* !CONFIG_UCLAMP_TASK: */
 
-static inline int uclamp_validate(struct task_struct *p,
-				  const struct sched_attr *attr)
+static inline int uclamp_validate(const struct sched_attr *attr)
 {
 	return -EOPNOTSUPP;
 }
+static inline int uclamp_validate_pair(struct task_struct *p,
+				       const struct sched_attr *attr)
+{
+	return 0;
+}
 static void __setscheduler_uclamp(struct task_struct *p,
 				  const struct sched_attr *attr) { }
 #endif /* !CONFIG_UCLAMP_TASK */
@@ -547,7 +571,7 @@ int __sched_setscheduler(struct task_struct *p,
 
 	/* Update task specific "requested" clamps */
 	if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP) {
-		retval = uclamp_validate(p, attr);
+		retval = uclamp_validate(attr);
 		if (retval)
 			return retval;
 	}
@@ -570,6 +594,11 @@ int __sched_setscheduler(struct task_struct *p,
 	 */
 	rq = task_rq_lock(p, &rf);
 	update_rq_clock(rq);
+	if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP) {
+		retval = uclamp_validate_pair(p, attr);
+		if (retval)
+			goto unlock;
+	}
 
 	/*
 	 * Changing the policy of the stop threads its a very bad idea:

base-commit: 045958acaa75205b31578cb22c4345e28dd2e4bf