Re: [PATCH] sched_ext: Validate cid override before updating tables
zhidao su <[email protected]> Tue, 14 Jul 2026 11:36:24 +0800
| Newsgroups | dev.linux.lists.sched-ext,dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
Thanks for the review. The current in-tree caller passes a BPF .bss array populated by userspacebefore SCX_OPS_LOAD, and doesn't modify it afterwards. The kfunc is alsoonly callable from the root scheduler's ops.init(), so the mapping isexpected to stay stable for the duration of the call. If we later allow a different input source or concurrent mutation duringthe call, we'd need to cache the validated values before updating thetables. I don't think that's needed for the current contract, > 2026年7月14日 10:55,[email protected] 写道: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] Splitting the `cpu_to_cid` array validation and update into two loops introduces a critical double-fetch TOCTOU vulnerability, allowing an out-of-bounds kernel memory write. > -- > > --- Patch [1]: [PATCH] sched_ext: Validate cid override before updating tables --- > commit 3a1a6d6109297fa0a7f1d91d2dccc68df1527d12 > Author: zhidao su (Xiaomi) <[email protected]> > > sched_ext: Validate cid override before updating tables > > This patch splits the validation and table updates in scx_bpf_cid_override() > into separate loops. This prevents partial table updates if the validation > fails midway through the user-supplied mapping. > >> diff --git a/kernel/sched/ext/cid.c b/kernel/sched/ext/cid.c >> index af83084ec740a..e398e3f209bd4 100644 >> --- a/kernel/sched/ext/cid.c >> +++ b/kernel/sched/ext/cid.c >> @@ -329,6 +329,11 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid, u32 cpu_to_cid__sz, >> scx_error(sch, "cid %d assigned to multiple cpus", c); >> return; >> } >> + } >> + >> + for_each_possible_cpu(cpu) { >> + s32 c = cpu_to_cid[cpu]; >> + >> scx_cpu_to_cid_tbl[cpu] = c; >> scx_cid_to_cpu_tbl[c] = cpu; >> } > > [Severity: High] > Can the contents of cpu_to_cid change concurrently between these two loops in > scx_bpf_cid_override()? > > If cpu_to_cid points to shared memory, such as a BPF map, could another thread > or BPF program modify cpu_to_cid[cpu] after it passes the validation in the > first loop? > > If this happens, the second loop would fetch the unvalidated value and use it > as an index for scx_cid_to_cpu_tbl[c], potentially resulting in an > out-of-bounds write. > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1