[PATCH sched_ext/for-7.3-fixes] sched_ext: Add a size argument to scx_bpf_cid_topo() so struct scx_cid_topo can grow

[PATCH sched_ext/for-7.3-fixes] sched_ext: Add a size argument to scx_bpf_cid_topo() so struct scx_cid_topo can grow

[Tejun Heo]

scx_bpf_cid_topo() copies struct scx_cid_topo into a buffer the BPF program
sized from its own vmlinux.h while the verifier sizes the write from the
running kernel's BTF. The struct may grow and each growth then breaks every
scheduler built against the older layout, rejected at load or written past
its buffer. This is the usual hole for a struct handed to BPF, closed
elsewhere with a size argument, and it was missed here.

Take the buffer size, copy the smaller of it and the kernel's struct and set
the rest to -1. Accesses to the copy are CO-RE relocated, so the struct can
grow by appending fields, which its comment now states. The kfunc changes in
place: the cid interface is still being finalized and no released scheduler
uses the current form.

Fixes: e9b55af47edf ("sched_ext: Add topological CPU IDs (cids)")
Cc: stable@vger.kernel.org # v7.2+
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext/cid.c                   |   26 ++++++++++++++++----------
 kernel/sched/ext/types.h                 |    4 ++++
 tools/sched_ext/include/scx/common.bpf.h |    2 +-
 3 files changed, 21 insertions(+), 11 deletions(-)

