Re: [PATCH sched_ext/for-7.3-fixes] sched_ext: Pass the initial cmask to cid-form ops.enable()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] sched_ext: Type confusion in scx_enable_args.cmask causes kernel panic
--

commit 68c6e46ced10fbf4dca95b3687b46574af4872a5
Author: Tejun Heo <tj@kernel.org>
Subject: sched_ext: Pass the initial cmask to cid-form ops.enable()

This commit adds `struct scx_enable_args` to pass the initial `cmask` to
cid-form `ops.enable()` callbacks. This avoids relying on `init_task()` to
seed the affinity mask, allowing schedulers to track affinity entirely within
`set_cmask()`.

> diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
> index 6d369b6487a3c..b053a11163c6f 100644
> --- a/kernel/sched/ext/internal.h
> +++ b/kernel/sched/ext/internal.h
[ ... ]
> @@ -250,6 +250,24 @@ struct scx_exit_task_args {
>  	bool cancelled;
>  };
>  
> +/**
> + * struct scx_enable_args - Argument container for cid-form ops.enable()
> + * @cmask: cids the task may run on, as a BPF arena pointer
> + *
> + * @cmask is the task's affinity as it enters the scheduler. set_cmask()
> + * delivers the same mask after enable() and before the task is first enqueued,
> + * then every affinity change afterwards, and is never called before enable(). A
> + * scheduler may therefore track affinity in set_cmask() alone.
> + *
> + * The kernel builds @cmask in the scheduler arena from its own geometry, so the
> + * header is valid regardless of what the scheduler last wrote there. The memory
> + * is per-cpu scratch reused once the callback returns: copy the bits out, don't
> + * keep the pointer. The set_cmask() argument follows the same rules.
> + */
> +struct scx_enable_args {
> +	const struct scx_cmask	*cmask;
> +};

[Severity: High]
Does this struct field need an arena BTF tag (e.g., __arena)?

Because it is currently typed as a regular kernel pointer, the verifier might
treat `args->cmask` as a regular kernel pointer (`PTR_TO_BTF_ID`) rather than
a 32-bit arena offset.

[ ... ]
> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index a7005deda6d87..99e660649d15a 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
[ ... ]
> @@ -3944,11 +3956,28 @@ static void __scx_enable_task(struct scx_sched *sch, struct task_struct *p)
>  
>  	p->scx.weight = sched_weight_to_cgroup(weight);
>  
> -	if (SCX_HAS_OP(sch, enable))
> -		SCX_CALL_OP_TASK(sch, enable, rq, p);
> +	if (SCX_HAS_OP(sch, enable)) {
> +		if (scx_is_cid_type()) {
> +			struct scx_cmask *cmask = scx_fill_cmask_scratch(sch, p->cpus_ptr);
> +			struct scx_enable_args args = {
> +				.cmask = scx_kaddr_to_arena(sch, cmask),
> +			};
> +
> +			SCX_CALL_CID_OP_TASK(sch, enable, rq, p, &args);

[Severity: High]
Could this assignment cause a kernel panic when accessed by the BPF program?

The kernel address is converted to a 32-bit arena offset via
scx_kaddr_to_arena() before being assigned to args.cmask, explicitly bypassing
the struct_ops trampoline rebasing.

If the verifier treats this field as a standard pointer (due to the missing
arena tag mentioned above), dereferencing it in the BPF program might cause
the JIT to omit the arena base addition. This would result in a raw 32-bit
memory access and trigger a kernel page fault.

Furthermore, if the BPF program attempts to cast it to an arena pointer using
bpf_addr_space_cast, the verifier would subtract the arena base a second time,
resulting in an invalid offset.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/c8343c2e457508a6cd3fcab66b0823b6@kernel.org?part=1