Sched_ext development
 help / color / mirror / Atom feed
* [PATCH] sched_ext: Close the pre-enable ops error claim window
@ 2026-09-10  8:45 Qiurong Fang
  2026-09-10  9:01 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Qiurong Fang @ 2026-09-10  8:45 UTC (permalink / raw)
  To: tj; +Cc: sched-ext, void, arighi, changwoo

From: fangqiurong <fangqiurong@kylinos.cn>

scx_alloc_and_add_sched() publishes ops->priv before
scx_root_enable_workfn() switches the state to SCX_ENABLING. An error
claimed via scx_bpf_error_bstr() from an associated BPF program in that
window is consumed by scx_disable_workfn(), which takes the pre-enable
shortcut in scx_root_disable(). The shortcut returns without any
teardown and restores SCX_DISABLED with an unconditional
scx_set_enable_state() xchg racing the enable workfn's own transition.
The enable then completes with the claim consumed: the scheduler stays
up but can never be disabled again, and bpf_scx_unreg() frees it while
still in use, resulting in a use-after-free. Both WARN_ON_ONCE()s fire
back to back:

  WARNING: kernel/sched/ext/ext.c:7522 at scx_root_enable_workfn+0xeec/0x1be0, CPU#3: scx_enable_help/276
  WARNING: kernel/sched/ext/ext.c:6398 at scx_root_disable+0xb50/0xdb8, CPU#0: sched_ext_helpe/664

scx_root_enable_workfn() switches to SCX_ENABLING before the
allocation, so ops->priv is never visible while SCX_DISABLED and
claimed errors get the regular scx_root_disable() teardown serialized
on scx_enable_mutex behind the in-flight enable.

Fixes: 105dcd005be2 ("sched_ext: Introduce scx_prog_sched()")
Cc: stable@vger.kernel.org
Signed-off-by: fangqiurong <fangqiurong@kylinos.cn>
---
 kernel/sched/ext/ext.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 10af28a9f2c0..67626c162878 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -6395,8 +6395,9 @@ static void scx_root_disable(struct scx_sched *sch)
 	case SCX_DISABLED:
 		pr_warn("sched_ext: ops error detected without ops (%s)\n",
 			sch->exit_info->msg);
-		WARN_ON_ONCE(scx_set_enable_state(SCX_DISABLED) != SCX_DISABLING);
-		goto done;
+		if (scx_tryset_enable_state(SCX_DISABLED, SCX_DISABLING))
+			goto done;
+		break;
 	default:
 		break;
 	}
@@ -7506,22 +7507,27 @@ static void scx_root_enable_workfn(struct kthread_work *work)
 #ifdef CONFIG_EXT_SUB_SCHED
 	cgroup_get(cgrp);
 #endif
+	/* Transition to SCX_ENABLING before @ops->priv publication */
+	if (!scx_tryset_enable_state(SCX_ENABLING, SCX_DISABLED)) {
+		pr_warn("sched_ext: enable raced with a state transition, aborting\n");
+		ret = -EBUSY;
+#ifdef CONFIG_EXT_SUB_SCHED
+		cgroup_put(cgrp);
+#endif
+		goto err_free_tid_hash;
+	}
+	WARN_ON_ONCE(scx_root);
+
 	sch = scx_alloc_and_add_sched(cmd, cgrp, NULL);
 	if (IS_ERR(sch)) {
 		ret = PTR_ERR(sch);
+		scx_tryset_enable_state(SCX_DISABLED, SCX_ENABLING);
 		goto err_free_tid_hash;
 	}
 
 	if (sch->is_cid_type)
 		static_branch_enable(&__scx_is_cid_type);
 
-	/*
-	 * Transition to ENABLING and clear exit info to arm the disable path.
-	 * Failure triggers full disabling from here on.
-	 */
-	WARN_ON_ONCE(scx_set_enable_state(SCX_ENABLING) != SCX_DISABLED);
-	WARN_ON_ONCE(scx_root);
-
 	atomic_long_set(&scx_nr_rejected, 0);
 
 	for_each_possible_cpu(cpu) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] sched_ext: Close the pre-enable ops error claim window
  2026-09-10  8:45 [PATCH] sched_ext: Close the pre-enable ops error claim window Qiurong Fang
