sched_ext: find_parent_sched() 空指针检查补丁与其可达性之争

一、基线信息

项目	内容
线程标题	`[PATCH] sched_ext: Fix NULL dereference in find_parent_sched()`
后续版本标题	`[PATCH] sched_ext: Add NULL check in find_parent_sched()`
作者	Cui Jian `<cjian720@163.com>`
评审者	Zhan Xusheng `<zhanxusheng@xiaomi.com>`
版本	无显式 vN 标记，实际为「原始版 → 改写 changelog 版」两轮
规模	1 个文件 `kernel/sched/ext/sub.c`，+4 行
Message-ID	`20260807093838.56002-1-cjian720@163.com`
来源	sched-ext 邮件列表
邮件数	3（patch → review → 重发 patch）

二、明确目的

find_parent_sched() 是 sched_ext「子调度器（sub-sched）」机制里的一个辅助函数：给定一个 cgroup，往上找到承载它的父 scx_sched 实例。

原始补丁的目的：

该函数直接解引用 cgrp->scx_sched（准确说是取到 parent 后立刻访问 parent->cgrp），没有 NULL 检查。
作者声称：BPF 程序把一个 cgroup v1 的 ID 当作 sub_cgroup_id 传进来，经由 cgroup_get_from_id() 拿到一个 v1 cgroup，而 v1 cgroup 从不设置 scx_sched，于是 parent == NULL，内核崩溃。
修复方式极简：if (!parent) return ERR_PTR(-ENODEV);

评审后目的发生了性质变化：代码一行没改，但 changelog 从「修复一个可从 BPF 触发的内核崩溃」降级为「实践中安全，加一个防御性检查提升健壮性」。也就是说，这条线程真正的产出不是代码，而是对「这个 bug 是否可达」的结论修正。

三、遍历代码

3.1 补丁本体

static struct scx_sched *find_parent_sched(struct cgroup *cgrp)
{
	/* ... 取得 parent ... */

	/* no SCX sched */
	if (!parent)
		return ERR_PTR(-ENODEV);

	lockdep_assert_held(&scx_sched_lock);

	/* can't attach twice to the same cgroup */
	if (parent->cgrp == cgrp)
		return ERR_PTR(-EBUSY);
	/* ... */
}

关键点：崩溃点是 parent->cgrp == cgrp 这一句。新增的 NULL 检查插在它之前，返回 ERR_PTR(-ENODEV)，与函数原有的 ERR_PTR(-EBUSY) 错误返回风格保持一致，调用方用 IS_ERR() 即可处理，不需要改动。

3.2 Zhan Xusheng 的反驳路径

评审意见逐层拆掉了原 changelog 的因果链：

第一层：v1 ID 根本查不到。

kn = kernfs_find_and_get_node_by_id(cgrp_dfl_root.kf_root, id);

cgroup_get_from_id() 只在 cgroup v2 默认层级（cgrp_dfl_root） 的 kernfs root 里查找 ID。传入一个 v1 的 ID，在这一步就返回 -ENOENT。

第二层：即使查到了也会被拒。

cgroup_get_from_id() 后续用 cgroup_is_descendant() 校验，而该函数第一件事就是比较 cgrp->root != ancestor->root——跨层级直接失败。

第三层：语义上 ->scx_sched 本来就是 v2-only。

scx_cgroup_lifetime_notify() 在 !cgroup_on_dfl(cgrp) 时直接 bail out，所以 v1 cgroup 永远不会被赋值 scx_sched，也永远走不到 find_parent_sched()。

第四层：有没有别的窗口？ 评审者主动自查了其他可能路径，结论是没找到：

sub-enable 在 scx_enable_mutex 下检查 scx_enabled()；
root enable 会给每一个存活的 v2 后代设置 ->scx_sched；
之后新建的 cgroup 在 ONLINE notifier 里继承；
全量扫描（sweep）和 notifier 都在 cgroup_mutex 下运行，不存在竞态窗口。

