Re: [PATCH sched_ext/for-7.4-fixes] sched_ext: Read exit_kind before flushing in scx_flush_disable_work()

[Andrea Righi]

Hi Kuba,

On Wed, Nov 04, 2026 at 11:14:39AM +0000, Kuba Piecuch wrote:
> scx_disable_workfn() sets exit_kind to SCX_EXIT_DONE before it tears the
> scheduler down. scx_flush_disable_work() reads exit_kind after syncing
> disable_irq_work and flushing disable_work, so it can observe DONE from
> a disable_work that was queued after its flush and return while the
> teardown is still running:
>   CPU A                          CPU B
>   -----                          -----
>   scx_vexit()
>     scx_claim_exit() -> true
>                                  bpf_struct_ops_map_link_detach()
>                                    bpf_scx_unreg()
>                                      scx_disable()
>                                        scx_claim_exit() -> false
>                                      scx_flush_disable_work()
>                                        irq_work_sync()      // not queued
>                                        kthread_flush_work() // not queued
>     irq_work_queue()
>   scx_disable_irq_workfn()
>     kthread_queue_work()
>   scx_disable_workfn()
>     exit_kind = SCX_EXIT_DONE
>     scx_root_disable()
>                                        atomic_read() -> SCX_EXIT_DONE
>                                    bpf_map_put()

I was also able to reproduce this in virtme-ng racing scx_bpf_exit()
against bpf_link_detach(), but only by adding an artificial sleep
between the flush and the exit_kind read to widen the window.

>
> 
> Once bpf_scx_unreg() returns, its caller drops the struct_ops map
> reference, and the struct_ops trampolines can be freed while
> scx_root_disable() is still running and can still call into the BPF
> scheduler, e.g. ops.exit().
> 
> Read exit_kind before syncing and flushing, with acquire ordering paired
> with the atomic_try_cmpxchg() in scx_disable_workfn(). DONE is only set
> from disable_work, so once it is observed, the following
> kthread_flush_work() sees the queued work and waits for the teardown to
> finish.
> 
> This patch was created with the help of an AI coding assistant.
> 
> Fixes: d99f7a32f09d ("sched_ext: Fix scx_flush_disable_work() UAF race")
> Cc: stable@xxxxxxxxxxxxxxx # v7.1+
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Closes: https://lore.kernel.org/all/sashiko-outbox-195908@xxxxxxxxxx/
> Signed-off-by: Kuba Piecuch <jpiecuch@xxxxxxxxxx>

With the artificial sleep I also hit a refcount_t underflow in
scx_root_disable(), followed by a GPF in kobject_add_internal() on the
next enable. I saw this only once and haven't root-caused it.

In any case, this looks good to me.

Reviewed-by: Andrea Righi <arighi@xxxxxxxxxx>

Thanks,
-Andrea

> ---
>  kernel/sched/ext/ext.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 6c593d3dee3c..949143bf4752 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -7810,10 +7810,17 @@ void scx_flush_disable_work(struct scx_sched *sch)
>  {
>  	int kind;
>  
> +	/*
> +	 * scx_disable_workfn() sets SCX_EXIT_DONE before tearing down. Read
> +	 * exit_kind with acquire ordering (paired with atomic_try_cmpxchg() in
> +	 * scx_disable_workfn()) before syncing and flushing so that, once DONE
> +	 * is seen, the following flush observes the queued work and waits for
> +	 * the teardown to finish.
> +	 */
>  	do {
> +		kind = atomic_read_acquire(&sch->exit_kind);
>  		irq_work_sync(&sch->disable_irq_work);
>  		kthread_flush_work(&sch->disable_work);
> -		kind = atomic_read(&sch->exit_kind);
>  	} while (kind != SCX_EXIT_NONE && kind != SCX_EXIT_DONE);
>  }
>  
> -- 
> 2.56.0.398.g882cbad275-goog
>