@ 2026-09-10  9:01 ` sashiko-bot
  2026-09-10 21:40 ` Tejun Heo
  2026-09-11  2:42 ` [PATCH v2] " Qiurong Fang
  2 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-09-10  9:01 UTC (permalink / raw)
  To: Qiurong Fang; +Cc: sched-ext

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent calls to scx_root_disable() on the same scheduler cause a double teardown due to fall-throughs on SCX_DISABLING and SCX_DISABLED.
--

commit dd97abeb536b6afa1a46d2a0b2de7e7eaddfef0a
Author: fangqiurong <fangqiurong@kylinos.cn>

sched_ext: Close the pre-enable ops error claim window

This commit addresses a race window where scx_alloc_and_add_sched()
publishes ops->priv before transitioning to SCX_ENABLING. It moves the state
transition before the allocation and adjusts the early exit logic in
scx_root_disable() to prevent a teardown shortcut from racing with the
enable workfn.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c

[ ... ]

> @@ -6395,8 +6395,9 @@ static void scx_root_disable(struct scx_sched *sch)
>  	case SCX_DISABLED:
>  		pr_warn("sched_ext: ops error detected without ops (%s)\n",
>  			sch->exit_info->msg);
> -		WARN_ON_ONCE(scx_set_enable_state(SCX_DISABLED) != SCX_DISABLING);
> -		goto done;
> +		if (scx_tryset_enable_state(SCX_DISABLED, SCX_DISABLING))
> +			goto done;
> +		break;

[Severity: High]
If two threads concurrently call scx_root_disable() (e.g., a SysRq disable
and a BPF error triggering scx_disable_workfn), both will serialize on
scx_enable_mutex.

For the thread that arrives second, if scx_set_enable_state() returns
SCX_DISABLED and scx_tryset_enable_state() fails, the newly added break
statement will cause it to fall through the switch block instead of going to
done.

Similarly, if scx_set_enable_state() returns SCX_DISABLING, the adjacent
case block logs a warning and also breaks out of the switch.

In both cases, does this allow the second thread to bypass the early exit,
acquire scx_enable_mutex, and execute the full teardown sequence again on an
already disabled scheduler?

Could this result in memory corruption from double frees (such as calling
rhashtable_free_and_destroy() on scx_tid_hash again) and duplicate calls to
the BPF program's exit callback?

>  	default:
>  		break;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910084533.2420199-1-fangqiurong@kylinos.cn?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] sched_ext: Close the pre-enable ops error claim window
  2026-09-10  8:45 [PATCH] sched_ext: Close the pre-enable ops error claim window Qiurong Fang
  2026-09-10  9:01 ` sashiko-bot
@ 2026-09-10 21:40 ` Tejun Heo
  2026-09-11  2:42 ` [PATCH v2] " Qiurong Fang
  2 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2026-09-10 21:40 UTC (permalink / raw)
  To: Qiurong Fang
  Cc: sched-ext, David Vernet, Andrea Righi, Changwoo Min,
	Emil Tsalapatis, linux-kernel, tj

Hello, Qiurong.

On Thu, Sep 10, 2026 at 04:45:33PM +0800, Qiurong Fang wrote:
> +		if (scx_tryset_enable_state(SCX_DISABLED, SCX_DISABLING))
> +			goto done;
> +		break;

Please drop this hunk. Once SCX_ENABLING precedes ops->priv publication,
scx_tryset_enable_state() cannot fail here.

> +	if (!scx_tryset_enable_state(SCX_ENABLING, SCX_DISABLED)) {
> +		pr_warn("sched_ext: enable raced with a state transition, aborting\n");
> +		ret = -EBUSY;

This failure path is also unreachable. The state was checked as SCX_DISABLED
under scx_enable_mutex, and no disable can start before publication. Please
move the existing transition and retain the reset on allocation failure.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] sched_ext: Close the pre-enable ops error claim window
  2026-09-10  8:45 [PATCH] sched_ext: Close the pre-enable ops error claim window Qiurong Fang
  2026-09-10  9:01 ` sashiko-bot
  2026-09-10 21:40 ` Tejun Heo
