Re: [PATCH 6.18.y 2/2] sched_ext: Fix scx_flush_disable_work() UAF race

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] sched_ext: premature return in scx_flush_disable_work() on SCX_EXIT_DONE
--

commit fe0315a0808b95d01cd45741655ca111deb8afc1
Author: Cheng-Yang Chou <yphbchou0911@gmail.com>

sched_ext: Fix scx_flush_disable_work() UAF race

This commit updates scx_flush_disable_work() to loop while syncing
sch->error_irq_work and flushing sch->disable_work until sch->exit_kind
reaches SCX_EXIT_NONE or SCX_EXIT_DONE. This aims to close a race window
where bpf_scx_unreg() frees the scheduler before a concurrent scx_vexit()
queues the disable work.

> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index d2460a88fba35..78f15ddc0aa5a 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -4217,8 +4217,13 @@ static void scx_disable(struct scx_sched *sch, enum scx_exit_kind kind)
>   */
>  static void scx_flush_disable_work(struct scx_sched *sch)
>  {
> -	irq_work_sync(&sch->error_irq_work);
> -	kthread_flush_work(&sch->disable_work);
> +	int kind;
> +
> +	do {
> +		irq_work_sync(&sch->error_irq_work);
> +		kthread_flush_work(&sch->disable_work);
> +		kind = atomic_read(&sch->exit_kind);
> +	} while (kind != SCX_EXIT_NONE && kind != SCX_EXIT_DONE);

[Severity: Critical]
Can scx_flush_disable_work() return while sch->disable_work is still
running if sch->exit_kind transitions to SCX_EXIT_DONE between
kthread_flush_work() and atomic_read(&sch->exit_kind)?

In scx_disable_workfn(), sch->exit_kind is set to SCX_EXIT_DONE at the
start of the work function before running scx_root_disable(sch):

kernel/sched/ext.c:scx_disable_workfn() {
	...
	while (true) {
		if (kind == SCX_EXIT_DONE)	/* already disabled? */
			return;
		WARN_ON_ONCE(kind == SCX_EXIT_NONE);
		if (atomic_try_cmpxchg(&sch->exit_kind, &kind, SCX_EXIT_DONE))
			break;
	}
	ei->kind = kind;
	ei->reason = scx_exit_reason(ei->kind);

	scx_root_disable(sch);
}

If CPU A is in scx_vexit() (or scx_disable()) between scx_claim_exit() and
queuing the work while CPU B enters bpf_scx_unreg() ->
scx_flush_disable_work():

1. On CPU B's first pass, irq_work_sync(&sch->error_irq_work) and
   kthread_flush_work(&sch->disable_work) return as no-ops because neither
   work item has been queued yet.
2. CPU A then queues sch->error_irq_work (or sch->disable_work), and
   scx_disable_workfn() starts running on sch->helper, sets sch->exit_kind
   to SCX_EXIT_DONE, and enters scx_root_disable(sch).
3. CPU B executes kind = atomic_read(&sch->exit_kind), observes
   SCX_EXIT_DONE, and breaks out of the loop without ever having flushed
   the running sch->disable_work.

Could that allow bpf_scx_unreg() to call kobject_put(&sch->kobj) and return
to free the BPF struct_ops map while scx_root_disable(sch) and ops.exit()
are still executing? Would reading sch->exit_kind before syncing and
flushing (or performing a final kthread_flush_work(&sch->disable_work)
after the loop) avoid returning before scx_disable_workfn() finishes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103190805.3487350-1-jpiecuch@google.com?part=2