Re: [PATCH 3/9] sched_ext: Fix TOCTOU race in consume_remote_task()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing tracking bug in dispatch_to_local_dsq(): it drops the locked_rq lock without clearing the scx_locked_rq_state tracking, which triggers a lockdep crash if a nested ops.dequeue() callback executes in the fallback path.
--

commit aff47a8e37704807f1fefe8e15adcb99c8836308
Author: Andrea Righi <arighi@nvidia.com>

sched_ext: Fix TOCTOU race in consume_remote_task()

This commit addresses a TOCTOU race condition in sched_ext during task
migration between runqueues. It ensures task eligibility is re-evaluated
after locking the source runqueue and properly tracks runqueue lock state
while dropping the lock to avoid lockdep warnings.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 242f6ffbba350..4c6cd694c86db 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
[ ... ]
> @@ -2240,20 +2246,58 @@ static bool unlink_dsq_and_lock_src_rq(struct task_struct *p,
>  		!WARN_ON_ONCE(src_rq != task_rq(p));
>  }
>  
> -static bool consume_remote_task(struct rq *this_rq,
> +static bool consume_remote_task(struct scx_sched *sch, struct rq *this_rq,
>  				struct task_struct *p, u64 enq_flags,
>  				struct scx_dispatch_q *dsq, struct rq *src_rq)
>  {
> +	struct rq *tracked_rq = scx_locked_rq();
> +	bool consumed = false;
> +
> +	/*
> +	 * consume_remote_task() may be called from an SCX op with @this_rq
> +	 * recorded as the currently locked rq. Clear the tracking while the rq
> +	 * lock is dropped so nested callbacks don't save and later try to restore
> +	 * an rq which isn't locked anymore.
> +	 */
> +	if (tracked_rq) {
> +		WARN_ON_ONCE(tracked_rq != this_rq);
> +		update_locked_rq(NULL);
> +	}
>  	raw_spin_rq_unlock(this_rq);

[Severity: High]
This is a pre-existing issue, but does dispatch_to_local_dsq() have a similar
problem with stale rq lock tracking?

Similar to consume_remote_task(), dispatch_to_local_dsq() drops the locked_rq
without temporarily clearing scx_locked_rq_state:

kernel/sched/ext/ext.c:dispatch_to_local_dsq() {
    ...
	/* switch to @src_rq lock */
	if (locked_rq != src_rq) {
		raw_spin_rq_unlock(locked_rq);
		locked_rq = src_rq;
		raw_spin_rq_lock(src_rq);
	}
    ...
}

If task_can_run_on_remote_rq() fails, it falls back to enqueueing to the global
DSQ:

		} else if (unlikely(!task_can_run_on_remote_rq(sch, p, dst_rq, true))) {
			p->scx.holding_cpu = -1;
			fallback = true;
			scx_dispatch_enqueue(sch, src_rq, find_global_dsq(sch, task_cpu(p)),
					     p, enq_flags | SCX_ENQ_GDSQ_FALLBACK);

When scx_dispatch_enqueue() triggers the nested ops.dequeue() callback via
SCX_CALL_OP_TASK, the stale, unlocked rq state is saved and later restored
in SCX_CALL_OP, which triggers a lockdep assertion failure.

Could this lead to a lockdep crash if a nested ops.dequeue() executes in the
fallback path?

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