@ 2026-09-11  2:42 ` Qiurong Fang
  2026-09-11 15:24   ` Tejun Heo
  2026-09-12 13:15   ` [PATCH v3] " Qiurong Fang
  2 siblings, 2 replies; 7+ messages in thread
From: Qiurong Fang @ 2026-09-11  2:42 UTC (permalink / raw)
  To: tj; +Cc: sched-ext, void, arighi, changwoo

From: fangqiurong <fangqiurong@kylinos.cn>

scx_alloc_and_add_sched() publishes ops->priv before
scx_root_enable_workfn() switches the state to SCX_ENABLING. An error
claimed via scx_bpf_error_bstr() from an associated BPF program in that
window is consumed by scx_disable_workfn(), which takes the pre-enable
shortcut in scx_root_disable(). The shortcut returns without any
teardown and restores SCX_DISABLED with an unconditional
scx_set_enable_state() xchg racing the enable workfn's own transition.
The enable then completes with the claim consumed: the scheduler stays
up but can never be disabled again, and bpf_scx_unreg() frees it while
still in use, resulting in a use-after-free. Both WARN_ON_ONCE()s fire
back to back:

  WARNING: kernel/sched/ext/ext.c:7522 at scx_root_enable_workfn+0xeec/0x1be0, CPU#3: scx_enable_help/276
  WARNING: kernel/sched/ext/ext.c:6398 at scx_root_disable+0xb50/0xdb8, CPU#0: sched_ext_helpe/664

scx_root_enable_workfn() switches to SCX_ENABLING before the
allocation, so ops->priv is never visible while SCX_DISABLED and
claimed errors get the regular scx_root_disable() teardown serialized
on scx_enable_mutex behind the in-flight enable. 

Fixes: 105dcd005be2 ("sched_ext: Introduce scx_prog_sched()")
Cc: stable@vger.kernel.org
Signed-off-by: fangqiurong <fangqiurong@kylinos.cn>
---

Changes since v1:
 - Drop the conditional restore in the pre-enable shortcut and the
   tryset/-EBUSY branch on the moved transition; both failure paths are
   unreachable once SCX_ENABLING precedes ops->priv publication
   (Tejun Heo).
 - Keep the allocation failure reset.
 kernel/sched/ext/ext.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 10af28a9f2c0..479ae257ea7c 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -7506,22 +7506,23 @@ static void scx_root_enable_workfn(struct kthread_work *work)
 #ifdef CONFIG_EXT_SUB_SCHED
 	cgroup_get(cgrp);
 #endif
+	/*
+	 * Transition to ENABLING and clear exit info to arm the disable path.
+	 * Failure triggers full disabling from here on.
+	 */
+	WARN_ON_ONCE(scx_set_enable_state(SCX_ENABLING) != SCX_DISABLED);
+	WARN_ON_ONCE(scx_root);
+
 	sch = scx_alloc_and_add_sched(cmd, cgrp, NULL);
 	if (IS_ERR(sch)) {
 		ret = PTR_ERR(sch);
+		scx_tryset_enable_state(SCX_DISABLED, SCX_ENABLING);
 		goto err_free_tid_hash;
 	}
 
 	if (sch->is_cid_type)
 		static_branch_enable(&__scx_is_cid_type);
 
-	/*
-	 * Transition to ENABLING and clear exit info to arm the disable path.
-	 * Failure triggers full disabling from here on.
-	 */
-	WARN_ON_ONCE(scx_set_enable_state(SCX_ENABLING) != SCX_DISABLED);
-	WARN_ON_ONCE(scx_root);
-
 	atomic_long_set(&scx_nr_rejected, 0);
 
 	for_each_possible_cpu(cpu) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] sched_ext: Close the pre-enable ops error claim window
  2026-09-11  2:42 ` [PATCH v2] " Qiurong Fang
@ 2026-09-11 15:24   ` Tejun Heo
  2026-09-12 13:15   ` [PATCH v3] " Qiurong Fang
  1 sibling, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2026-09-11 15:24 UTC (permalink / raw)
  To: Qiurong Fang
  Cc: sched-ext, David Vernet, Andrea Righi, Changwoo Min,
	Emil Tsalapatis, linux-kernel, tj

Hello, Qiurong.

On Fri, Sep 11, 2026 at 10:42:56AM +0800, Qiurong Fang wrote:
> +		scx_tryset_enable_state(SCX_DISABLED, SCX_ENABLING);

All allocation failures precede ops->priv publication, so no disable can
race here and the state must still be SCX_ENABLING. Please use:

        WARN_ON_ONCE(scx_set_enable_state(SCX_DISABLED) != SCX_ENABLING);

Please also update the moved comment. Allocation failure still unwinds
locally. Full disabling on failure applies only after
scx_alloc_and_add_sched() succeeds.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3] sched_ext: Close the pre-enable ops error claim window
  2026-09-11  2:42 ` [PATCH v2] " Qiurong Fang
  2026-09-11 15:24   ` Tejun Heo
