sched: fix ABI issues and add full sched-family test suite#970
sched: fix ABI issues and add full sched-family test suite#97004megumi wants to merge 2 commits into
Conversation
![mai-team-app[bot]](https://avatars.githubusercontent.com/u/34859362?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review：sched ABI 修复与测试套件
本 PR 将 sched_setscheduler/sched_getscheduler/sched_getparam 从原来的桩实现升级为真正读取/写入 task 调度策略和优先级的实现，新增了 SchedParam 结构体、权限检查 check_sched_permission，并在 TaskInner 中添加了 sched_policy/sched_priority 字段。同时新增了完整的 test-sched-family C 测试用例，覆盖 sched 系列系统调用的边界条件。整体方向正确，测试覆盖也较为全面。
但存在一个 阻塞性问题 需要修复后才能合并：
阻塞性问题
实时调度策略缺少 CAP_SYS_NICE 强制检查
check_sched_permission 在目标 task 为自身时直接返回 Ok(())，跳过所有权限检查。然而在 Linux 中，sched_setscheduler 设置 SCHED_FIFO 或 SCHED_RR 时，即使是自身也需要 CAP_SYS_NICE 权限（除非 RLIMIT_RTPRIO 允许，而 StarryOS 尚未实现该机制）。当前实现允许任何非特权进程将自己设置为 SCHED_FIFO 优先级 99 的实时进程，这构成权限提升风险。
建议在 sys_sched_setscheduler 中，在通过 check_sched_permission 之后、设置 policy 之前，额外检查：如果 policy 为 SCHED_FIFO 或 SCHED_RR，则要求 caller.has_cap_sys_nice() 为 true，否则返回 EPERM。
非阻塞性建议
SCHED_RESET_ON_FORK被解析但未使用：_reset_on_fork被计算但从未使用，建议至少添加// TODO注释说明。- 负 pid 返回 EINVAL 而非 ESRCH：
get_task_by_sched_pid对负 pid 返回InvalidInput（EINVAL），但 Linux 上sched_getscheduler/sched_setscheduler对负 pid 返回 ESRCH。测试用例也验证了这一不正确的行为。此为既有问题，不阻塞本次合并，但应在后续修复。 - 多个新增文件缺少末尾换行：
CMakeLists.txt、qemu-riscv64.toml、qemu-x86_64.toml、test_framework.h、main.c末尾缺少换行符。
重复/冲突分析
在 open PR 中未发现与 sched 系列系统调用重叠的其他 PR。base 分支上的 sys_sched_getscheduler 固定返回 SCHED_RR，sys_sched_setscheduler/sys_sched_getparam 为空桩，本 PR 是该区域的首次实质性实现。
验证
cargo fmt --check存在 pre-existing 的 let-chain 警告（process_cred函数），与本 PR 无关- PR 变更部分的 Rust 代码格式正确
- 新增测试位于
qemu-smp4分组下，符合项目测试组织方式 - TOML 配置的
success_regex/fail_regex设计合理
Powered by glm-5.1
| let caller = current().as_thread().cred(); | ||
| let task = get_task_by_sched_pid(pid)?; | ||
| if task.id() == current().id() { | ||
| return Ok(()); |
There was a problem hiding this comment.
Ok(())，跳过所有权限检查。
在 Linux 中，sched_setscheduler 对 SCHED_FIFO/SCHED_RR 实时调度策略要求 CAP_SYS_NICE 权限，即使目标是自身进程也不例外（除非进程拥有非零 RLIMIT_RTPRIO，而 StarryOS 尚未实现该限制）。
当前实现允许任何非特权进程通过 sched_setscheduler(0, SCHED_FIFO, &{.sched_priority=99}) 将自身提升为最高优先级实时进程，构成权限提升风险。
建议修复方向：
不在 check_sched_permission 中修改自检查逻辑（因为 setaffinity 对自身确实不需要权限），而是在 sys_sched_setscheduler 的 policy 校验阶段，对 SCHED_FIFO/SCHED_RR 额外检查 caller.has_cap_sys_nice()。
// 在 sys_sched_setscheduler 中，match policy 之后，set_sched_policy 之前：
if matches!(policy, SCHED_FIFO | SCHED_RR) && !caller.has_cap_sys_nice() {
return Err(AxError::PermissionDenied);
}| let _reset_on_fork = (policy & SCHED_RESET_ON_FORK) != 0; | ||
| policy &= !SCHED_RESET_ON_FORK; | ||
| let prio = user_param.sched_priority; | ||
| match policy { |
There was a problem hiding this comment.
建议补充注释说明 SCHED_RESET_ON_FORK 已正确解析但目前未实际使用（未记录到 task 中），例如：
// TODO: SCHED_RESET_ON_FORK — 已解析，待后续 fork 时实现继承逻辑这样后续开发者能明确知道这是待完成项而非遗漏。
| cpu_set_t mask; | ||
| CPU_ZERO(&mask); | ||
| CPU_SET(0, &mask); | ||
|
|
There was a problem hiding this comment.
在 Linux 上，sched_getscheduler(-5) 返回 ESRCH（因为 find_process_by_pid 找不到对应进程），而非 EINVAL。
StarryOS 的 get_task_by_sched_pid 将负 pid 映射为 InvalidInput（EINVAL），这是既有行为。测试用例现在显式验证了这个与 Linux 不一致的行为。
建议改为期望 ESRCH（即 CHECK_ERR(..., ESRCH, ...)），并在内核侧后续修复 get_task_by_sched_pid 使其对 sched 系列系统调用返回 ESRCH。如果不打算在本次 PR 中修复内核，至少在测试中添加注释说明当前行为与 Linux 不一致，避免后续误解。
There was a problem hiding this comment.
Review：sched ABI 修复与测试套件（第二版）
本 PR 由两个 commit 组成：
16dcbc93— 初始 sched ABI 实现与测试套件5874f808— 修复实时调度策略的 CAP_SYS_NICE 要求
变更概述
TaskInner新增sched_policy: AtomicI32和sched_priority: AtomicI32字段，默认值分别为 0（SCHED_NORMAL）和 0。sys_sched_getscheduler从硬编码SCHED_RR改为读取 task 实际调度策略。sys_sched_setscheduler从空桩实现升级为完整的参数校验：解析SCHED_RESET_ON_FORK、校验 policy 合法性、校验优先级范围（非实时策略必须为 0，SCHED_FIFO/SCHED_RR必须在 1..=99）、检查权限。sys_sched_getparam从空桩实现升级为读取 task 实际优先级。check_sched_permission新增通用权限检查函数（uid/euid 匹配或CAP_SYS_NICE）。- 测试套件
test-sched-family覆盖了setaffinity/getaffinity/setscheduler/getscheduler/getparam/getpriority/sched_yield的边界条件和错误路径，包含 EFAULT/EINVAL/ESRCH/EPERM 测试。
前次 review 阻塞问题修复确认
前次 bot review 指出的核心阻塞问题——check_sched_permission 在目标为自身时跳过权限检查导致非特权进程可提升为实时调度——已在第二个 commit 中正确修复：
sys_sched_setscheduler 在 SCHED_FIFO/SCHED_RR 分支中独立检查 caller.has_cap_sys_nice()，与 check_sched_permission 的自检查逻辑解耦。这意味着：
sched_setaffinity对自身操作不需要 CAP_SYS_NICE（与 POSIX 一致）sched_setscheduler设置实时策略始终需要 CAP_SYS_NICE（与 Linux 一致）
设计正确，阻塞问题已解决。
实现逻辑验证
- policy 默认值：
AtomicI32::new(0)对应SCHED_NORMAL（Linux 值为 0），正确。 - 优先级范围校验：
SCHED_NORMAL/SCHED_BATCH/SCHED_IDLE要求 prio == 0，SCHED_FIFO/SCHED_RR要求 prio ∈ [1, 99]，与man sched_setscheduler一致。 SchedParam使用bytemuck::Pod：确保从用户空间加载时的内存安全性，正确。has_cap_sys_nice()在 StarryOS 中等价于euid == 0：测试以 root 身份运行，CAP_SYS_NICE 检查通过，测试可正常覆盖实时策略路径。vm_write_slice在sys_sched_getparam中的使用：使用unsafe块构造字节 slice 并通过vm_write_slice写入用户空间，方式与项目既有模式一致。
非阻塞性建议
_reset_on_fork未使用：建议添加// TODO: SCHED_RESET_ON_FORK 已解析，待 fork 时实现继承逻辑注释，便于后续开发者明确。- 负 pid 返回 EINVAL 而非 ESRCH：
get_task_by_sched_pid对负 pid 返回InvalidInput，而 Linux 返回 ESRCH。此为既有行为，测试显式验证了当前行为。建议后续在get_task_by_sched_pid中修复。 - 多个新增文件缺少末尾换行：
CMakeLists.txt、qemu-riscv64.toml、qemu-x86_64.toml、test_framework.h、main.c末尾缺少换行符。 - EPERM 测试在 root 环境下始终走 "allowed" 分支：fork 子进程的 uid 与父进程相同（root），
has_cap_sys_nice()也为 true，所以 EPERM 路径无法被覆盖。这是测试环境的固有限制，不阻塞。
重复/冲突分析
- base 分支：
sys_sched_getscheduler固定返回SCHED_RR，sys_sched_setscheduler/sys_sched_getparam为空桩。本 PR 是该区域的首次实质性实现。 - open PR 搜索：搜索了所有包含 "sched" 关键词的 open PR，未发现与 sched 系列系统调用重叠的其他 PR。#963（HAL 重构）、#966（FDT 处理）、#965（串口驱动）均不涉及此变更面。
本地验证
cargo fmt --check：仅存在 pre-existing 的 let-chain 警告（process_cred函数，第 327-328 行），与本次 PR 变更无关。PR 变更部分格式正确。- 代码审查未发现逻辑错误或安全问题。
结论
第二个 commit 正确修复了前次 review 的阻塞问题，实现逻辑与 Linux/POSIX 语义一致，测试覆盖全面。建议合并且关注上述非阻塞性改进项。
Powered by glm-5.1
1 participant
No description provided.