linux-kernel - Re: [PATCH v6 0/3] sched_ext: Harden scx_bpf_cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aLb63B1THHD-aF0W@gpd4>
Date: Tue, 2 Sep 2025 16:10:36 +0200
From: Andrea Righi <arighi@...dia.com>
To: Christian Loehle <christian.loehle@....com>
Cc: tj@...nel.org, void@...ifault.com, linux-kernel@...r.kernel.org,
	sched-ext@...ts.linux.dev, changwoo@...lia.com, hodgesd@...a.com,
	mingo@...hat.com, peterz@...radead.org, jake@...lion.co.uk
Subject: Re: [PATCH v6 0/3] sched_ext: Harden scx_bpf_cpu_rq()

On Tue, Sep 02, 2025 at 02:53:56PM +0100, Christian Loehle wrote:
> On 9/2/25 12:58, Andrea Righi wrote:
> > On Tue, Sep 02, 2025 at 12:11:40PM +0100, Christian Loehle wrote:
> >> scx_bpf_cpu_rq() currently allows accessing struct rq fields without
> >> holding the associated rq.
> >> It is being used by scx_cosmos, scx_flash, scx_lavd, scx_layered, and
> >> scx_tickless. Fortunately it is only ever used to fetch rq->curr.
> >> So provide an alternative scx_bpf_remote_curr() that doesn't expose struct rq
> >> and provide a hardened scx_bpf_cpu_rq_locked() by ensuring we hold the rq lock.
> >> Add a deprecation warning to scx_bpf_cpu_rq() that mentions the two alternatives.
> >>
> >> This also simplifies scx code from:
> >>
> >> rq = scx_bpf_cpu_rq(cpu);
> >> if (!rq)
> >> 	return;
> >> p = rq->curr
> >> /* ... Do something with p */
> >>
> >> into:
> >>
> >> p = scx_bpf_remote_curr(cpu);
> >> /* ... Do something with p */
> > 
> > This looks good to me.
> > 
> > We should probably add a __COMPAT_scx_bpf_remote_curr() macro, so that the
> > BPF schedulers can be updated to use this new kfunc without breaking the
> > compatibility with older kernels, but we can do this later, I'll send a
> > follow-up patch. For now:
> > 
> > Acked-by: Andrea Righi <arighi@...dia.com>
> 
> Thanks!
> I'd have the compat patch ready as well and would send it out in a bit.

Awesome, I was thinking about something like the following (untested).
Feel free to include this in your patch.

Thanks,
-Andrea

 tools/sched_ext/include/scx/compat.bpf.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/sched_ext/include/scx/compat.bpf.h b/tools/sched_ext/include/scx/compat.bpf.h
index 36e0cd2fd4eda..67594ff99a461 100644
--- a/tools/sched_ext/include/scx/compat.bpf.h
+++ b/tools/sched_ext/include/scx/compat.bpf.h
@@ -230,6 +230,15 @@ static inline bool __COMPAT_is_enq_cpu_selected(u64 enq_flags)
 	 scx_bpf_pick_any_cpu_node(cpus_allowed, node, flags) :			\
 	 scx_bpf_pick_any_cpu(cpus_allowed, flags))
 
+/*
+ * v6.18: Add a helper to retrieve the current task from a runqueue.
+ *
+ * Keep this macro available until v6.20 for compatibility.
+ */
+#define __COMPAT_scx_bpf_remote_curr(cpu)					\
+	(bpf_ksym_exists(scx_bpf_remote_curr) ?					\
+	 scx_bpf_remote_curr(cpu) : scx_bpf_cpu_rq(cpu)->curr)
+
 /*
  * Define sched_ext_ops. This may be expanded to define multiple variants for
  * backward compatibility. See compat.h::SCX_OPS_LOAD/ATTACH().