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 <arighi@nvidia.com>]

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;