@ 2026-09-12 13:15   ` Qiurong Fang
  2026-09-13 15:59     ` Tejun Heo
  1 sibling, 1 reply; 7+ messages in thread
From: Qiurong Fang @ 2026-09-12 13:15 UTC (permalink / raw)
  To: tj; +Cc: sched-ext, void, arighi, changwoo

From: fangqiurong <fangqiurong@kylinos.cn>

scx_alloc_and_add_sched() publishes ops->priv before
scx_root_enable_workfn() switches the state to SCX_ENABLING. An error
claimed via scx_bpf_error_bstr() from an associated BPF program in that
window is consumed by scx_disable_workfn(), which takes the pre-enable
shortcut in scx_root_disable(). The shortcut returns without any
teardown and restores SCX_DISABLED with an unconditional
scx_set_enable_state() xchg racing the enable workfn's own transition.
The enable then completes with the claim consumed: the scheduler stays
up but can never be disabled again, and bpf_scx_unreg() frees it while
still in use, resulting in a use-after-free. Both WARN_ON_ONCE()s fire
back to back:

  WARNING: kernel/sched/ext/ext.c:7522 at scx_root_enable_workfn+0xeec/0x1be0, CPU#3: scx_enable_help/276
  WARNING: kernel/sched/ext/ext.c:6398 at scx_root_disable+0xb50/0xdb8, CPU#0: sched_ext_helpe/664

scx_root_enable_workfn() switches to SCX_ENABLING before the
ops->priv allocation, so ops->priv is never visible while
SCX_DISABLED. The allocation failure path restores SCX_DISABLED.

Fixes: 105dcd005be2 ("sched_ext: Introduce scx_prog_sched()")
Cc: stable@vger.kernel.org
Signed-off-by: fangqiurong <fangqiurong@kylinos.cn>

---

Changes since v2:
 - Restore the state with WARN_ON_ONCE() on the allocation failure
   path (Tejun Heo).
 - Fix up the moved comment (Tejun Heo).
---
 kernel/sched/ext/ext.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 3b525f06a720..ea2f1257aee1 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -7523,22 +7523,24 @@ static void scx_root_enable_workfn(struct kthread_work *work)
 #ifdef CONFIG_EXT_SUB_SCHED
 	cgroup_get(cgrp);
 #endif
+	/*
+	 * Transition to ENABLING and clear exit info to arm the disable path.
+	 * Allocation failure still unwinds locally. Full disabling on failure
+	 * applies only after scx_alloc_and_add_sched() succeeds.
+	 */
+	WARN_ON_ONCE(scx_set_enable_state(SCX_ENABLING) != SCX_DISABLED);
+	WARN_ON_ONCE(scx_root);
+
 	sch = scx_alloc_and_add_sched(cmd, cgrp, NULL);
 	if (IS_ERR(sch)) {
 		ret = PTR_ERR(sch);
+		WARN_ON_ONCE(scx_set_enable_state(SCX_DISABLED) != SCX_ENABLING);
 		goto err_free_tid_hash;
 	}
 
 	if (sch->is_cid_type)
 		static_branch_enable(&__scx_is_cid_type);
 
-	/*
-	 * Transition to ENABLING and clear exit info to arm the disable path.
-	 * Failure triggers full disabling from here on.
-	 */
-	WARN_ON_ONCE(scx_set_enable_state(SCX_ENABLING) != SCX_DISABLED);
-	WARN_ON_ONCE(scx_root);
-
 	atomic_long_set(&scx_nr_rejected, 0);
 
 	for_each_possible_cpu(cpu) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] sched_ext: Close the pre-enable ops error claim window
  2026-09-12 13:15   ` [PATCH v3] " Qiurong Fang
@ 2026-09-13 15:59     ` Tejun Heo
  0 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2026-09-13 15:59 UTC (permalink / raw)
  To: Qiurong Fang
  Cc: tj, David Vernet, Andrea Righi, Changwoo Min, Emil Tsalapatis,
	sched-ext, linux-kernel

Applied to sched_ext/for-7.3-fixes with minor wording cleanups.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-09-13 15:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-10  8:45 [PATCH] sched_ext: Close the pre-enable ops error claim window Qiurong Fang
2026-09-10  9:01 ` sashiko-bot
2026-09-10 21:40 ` Tejun Heo
2026-09-11  2:42 ` [PATCH v2] " Qiurong Fang
2026-09-11 15:24   ` Tejun Heo
2026-09-12 13:15   ` [PATCH v3] " Qiurong Fang
2026-09-13 15:59     ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox