public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] sched_ext: Use rcu_dereference() for scx_root in dump paths
@ 2026-02-26  5:26 David Carlier
  2026-02-26  5:26 ` [PATCH 2/2] sched_ext: Fix TOCTOU on p->scx.dsq in scx_dump_task() David Carlier
  2026-02-27 18:31 ` [PATCH 1/2] sched_ext: Use rcu_dereference() for scx_root in dump paths Tejun Heo
  0 siblings, 2 replies; 6+ messages in thread
From: David Carlier @ 2026-02-26  5:26 UTC (permalink / raw)
  To: Tejun Heo, David Vernet; +Cc: linux-kernel, David Carlier

scx_dump_task() and scx_dump_state() read scx_root directly without
rcu_dereference() or NULL check. If the BPF scheduler is torn down
concurrently, scx_root can become NULL between the read and the
dereference in SCX_HAS_OP(), causing a NULL pointer dereference.

Use rcu_dereference() to properly access scx_root under RCU protection
and bail out early if it is NULL.

Signed-off-by: David Carlier <devnexen@gmail.com>
---
 kernel/sched/ext.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 9280381f8923..eb539b671c49 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -4580,7 +4580,19 @@ static void scx_dump_task(struct seq_buf *s, struct scx_dump_ctx *dctx,
 			  struct task_struct *p, char marker)
 {
 	static unsigned long bt[SCX_EXIT_BT_LEN];
-	struct scx_sched *sch = scx_root;
+	struct scx_sched *sch;
+
+	/*
+	 * The BPF scheduler can be torn down concurrently
+	 */
+	rcu_read_lock();
+	sch = rcu_dereference(scx_root);
+	if (!sch) {
+		rcu_read_unlock();
+		return;
+	}
+	rcu_read_unlock();
+
 	char dsq_id_buf[19] = "(n/a)";
 	unsigned long ops_state = atomic_long_read(&p->scx.ops_state);
 	unsigned int bt_len = 0;
@@ -4623,7 +4635,19 @@ static void scx_dump_state(struct scx_exit_info *ei, size_t dump_len)
 {
 	static DEFINE_SPINLOCK(dump_lock);
 	static const char trunc_marker[] = "\n\n~~~~ TRUNCATED ~~~~\n";
-	struct scx_sched *sch = scx_root;
+	struct scx_sched *sch;
+
+	/*
+	 * The BPF scheduler can be torn down concurrently
+	 */
+	rcu_read_lock();
+	sch = rcu_dereference(scx_root);
+	if (!sch) {
+		rcu_read_unlock();
+		return;
+	}
+	rcu_read_unlock();
+
 	struct scx_dump_ctx dctx = {
 		.kind = ei->kind,
 		.exit_code = ei->exit_code,
-- 
2.51.0


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

* [PATCH 2/2] sched_ext: Fix TOCTOU on p->scx.dsq in scx_dump_task()
  2026-02-26  5:26 [PATCH 1/2] sched_ext: Use rcu_dereference() for scx_root in dump paths David Carlier
@ 2026-02-26  5:26 ` David Carlier
  2026-02-27 18:33   ` Tejun Heo
  2026-02-27 18:31 ` [PATCH 1/2] sched_ext: Use rcu_dereference() for scx_root in dump paths Tejun Heo
  1 sibling, 1 reply; 6+ messages in thread
From: David Carlier @ 2026-02-26  5:26 UTC (permalink / raw)
  To: Tejun Heo, David Vernet; +Cc: linux-kernel, David Carlier

p->scx.dsq is checked for NULL then dereferenced without
synchronization. Another CPU can NULL the pointer between the check
and the use. Use READ_ONCE() to capture the pointer into a local
variable before dereferencing.

Signed-off-by: David Carlier <devnexen@gmail.com>
---
 kernel/sched/ext.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index eb539b671c49..444398f3686a 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -4597,9 +4597,11 @@ static void scx_dump_task(struct seq_buf *s, struct scx_dump_ctx *dctx,
 	unsigned long ops_state = atomic_long_read(&p->scx.ops_state);
 	unsigned int bt_len = 0;
 
-	if (p->scx.dsq)
+	struct scx_dispatch_q *dsq = READ_ONCE(p->scx.dsq);
+
+	if (dsq)
 		scnprintf(dsq_id_buf, sizeof(dsq_id_buf), "0x%llx",
-			  (unsigned long long)p->scx.dsq->id);
+			  (unsigned long long)dsq->id);
 
 	dump_newline(s);
 	dump_line(s, " %c%c %s[%d] %+ldms",
-- 
2.51.0


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

* Re: [PATCH 1/2] sched_ext: Use rcu_dereference() for scx_root in dump paths
  2026-02-26  5:26 [PATCH 1/2] sched_ext: Use rcu_dereference() for scx_root in dump paths David Carlier
  2026-02-26  5:26 ` [PATCH 2/2] sched_ext: Fix TOCTOU on p->scx.dsq in scx_dump_task() David Carlier
@ 2026-02-27 18:31 ` Tejun Heo
  2026-02-27 19:04   ` David CARLIER
  1 sibling, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2026-02-27 18:31 UTC (permalink / raw)
  To: David Carlier; +Cc: David Vernet, linux-kernel

Hello,

On Thu, Feb 26, 2026 at 05:26:39AM +0000, David Carlier wrote:
> scx_dump_task() and scx_dump_state() read scx_root directly without
> rcu_dereference() or NULL check. If the BPF scheduler is torn down
> concurrently, scx_root can become NULL between the read and the
> dereference in SCX_HAS_OP(), causing a NULL pointer dereference.

scx_dump_state() is called from scx_error_irq_workfn() and
sysrq_handle_sched_ext_dump(). SCX can't turn off before dump is complete in
the former case. In the latter, scx_enabled() gates the call and it's in the
irq context. When scx_enabled() turns off, there's synchronize_rcu() call
afterwards before anything happens to scx_root. ie. It cannot go away in
flight. This is the same synchronization that protect other in-flight sched
ops.

Thanks.

-- 
tejun

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

* Re: [PATCH 2/2] sched_ext: Fix TOCTOU on p->scx.dsq in scx_dump_task()
  2026-02-26  5:26 ` [PATCH 2/2] sched_ext: Fix TOCTOU on p->scx.dsq in scx_dump_task() David Carlier
@ 2026-02-27 18:33   ` Tejun Heo
  2026-02-27 18:41     ` David CARLIER
  0 siblings, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2026-02-27 18:33 UTC (permalink / raw)
  To: David Carlier; +Cc: David Vernet, linux-kernel

On Thu, Feb 26, 2026 at 05:26:40AM +0000, David Carlier wrote:
> p->scx.dsq is checked for NULL then dereferenced without
> synchronization. Another CPU can NULL the pointer between the check
> and the use. Use READ_ONCE() to capture the pointer into a local
> variable before dereferencing.

Aren't we holding rq lock for the task? David, are these patches AI
generated? I'm not against AI generated patches but a human gotta verify it
end-to-end.

Thanks.

-- 
tejun

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

* Re: [PATCH 2/2] sched_ext: Fix TOCTOU on p->scx.dsq in scx_dump_task()
  2026-02-27 18:33   ` Tejun Heo
@ 2026-02-27 18:41     ` David CARLIER
  0 siblings, 0 replies; 6+ messages in thread
From: David CARLIER @ 2026-02-27 18:41 UTC (permalink / raw)
  To: Tejun Heo; +Cc: David Vernet, linux-kernel

Hi,

On Fri, 27 Feb 2026 at 18:33, Tejun Heo <tj@kernel.org> wrote:
>
> On Thu, Feb 26, 2026 at 05:26:40AM +0000, David Carlier wrote:
> > p->scx.dsq is checked for NULL then dereferenced without
> > synchronization. Another CPU can NULL the pointer between the check
> > and the use. Use READ_ONCE() to capture the pointer into a local
> > variable before dereferencing.
>
> Aren't we holding rq lock for the task? David, are these patches AI
> generated? I'm not against AI generated patches but a human gotta verify it
> end-to-end.

I used AI only to learn quicker the kernel internals as I never
contributed to it before.
So I double-checked and you are right, sorry for the noise.

Cheers !
>
> Thanks.
>
> --
> tejun

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

* Re: [PATCH 1/2] sched_ext: Use rcu_dereference() for scx_root in dump paths
  2026-02-27 18:31 ` [PATCH 1/2] sched_ext: Use rcu_dereference() for scx_root in dump paths Tejun Heo
@ 2026-02-27 19:04   ` David CARLIER
  0 siblings, 0 replies; 6+ messages in thread
From: David CARLIER @ 2026-02-27 19:04 UTC (permalink / raw)
  To: Tejun Heo; +Cc: David Vernet, linux-kernel

Hi

On Fri, 27 Feb 2026 at 18:31, Tejun Heo <tj@kernel.org> wrote:
>
> Hello,
>
> On Thu, Feb 26, 2026 at 05:26:39AM +0000, David Carlier wrote:
> > scx_dump_task() and scx_dump_state() read scx_root directly without
> > rcu_dereference() or NULL check. If the BPF scheduler is torn down
> > concurrently, scx_root can become NULL between the read and the
> > dereference in SCX_HAS_OP(), causing a NULL pointer dereference.
>
> scx_dump_state() is called from scx_error_irq_workfn() and
> sysrq_handle_sched_ext_dump(). SCX can't turn off before dump is complete in
> the former case. In the latter, scx_enabled() gates the call and it's in the
> irq context. When scx_enabled() turns off, there's synchronize_rcu() call
> afterwards before anything happens to scx_root. ie. It cannot go away in
> flight. This is the same synchronization that protect other in-flight sched
> ops.

Makes sense, the existing synchronization already covers it.
  Sorry for the noise.

  Cheers !

>
> Thanks.
>
> --
> tejun

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

end of thread, other threads:[~2026-02-27 19:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-26  5:26 [PATCH 1/2] sched_ext: Use rcu_dereference() for scx_root in dump paths David Carlier
2026-02-26  5:26 ` [PATCH 2/2] sched_ext: Fix TOCTOU on p->scx.dsq in scx_dump_task() David Carlier
2026-02-27 18:33   ` Tejun Heo
2026-02-27 18:41     ` David CARLIER
2026-02-27 18:31 ` [PATCH 1/2] sched_ext: Use rcu_dereference() for scx_root in dump paths Tejun Heo
2026-02-27 19:04   ` David CARLIER

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