Re: [PATCH 6/9] sched_ext: Delegate proxy donor admission to BPF schedulers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] task_proxy_cpu() locklessly accesses p->blocked_on and dereferences the mutex to acquire its wait_lock, leading to a Use-After-Free if the target task is awakened and the mutex is concurrently destroyed.
- [High] task_proxy_cpu() acquires p->blocked_lock using raw_spinlock without disabling interrupts, risking a hard deadlock against hardware IRQ handlers.
- [High] sched_proxy_block_task() asynchronously swaps the proxy donor to idle without forcing a reschedule on the target CPU, leaving the target CPU to run the mutex owner indefinitely under idle accounting.
- [High] sched_setscheduler() fails to block proxy donors when transitioning them to a BPF scheduler without SCX_OPS_ENQ_BLOCKED, violating scheduler admission policies.
- [High] scx_prepare_task_sched_change() is called locklessly, modifying runqueue state and updating the runqueue clock without holding rq->lock or pi_lock.
--

commit fbf923089be5e0555003149fc8823b7f5e5723f7
Author: Andrea Righi <arighi@nvidia.com>
Subject: sched_ext: Delegate proxy donor admission to BPF schedulers

Proxy execution keeps a blocked donor runnable so its scheduling context
can execute the mutex owner. This commit adds SCX_OPS_ENQ_BLOCKED to delegate
proxy execution admission to BPF schedulers, allowing them to handle blocked
donors according to their own policies. Schedulers without this capability
have their donors blocked normally via sched_proxy_block_task().

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 3673b17f6098c..19a6a78ef8472 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c

[ ... ]

> @@ -6767,6 +6767,28 @@ static void proxy_deactivate(struct rq *rq, struct task_struct *donor)
>  	block_task(rq, donor, state);
>  }
>  
> +/*
> + * Remove a retained proxy donor before changing its scheduler ownership.
> + * The caller holds p->pi_lock, so p cannot wake and migrate after block_task()
> + * drops it from the runqueue.
> + */
> +void sched_proxy_block_task(struct rq *rq, struct task_struct *p)
> +{
> +	unsigned long state = READ_ONCE(p->__state);
> +
> +	lockdep_assert_held(&p->pi_lock);
> +	lockdep_assert_rq_held(rq);
> +
> +	if (!task_is_blocked(p) || !task_on_rq_queued(p))
> +		return;
> +	if (WARN_ON_ONCE(state == TASK_RUNNING))
> +		return;
> +
> +	if (task_current_donor(rq, p))
> +		proxy_resched_idle(rq);
> +	block_task(rq, p, state);
> +}

[Severity: High]
Is this missing a resched_curr(rq) call?

When scx_prepare_task_sched_change() calls sched_proxy_block_task() remotely, it
replaces rq->donor with rq->idle via proxy_resched_idle(rq). If no IPI is sent
to notify the remote CPU (since resched_curr() is omitted), won't the remote CPU
continue running the mutex owner indefinitely under idle accounting, completely
bypassing scheduler preemption?

[ ... ]