--- a/kernel/sched/ext/cid.c
+++ b/kernel/sched/ext/cid.c
@@ -912,30 +912,36 @@ bool scx_cmask_empty(const struct scx_cm
 /**
  * scx_bpf_cid_topo - Copy out per-cid topology info
  * @cid: cid to look up
- * @out__uninit: where to copy the topology info; fully written by this call
+ * @out: where to copy the topology info
+ * @out__sz: size of @out, the program's sizeof(struct scx_cid_topo)
  * @aux: implicit BPF argument to access bpf_prog_aux hidden from BPF progs
  *
- * Fill @out__uninit with the topology info for @cid. Trigger scx_error() if
- * @cid is out of range. If @cid is valid but in the no-topo section, all fields
- * are set to -1. All fields are also set to -1 when no cid tables have been
- * published yet, which a program may observe while racing the root enable.
+ * Fill @out with the topology info for @cid. Trigger scx_error() if @cid is out
+ * of range. If @cid is valid but in the no-topo section, all fields are set to
+ * -1. All fields are also set to -1 when no cid tables have been published yet,
+ * which a program may observe while racing the root enable.
+ *
+ * The program's struct may be older or newer than the kernel's. The smaller of
+ * @out__sz and the kernel's size is copied and the rest of @out is set to -1.
  */
-__bpf_kfunc void scx_bpf_cid_topo(s32 cid, struct scx_cid_topo *out__uninit,
+__bpf_kfunc void scx_bpf_cid_topo(s32 cid, struct scx_cid_topo *out, size_t out__sz,
 				  const struct bpf_prog_aux *aux)
 {
+	size_t len = min(out__sz, sizeof(*out));
 	struct scx_cid_topo *topo;
 	struct scx_sched *sch;
 
+	/* the error cases and fields the kernel lacks read as -1 */
+	memset(out, 0xff, out__sz);
+
 	guard(rcu)();
 
 	sch = scx_prog_sched(aux);
 	topo = rcu_dereference(scx_cid_topo);
-	if (unlikely(!sch) || !cid_valid(sch, cid) || unlikely(!topo)) {
-		*out__uninit = SCX_CID_TOPO_NEG;
+	if (unlikely(!sch) || !cid_valid(sch, cid) || unlikely(!topo))
 		return;
-	}
 
-	*out__uninit = topo[cid];
+	memcpy(out, &topo[cid], len);
 }
 
 __bpf_kfunc_end_defs();
--- a/kernel/sched/ext/types.h
+++ b/kernel/sched/ext/types.h
@@ -70,6 +70,10 @@ enum scx_consts {
  * smaller shards if the LLC exceeds the target size. No-topo cids are packed
  * into their own max-sized shards.
  *
+ * New fields are appended, never inserted: scx_bpf_cid_topo() copies this
+ * struct out sized by the program's own layout, and an older program's copy
+ * must stay a prefix of the kernel's.
+ *
  * @core_cid: first cid of this cid's core (smt-sibling group)
  * @core_idx: global index of that core, in [0, nr_cores_at_init)
  * @llc_cid: first cid of this cid's LLC
--- a/tools/sched_ext/include/scx/common.bpf.h
+++ b/tools/sched_ext/include/scx/common.bpf.h
@@ -106,7 +106,7 @@ u64 scx_bpf_now(void) __ksym __weak;
 void scx_bpf_events(struct scx_event_stats *events, size_t events__sz) __ksym __weak;
 s32 scx_bpf_cpu_to_cid(s32 cpu) __ksym __weak;
 s32 scx_bpf_cid_to_cpu(s32 cid) __ksym __weak;
-void scx_bpf_cid_topo(s32 cid, struct scx_cid_topo *out) __ksym __weak;
+void scx_bpf_cid_topo(s32 cid, struct scx_cid_topo *out, size_t out__sz) __ksym __weak;
 void scx_bpf_kick_cid(s32 cid, u64 flags) __ksym __weak;
 s32 scx_bpf_task_cid(const struct task_struct *p) __ksym __weak;
 s32 scx_bpf_this_cid(void) __ksym __weak;

Re: [PATCH sched_ext/for-7.3-fixes] sched_ext: Add a size argument to scx_bpf_cid_topo() so struct scx_cid_topo can grow

[Andrea Righi]

Hi Tejun,

On Sat, Sep 26, 2026 at 09:23:02AM -1000, Tejun Heo wrote:
> scx_bpf_cid_topo() copies struct scx_cid_topo into a buffer the BPF program
> sized from its own vmlinux.h while the verifier sizes the write from the
> running kernel's BTF. The struct may grow and each growth then breaks every
> scheduler built against the older layout, rejected at load or written past
> its buffer. This is the usual hole for a struct handed to BPF, closed
> elsewhere with a size argument, and it was missed here.
> 
> Take the buffer size, copy the smaller of it and the kernel's struct and set
> the rest to -1. Accesses to the copy are CO-RE relocated, so the struct can
> grow by appending fields, which its comment now states. The kfunc changes in
> place: the cid interface is still being finalized and no released scheduler
> uses the current form.
> 
> Fixes: e9b55af47edf ("sched_ext: Add topological CPU IDs (cids)")
> Cc: stable@vger.kernel.org # v7.2+
> Signed-off-by: Tejun Heo <tj@kernel.org>

Makes sense to me.

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Thanks,
-Andrea

> ---
>  kernel/sched/ext/cid.c                   |   26 ++++++++++++++++----------
>  kernel/sched/ext/types.h                 |    4 ++++
>  tools/sched_ext/include/scx/common.bpf.h |    2 +-
>  3 files changed, 21 insertions(+), 11 deletions(-)
> 
> --- a/kernel/sched/ext/cid.c
> +++ b/kernel/sched/ext/cid.c
> @@ -912,30 +912,36 @@ bool scx_cmask_empty(const struct scx_cm
>  /**
>   * scx_bpf_cid_topo - Copy out per-cid topology info
>   * @cid: cid to look up
> - * @out__uninit: where to copy the topology info; fully written by this call
> + * @out: where to copy the topology info
> + * @out__sz: size of @out, the program's sizeof(struct scx_cid_topo)
>   * @aux: implicit BPF argument to access bpf_prog_aux hidden from BPF progs
>   *
> - * Fill @out__uninit with the topology info for @cid. Trigger scx_error() if
> - * @cid is out of range. If @cid is valid but in the no-topo section, all fields
> - * are set to -1. All fields are also set to -1 when no cid tables have been
> - * published yet, which a program may observe while racing the root enable.
> + * Fill @out with the topology info for @cid. Trigger scx_error() if @cid is out
> + * of range. If @cid is valid but in the no-topo section, all fields are set to
> + * -1. All fields are also set to -1 when no cid tables have been published yet,
> + * which a program may observe while racing the root enable.
> + *
> + * The program's struct may be older or newer than the kernel's. The smaller of
> + * @out__sz and the kernel's size is copied and the rest of @out is set to -1.
>   */
> -__bpf_kfunc void scx_bpf_cid_topo(s32 cid, struct scx_cid_topo *out__uninit,
> +__bpf_kfunc void scx_bpf_cid_topo(s32 cid, struct scx_cid_topo *out, size_t out__sz,
>  				  const struct bpf_prog_aux *aux)
>  {
> +	size_t len = min(out__sz, sizeof(*out));
>  	struct scx_cid_topo *topo;
>  	struct scx_sched *sch;
>  
> +	/* the error cases and fields the kernel lacks read as -1 */
> +	memset(out, 0xff, out__sz);
> +
>  	guard(rcu)();
>  
>  	sch = scx_prog_sched(aux);
>  	topo = rcu_dereference(scx_cid_topo);
> -	if (unlikely(!sch) || !cid_valid(sch, cid) || unlikely(!topo)) {
> -		*out__uninit = SCX_CID_TOPO_NEG;
> +	if (unlikely(!sch) || !cid_valid(sch, cid) || unlikely(!topo))
>  		return;
> -	}
>  
> -	*out__uninit = topo[cid];
> +	memcpy(out, &topo[cid], len);
>  }
>  
>  __bpf_kfunc_end_defs();
> --- a/kernel/sched/ext/types.h
> +++ b/kernel/sched/ext/types.h
> @@ -70,6 +70,10 @@ enum scx_consts {
>   * smaller shards if the LLC exceeds the target size. No-topo cids are packed
>   * into their own max-sized shards.
>   *
> + * New fields are appended, never inserted: scx_bpf_cid_topo() copies this
> + * struct out sized by the program's own layout, and an older program's copy
> + * must stay a prefix of the kernel's.
> + *
>   * @core_cid: first cid of this cid's core (smt-sibling group)
>   * @core_idx: global index of that core, in [0, nr_cores_at_init)
>   * @llc_cid: first cid of this cid's LLC
> --- a/tools/sched_ext/include/scx/common.bpf.h
> +++ b/tools/sched_ext/include/scx/common.bpf.h
> @@ -106,7 +106,7 @@ u64 scx_bpf_now(void) __ksym __weak;
>  void scx_bpf_events(struct scx_event_stats *events, size_t events__sz) __ksym __weak;
>  s32 scx_bpf_cpu_to_cid(s32 cpu) __ksym __weak;
>  s32 scx_bpf_cid_to_cpu(s32 cid) __ksym __weak;
> -void scx_bpf_cid_topo(s32 cid, struct scx_cid_topo *out) __ksym __weak;
> +void scx_bpf_cid_topo(s32 cid, struct scx_cid_topo *out, size_t out__sz) __ksym __weak;
>  void scx_bpf_kick_cid(s32 cid, u64 flags) __ksym __weak;
>  s32 scx_bpf_task_cid(const struct task_struct *p) __ksym __weak;
>  s32 scx_bpf_this_cid(void) __ksym __weak;

Re: [PATCH sched_ext/for-7.3-fixes] sched_ext: Add a size argument to scx_bpf_cid_topo() so struct scx_cid_topo can grow

[Tejun Heo]

Applied to sched_ext/for-7.3-fixes with the following tag added:

 Reviewed-by: Andrea Righi <arighi@nvidia.com>

Thanks.

-- 
tejun