Re: [PATCH v4 04/26] sched: Let a task boosted by a deadline waiter change its policy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] sched: dangling pi_se in __sched_setscheduler() when pi is
  false
- [Medium] sched: unconditional ENQUEUE_REPLENISH in
  __setscheduler_dl_pi()
--

commit 8fd411e98dd72e517f278200f2a112aeff758f77
Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

sched: Let a task boosted by a deadline waiter change its policy

This commit changes __sched_setscheduler() to test task_has_dl_policy(p)
rather than dl_task(p) when deciding whether to run sched_dl_overflow(),
allowing a task boosted by a deadline waiter to change its policy.

> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
> index b8c036d03d6f5..bdffe5452dc8c 100644
> --- a/kernel/sched/syscalls.c
> +++ b/kernel/sched/syscalls.c
> @@ -689,7 +689,9 @@ int __sched_setscheduler(struct task_struct *p,
>  	 * If setscheduling to SCHED_DEADLINE (or changing the parameters
>  	 * of a SCHED_DEADLINE task) we need to check if enough bandwidth
>  	 * is available. Last of the checks: nothing gives back the
> -	 * bandwidth it reserves.
> +	 * bandwidth it reserves. A task boosted into the class by priority
> +	 * inheritance has nothing to admit nor to leave: the policy
> +	 * decides, not the priority.
>  	 *
>  	 * Nothing to admit for parameters that are kept
>  	 * (SCHED_FLAG_KEEP_PARAMS): they are the ones the task has, and
> @@ -697,7 +699,8 @@ int __sched_setscheduler(struct task_struct *p,
>  	 * change of its own that landed in between would leave them stale.
>  	 */
>  	if (!(attr->sched_flags & SCHED_FLAG_KEEP_PARAMS) &&
> -	    (dl_policy(policy) || dl_task(p)) && sched_dl_overflow(p, policy, attr)) {
> +	    (dl_policy(policy) || task_has_dl_policy(p)) &&
> +	    sched_dl_overflow(p, policy, attr)) {
>  		retval = -EBUSY;
>  		goto unlock;
>  	}

[Severity: Medium]
Does this leave a dangling p->dl.pi_se pointer when normalize_rt_tasks()
(sysrq-n) calls __sched_setscheduler() with pi set to false on a non-deadline
task boosted by a deadline rt_mutex waiter?

With pi set to false, __sched_setscheduler() skips rt_effective_prio(), so
newprio is normal priority (120) and the task moves to fair_sched_class.
However, __setscheduler_dl_pi() only updates p->dl.pi_se when dl_prio(newprio)
is true:

kernel/sched/syscalls.c:__setscheduler_dl_pi() {
	if (dl_prio(newprio) && !dl_policy(policy)) {
		struct task_struct *pi_task = rt_mutex_get_top_task(p);

		if (pi_task) {
			p->dl.pi_se = pi_task->dl.pi_se;
			scope->flags |= ENQUEUE_REPLENISH;
		}
	}
}

switched_from_dl() does not reset p->dl.pi_se either because p->dl.dl_runtime
is 0, and when p later releases the rt_mutex, rt_mutex_setprio() sees
prio == p->prio && !dl_prio(prio) and returns early without resetting
p->dl.pi_se = &p->dl.  If the waiter later exits (or has its deadline
parameters cleared) and p subsequently enters the deadline policy,
is_dl_boosted(&p->dl) remains true and replenish_dl_entity() dereferences the
stale p->dl.pi_se pointer, causing a use-after-free or an infinite loop under
rq->__lock.

[Severity: Medium]
Can a non-deadline task boosted by a deadline waiter also bypass deadline
bandwidth enforcement by repeatedly calling sched_setscheduler() or
sched_setattr()?

With this change, a boosted non-deadline task calling __sched_setscheduler()
with pi set to true (for example, toggling between normal and batch policies)
keeps dl_prio(newprio) and reaches __setscheduler_dl_pi(), which
unconditionally sets ENQUEUE_REPLENISH on scope->flags.  When
sched_change_end() re-enqueues the task, enqueue_task_dl() clears
dl_se->dl_throttled and replenish_dl_entity() sees dl_se->dl_deadline == 0:

kernel/sched/deadline.c:replenish_dl_entity() {
	if (dl_se->dl_deadline == 0 ||
	    (dl_se->dl_defer_armed && dl_entity_overflow(dl_se, rq_clock(rq)))) {
		dl_se->deadline = rq_clock(rq) + pi_of(dl_se)->dl_deadline;
		dl_se->runtime = pi_of(dl_se)->dl_runtime;
	}
}

This refills dl_se->runtime to the donor's full runtime and unthrottles the
task on every call, allowing the mutex owner to replenish its inherited budget
indefinitely and starve lower-priority tasks on that CPU.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107014451.1487825-1-mathieu.desnoyers@efficios.com?part=4