Re: [PATCH sched_ext/for-7.3 30/32] sched_ext: Add scx_bpf_sub_kill() to evict a child sub-scheduler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Out-of-bounds read due to missing `__str` suffix on the `fmt` argument in `scx_bpf_sub_kill_bstr`.
--

commit dae92dbfa20b120640a7210997136f2e4d4af380
Author: Tejun Heo <tj@kernel.org>

sched_ext: Add scx_bpf_sub_kill() to evict a child sub-scheduler

This commit adds scx_bpf_sub_kill() to allow a cid-form scheduler to evict
a direct child sub-scheduler. It exposes BPF string formatting infrastructure
and introduces a new exit kind, SCX_EXIT_PARENT_KILL, to handle the teardown.

> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 15edcf4f81ee2..4baa6f7be71bd 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
[ ... ]
> @@ -1636,6 +1636,63 @@ __bpf_kfunc s32 scx_bpf_sub_caps(u64 cgroup_id, u64 caps, struct scx_cmask *out_
>  	return 0;
>  }
>  
> +/**
> + * scx_bpf_sub_kill_bstr - Kill a direct child sub-scheduler
> + * @cgroup_id: cgroup id of the direct child to kill
> + * @fmt: reason message format string
> + * @data: format string parameters packaged using ___bpf_fill() macro
> + * @data__sz: @data len, must end in '__sz' for the verifier
> + * @aux: implicit BPF argument to access bpf_prog_aux hidden from BPF progs
[ ... ]
> + */
> +__printf(2, 0)
> +__bpf_kfunc s32 scx_bpf_sub_kill_bstr(u64 cgroup_id, char *fmt,
> +				      unsigned long long *data, u32 data__sz,
> +				      const struct bpf_prog_aux *aux)

[Severity: High]
Does this kfunc definition miss the __str suffix on the fmt parameter?

Because the argument is named fmt instead of fmt__str, the BPF verifier
treats it as a standard memory pointer (KF_ARG_PTR_TO_MEM) with a size of
1 byte (the size of char), rather than as a null-terminated string.

If a BPF program passes a pointer to a 1-byte map value without a null
terminator, the verifier will allow it. Later, this pointer is passed
through scx_bstr_format() to bpf_bprintf_prepare(), which scans for a null
terminator up to UINT_MAX:

kernel/sched/ext/ext.c:__bstr_format() {
    ...
    ret = bpf_bprintf_prepare(fmt, UINT_MAX, data_buf, data__sz / 8,
                              &bprintf_data);
    ...
}

Could this lead to an out-of-bounds read in the kernel if
bpf_bprintf_prepare() scans beyond the 1-byte verified bounds?

> +{
> +	struct scx_sched *parent, *child;
> +	s32 ret;
> +
> +	guard(rcu)();
> +
> +	parent = scx_prog_sched(aux);
> +	if (unlikely(!parent))
> +		return -ENODEV;
> +
[ ... ]

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