> @@ -7009,6 +7031,39 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>  	proxy_migrate_task(rq, rf, p, owner_cpu);
>  	return NULL;
>  }
> +
> +int task_proxy_cpu(struct task_struct *p)
> +{
> +	struct task_struct *owner;
> +	struct mutex *mutex;
> +	int owner_cpu;
> +
> +	if (!task_is_blocked(p))
> +		return -ENOENT;
> +
> +	mutex = READ_ONCE(p->blocked_on);
> +	if (!mutex)
> +		return -ENOENT;
> +
> +	guard(raw_spinlock)(&mutex->wait_lock);

[Severity: Critical]
Can this result in a use-after-free?

The scx_bpf_task_proxy_cpu() kfunc can invoke this on any RCU-protected task.
The lockless read of p->blocked_on above occurs without holding p->pi_lock or
rq->lock. If the target task concurrently wakes up, finishes its execution, and
frees the mutex, the wait_lock acquisition here would be dereferencing freed
memory.

> +	guard(raw_spinlock)(&p->blocked_lock);

[Severity: High]
Can this cause a spinlock deadlock against hardware IRQs?

The scx_bpf_task_proxy_cpu() kfunc can be called from IRQ-enabled contexts like
BPF_PROG_TYPE_SYSCALL programs. Because guard(raw_spinlock) does not disable
interrupts (unlike raw_spinlock_irqsave), if an interrupt fires while the lock
is held and tries to wake the same task (calling clear_task_blocked_on() from
hardirq context), it will deadlock trying to acquire p->blocked_lock.

[ ... ]

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 85e07cbe626f6..a47ddbf741ea3 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -23,6 +23,27 @@
>  
>  DEFINE_RAW_SPINLOCK(scx_sched_lock);
>  
> +bool scx_allow_proxy_exec(const struct task_struct *p)
> +{
> +	if (!task_on_scx(p))
> +		return true;
> +
> +	return scx_task_sched(p)->ops.flags & SCX_OPS_ENQ_BLOCKED;
> +}
> +
> +/*
> + * Called immediately before sched_change_begin(). The caller must pass
> + * DEQUEUE_NOCLOCK so the rq clock is updated only once.
> + */
> +void scx_prepare_task_sched_change(struct task_struct *p, struct scx_sched *sch)
> +{
> +	update_rq_clock(task_rq(p));
> +
> +	/* Block retained donors that the incoming scheduler cannot manage. */
> +	if (!(sch->ops.flags & SCX_OPS_ENQ_BLOCKED))
> +		sched_proxy_block_task(task_rq(p), p);
> +}

[Severity: High]
How are blocked donors handled when a task policy is changed via the
sched_setscheduler() syscall?

It appears __sched_setscheduler() changes the task's class but does not invoke
scx_prepare_task_sched_change(), nor does scx_check_setscheduler() validate
this transition. If a blocked proxy donor transitions to a BPF scheduler that
lacks SCX_OPS_ENQ_BLOCKED, won't it bypass the admission check and get enqueued
as a normal runnable task instead of a blocked donor?

[ ... ]

> @@ -6950,6 +6985,10 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  
>  		if (old_class != new_class)
>  			queue_flags |= DEQUEUE_CLASS;
> +		if (new_class == &ext_sched_class) {
> +			scx_prepare_task_sched_change(p, sch);
> +			queue_flags |= DEQUEUE_NOCLOCK;
> +		}
>  
>  		scoped_guard (sched_change, p, queue_flags) {

[Severity: High]
Is it safe to call scx_prepare_task_sched_change() outside the sched_change guard?

scx_prepare_task_sched_change() calls update_rq_clock() and
sched_proxy_block_task(), both of which strictly require rq->lock. However, here
they are called locklessly before scoped_guard() acquires p->pi_lock and rq->lock.
Could this cause runqueue RB-tree corruption and lockdep panics?

[ ... ]

> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 0504204272732..0a5b84575ba5a 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c

[ ... ]

> @@ -117,7 +117,9 @@ static void scx_fail_parent(struct scx_sched *sch,
>  		if (scx_task_on_sched(parent, p))
>  			continue;
>  
> -		scoped_guard (sched_change, p, DEQUEUE_SAVE | DEQUEUE_MOVE) {
> +		scx_prepare_task_sched_change(p, parent);
> +		scoped_guard (sched_change, p, DEQUEUE_SAVE | DEQUEUE_MOVE |
> +			      DEQUEUE_NOCLOCK) {

[Severity: High]
Same issue here. scx_prepare_task_sched_change() is called locklessly before
the rq->lock is acquired by the sched_change guard.

[ ... ]

> @@ -209,7 +211,9 @@ void scx_sub_disable(struct scx_sched *sch)
>  			continue;
>  		}
>  
> -		scoped_guard (sched_change, p, DEQUEUE_SAVE | DEQUEUE_MOVE) {
> +		scx_prepare_task_sched_change(p, parent);
> +		scoped_guard (sched_change, p, DEQUEUE_SAVE | DEQUEUE_MOVE |
> +			      DEQUEUE_NOCLOCK) {

[Severity: High]
Same issue here. scx_prepare_task_sched_change() is called locklessly.

[ ... ]

> @@ -503,7 +507,9 @@ void scx_sub_enable_workfn(struct kthread_work *work)
>  		if (!(p->scx.flags & SCX_TASK_SUB_INIT))
>  			continue;
>  
> -		scoped_guard (sched_change, p, DEQUEUE_SAVE | DEQUEUE_MOVE) {
> +		scx_prepare_task_sched_change(p, sch);
> +		scoped_guard (sched_change, p, DEQUEUE_SAVE | DEQUEUE_MOVE |
> +			      DEQUEUE_NOCLOCK) {

[Severity: High]
Same issue here. scx_prepare_task_sched_change() is called locklessly.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706070410.282826-1-arighi@nvidia.com?part=6