Re: [PATCH 06/11] sched_ext: Split curr|donor references properly

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete migration from rq->curr to rq->donor for proxy execution breaks capability revocation, preemption checks, and BPF kfuncs.
--

--- Patch [6]: [PATCH 06/11] sched_ext: Split curr|donor references properly ---
commit 8ca59038e09f66e910a65ddcaf538ce84385abf5
Author: Andrea Righi <[email protected]>

sched_ext: Split curr|donor references properly

This commit updates sched_ext to use rq->donor instead of rq->curr where
appropriate to correctly handle proxy execution. This ensures that the
scheduler-selected task (donor) supplies the scheduling context and budget,
even when a different task physically executes on the CPU.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 8d8057b80d544..0d2f34c79083f 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -8030,7 +8049,7 @@ static bool kick_one_cpu(s32 cpu, struct scx_sched_pcpu *pcpu, struct rq *this_r
>  	unsigned long flags;
>  
>  	raw_spin_rq_lock_irqsave(rq, flags);
> -	cur_class = rq->curr->sched_class;
> +	cur_class = rq->donor->sched_class;
>  
>  	/*
>  	 * During CPU hotplug, a CPU may depend on kicking itself to make
> @@ -8047,7 +8066,7 @@ static bool kick_one_cpu(s32 cpu, struct scx_sched_pcpu *pcpu, struct rq *this_r
>  			if (cur_class == &ext_sched_class) {
>  				if (likely(!scx_missing_caps(pcpu->sch, cpu,
>  							     scx_caps_for_preempt(pcpu->sch, rq))))
> -					set_task_slice(rq->curr, 0);
> +					set_task_slice(rq->donor, 0);
>  				else
>  					__scx_add_event(pcpu->sch,
>  							SCX_EV_SUB_PREEMPT_DENIED, 1);

[Severity: High]
Should this migration from rq->curr to rq->donor also be applied to other
functions that interact with the scheduling context?

For example, in reenq_local(), capability revocation eviction appears to check
the physical lock owner instead of the proxy donor:

kernel/sched/ext/ext.c:reenq_local() {
...
	if ((reenq_flags & SCX_REENQ_CAP_REVOKE) &&
	    rq->curr->sched_class == &ext_sched_class &&
	    scx_task_reenq_on_cap_revoke(rq, rq->curr)) {
		set_task_slice(rq->curr, 0);
...
}

Could this cause capability revocation to fail for proxy donors executing
on behalf of non-EXT tasks?

Similarly, scx_caps_for_preempt() evaluates capabilities against the physical
lock owner:

kernel/sched/ext/sub.h:scx_caps_for_preempt() {
	struct task_struct *curr = rq->curr;
...
	if (curr->sched_class != &ext_sched_class ||
...
}

Could this allow unauthorized preemption if the proxy donor is an EXT task
but the physical lock owner is a non-EXT task?

Additionally, BPF kfuncs scx_bpf_task_running() and scx_bpf_cpu_curr() report
the physical owner instead of the expected scheduling context:

kernel/sched/ext/ext.c:scx_bpf_task_running() {
	return task_rq(p)->curr == p;
}

kernel/sched/ext/ext.c:scx_bpf_cpu_curr() {
	return rcu_dereference(cpu_rq(cpu)->curr);
}

Does this expose the physical lock owner instead of the proxy donor context,
potentially confusing the BPF scheduler which expects to see the EXT task it
dispatched?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6