第五层：诚实的免责声明。 评审者说明自己读的是 ext.c，因为 sub.c 尚未进入 upstream，所以拆分过程中可能引入了他看不到的东西——但 cgroup_get_from_id() 那部分论证与拆分无关，是独立成立的。

结论： 加检查本身无害（harmless hardening），但 changelog 写成「可达的崩溃」是错的，而这种措辞会传播到 stable backport 决策和 CVE 评级里去。

3.3 第三封：作者接受并重发

作者没有争辩，直接：

标题 Fix NULL dereference → Add NULL check；
正文改为「实践中是安全的，因为能到达此函数的 cgroup 一定设置了 scx_sched，但为健壮性加一个防御性检查」；
diff 完全不变。

四、ASCII 流程图

4.1 声称的崩溃路径 vs 实际被拦截的位置

  BPF prog: sub_cgroup_id = <v1 cgroup id>
                |
                v
  +-------------------------------------------------+
  | cgroup_get_from_id(id)                          |
  |                                                 |
  |  kernfs_find_and_get_node_by_id(               |
  |         cgrp_dfl_root.kf_root, id)  <--[拦截1]--+---> -ENOENT (v1 id 不在 v2 root)
  |                                                 |
  |  cgroup_is_descendant():                        |
  |         cgrp->root != ancestor->root <--[拦截2]-+---> fail (跨层级)
  +-------------------------------------------------+
                |
                | (v1 永远到不了这里)
                v
  +-------------------------------------------------+
  | find_parent_sched(cgrp)                         |
  |   parent = cgrp->scx_sched   /* 声称为 NULL */  |
  |   [新增] if (!parent) return ERR_PTR(-ENODEV);  |
  |   parent->cgrp == cgrp ?     /* 声称崩溃点 */   |
  +-------------------------------------------------+

4.2 `->scx_sched` 的赋值来源（为什么 v2 cgroup 总是有值）

   scx_cgroup_lifetime_notify(cgrp)
        |
        +-- !cgroup_on_dfl(cgrp) ? --> return   (v1 直接出局)
        |
        v
   [root enable]  --> 遍历所有存活 v2 后代, 逐个设置 ->scx_sched
   [ONLINE 通知]  --> 之后新建的 cgroup 继承 ->scx_sched
        |
        +----> sweep 与 notifier 均持有 cgroup_mutex
               => 无竞态窗口 => v2 cgroup 的 ->scx_sched 恒非 NULL

4.3 评审导致的状态迁移

  v1 changelog                        v2 changelog
  ------------                        ------------
  "Fix NULL dereference"    --review--> "Add NULL check"
  "would crash the kernel"             "safe in practice"
  "BPF passes v1 cgroup id"            "defensive check for robustness"
        |                                     |
        v                                     v
   触发 stable backport / CVE           普通 hardening 补丁
   [代码 diff: 完全相同, 一字未改]

五、概念类比

类比一：给一扇通不到的门加锁。

想象一栋只有刷卡才能进的写字楼。有人提交报告说：「拿地铁卡（v1 ID）刷公司门禁（v2 lookup）能直接进来，安全事故！」评审者去实地验证：地铁卡在读卡器上根本不识别（kernfs_find_and_get_node_by_id 返回 -ENOENT），就算识别了，闸机还会二次校验发卡机构（cgrp->root != ancestor->root）。所以「地铁卡能闯门禁」不成立。给门再加一把锁没坏处，但报告标题不能写成「已发生入侵事件」——因为保安部门会据此启动全楼应急预案（stable backport + CVE）。

类比二：体检报告的措辞。

同一个「加一道保险」的动作，写成「修复致命缺陷」还是「预防性加固」，会决定医院是不是要连夜召回所有同批病人。代码是同一片药，标签不同，后果差了几个数量级。

六、Highlight 突出问题

最核心的争议：changelog 措辞的下游代价。 Linux 的 stable 规则和 CVE 分配高度依赖 commit message 语义。写「Fix NULL dereference ... crash the kernel」会自动吸引 AUTOSEL 机器人和 CVE 编号，即使代码只是防御性加固。评审者点出这一点，是整条线程最有价值的部分。
原 changelog 的事实错误未被完全承认。 作者第二版只写「safe in practice」，但没有说明为什么安全（即 cgroup_get_from_id() 只查 v2 root）。审阅者提供的三层论证没有被写进 changelog，未来读者仍然会疑惑「为什么安全」。理想做法是把 Zhan 的分析摘要进 commit body。
sub.c 尚未 upstream，评审存在盲区。 评审者明确说自己读的是 ext.c，无法看到 sub.c 拆分后的实际代码。也就是说，「没有其他窗口」这个结论是基于旧代码得出的——若 sub-sched 拆分引入了新的调用路径（例如不经过 cgroup_get_from_id() 的入口），NULL 仍可能出现。这是需要维护者（Tejun / David）确认的点。
重发方式不规范。 第三封是新 Message-ID 的独立 patch，没有 v2 标记、没有 In-Reply-To 挂到原线程、没有 Changes since v1 段落，也没有给 Zhan Xusheng 的 Reported-by: / Suggested-by: 之类致谢。这在 kernel 流程上属于不合规，容易被 patchwork 当成两个独立补丁。
-ENODEV 的选择值得商榷。 若这条路径在实践中不可达，返回码几乎不会被观测到；但若将来变得可达，-ENODEV（无此设备）在语义上是否比 -ENOENT / -EINVAL 更合适，没人讨论过。
防御性检查也可能是负担。 内核社区对「不可达路径加 NULL 检查」向来态度分裂：一派认为无害且抗未来重构，一派认为会掩盖真正的不变量、让静态分析和读者误以为 NULL 是合法状态。这条补丁最终是否被接受，取决于维护者站哪一边——线程中维护者尚未表态。

七、版本演进

版本	标题	代码 diff	changelog 主张
初版	`Fix NULL dereference in find_parent_sched()`	+4 行 NULL 检查	BPF 传入 v1 cgroup ID → `parent` 为 NULL → 内核崩溃（可达 bug）
重发	`Add NULL check in find_parent_sched()`	完全相同	实践中安全，仅为健壮性添加防御检查（hardening）

关键变化：零代码改动，纯粹是 bug 性质定级的回退。这在 kernel 邮件列表上是相对少见但很健康的一类演进——评审阻止了一个错误的「fix」标签进入 git 历史。

八、与相关工作的关联

sched_ext sub-scheduler（kernel/sched/ext/sub.c）：本补丁所在文件是 sched_ext 层级化子调度器工作的一部分，尚未合入 upstream。find_parent_sched() 服务于「把一个 BPF 调度器 attach 到某个 cgroup 子树」的场景，-EBUSY（同一 cgroup 重复 attach）与新增的 -ENODEV 是同一族错误返回。
cgroup v1/v2 双层级基础设施：论证核心依赖 cgrp_dfl_root、cgroup_is_descendant()、cgroup_on_dfl() 这些 cgroup 核心 API 的行为，属于 cgroup 子系统与 sched_ext 的接口面。
scx_cgroup_lifetime_notify()：->scx_sched 的唯一赋值通道，是判断该指针不变量的关键。

九、一句话总结

一个 4 行的 NULL 检查补丁，被评审者用三层 cgroup v1/v2 层级论证证明其声称的崩溃路径不可达，作者随即重发并把标题从「Fix NULL dereference」改为「Add NULL check」——代码一字未改，改的是会影响 stable backport 与 CVE 判定的 changelog 定性。

[PATCH] sched_ext: Fix NULL dereference in find_parent_sched()

LLM 分析