fix(starry-kernel): validate sync_file_range flags and offsets#823
Conversation
![mai-team-app[bot]](https://avatars.githubusercontent.com/u/34859362?s=60&v=4){kind=small}
There was a problem hiding this comment.
整体来看，这个 PR 方向正确，对 sync_file_range 的参数校验是必要的改进。代码整洁，测试覆盖了主要边界场景。
cargo fmt --check 和 cargo clippy --package starry-kernel 均通过，无警告。
发现的问题：
flags == 0的处理与 Linux 不一致（见内联评论）——内核会误拒合法的零 flags 调用- 缺少
nbytes < 0的测试用例 —— 内核代码已校验但测试未覆盖（建议后续补充）
建议修复第 1 点后合并。
Powered by mimo-v2.5-pro
There was a problem hiding this comment.
Review 总结
PR 概述：为 sys_sync_file_range 增加参数校验（负 offset/nbytes 返回 EINVAL、非法 flags 返回 EINVAL），同时扩展 test-fsync-dir 测试用例，覆盖 fsync/fdatasync 对非法 fd、pipe、socket 的错误语义，以及 sync_file_range 的非法 flags 和负 offset/nbytes 边界。
实现逻辑：
sys_sync_file_range在 base 分支是完全 no-op，忽略所有参数。本 PR 在 fd 校验之前增加了offset < 0、nbytes < 0和(flags & !SYNC_FILE_RANGE_ALL) != 0的 EINVAL 检查，与 Linuxsync_file_range(2)man page 一致。- 函数保持 advisory no-op 语义不变，仅增加参数前置校验。
- 测试新增 Test 3（非法 fd → EBADF）、Test 4（pipe → EINVAL）、Test 5（socket → EINVAL）、Test 6 扩展（flags=0 成功、flags=0x8000 → EINVAL、负 offset → EINVAL、负 nbytes → EINVAL）。
之前 Review 反馈的修复情况：
- ✅
flags == 0误拒问题已修复：当前代码仅检查(flags & !SYNC_FILE_RANGE_ALL) != 0，flags=0 正常通过。 - ✅ 缺少
nbytes < 0测试已补充：新增CHECK_ERR(syscall(SYS_sync_file_range, fd, 0, (off_t)-1, 2), EINVAL, ...)测试。
本地验证结果：
cargo fmt --check→ 通过（无格式问题）cargo clippy --package starry-kernel -- -D warnings→ 通过（无警告）- 测试用例
/usr/bin/test-fsync-dir已在qemu-aarch64.toml等配置中注册，测试发现正常。
CI 状态：所有 GitHub Actions 检查均为 skipped（fork PR，CI 未自动触发）。本地验证通过。
与 Linux 语义对比：
fsync/fdatasync对 pipe/socket 返回 EINVAL —— 与 Linux 行为一致。sync_file_rangeflags=0 返回 0 —— 与 Linux 行为一致。sync_file_range非法 flags 返回 EINVAL —— 与 Linux 行为一致。sync_file_range负 offset/nbytes 返回 EINVAL —— 与 Linux 行为一致。- 注意：
sync_file_range对 pipe fd 返回 EBADF（通过File::from_fd），而 Linux 返回 ESPIPE。这是 base 分支的已有行为，非本 PR 引入，不阻塞合并。
结论：代码正确，测试覆盖充分，之前 review 反馈已全部修复。APPROVE。
Powered by mimo-v2.5-pro
There was a problem hiding this comment.
Review 总结
PR 目标：为 sys_sync_file_range 增加参数校验，扩展 fsync/fdatasync 错误语义测试，修复 shm-deadlock 看门狗竞态。
✅ 正确的部分
- 参数校验逻辑正确：
offset < 0 || nbytes < 0→ EINVAL、(flags & !SYNC_FILE_RANGE_ALL) != 0→ EINVAL，与 Linuxsync_file_range(2)man page 一致 - flags=0 正确接受：前次 review 反馈已修复，
flags=0不再被误拒 - pipe/socket 返回 ESPIPE：使用
get_file_like+downcast_ref模式，对非文件 fd 返回 ESPIPE，比旧代码（依赖File::from_fd返回的错误类型）更准确 - 测试覆盖全面：覆盖了非法 fd → EBADF、pipe → EINVAL、socket → EINVAL、flags=0 → 成功、非法 flags → EINVAL、负 offset → EINVAL、负 nbytes → EINVAL
- watchdog 竞态修复正确：
g_threads_done只在所有waitpid完成后设置，避免看门狗过早退出 - cargo clippy / fmt 均通过
⚠️ 需要修复的问题
Memfd fd 回归（见内联评论）：sync_file_range 对 Memfd fd 现在错误返回 ESPIPE。旧代码通过 File::from_fd 透明解包 Memfd 可以正常工作，新代码改用 get_file_like + downcast_ref::<File>() 后 Memfd 无法匹配，应参照同文件 sys_fadvise64 的模式增加 downcast_ref::<Memfd>() 检查。
💡 建议（非阻塞）
test-shm-deadlock的 watchdog 修复与 PR 主题（fsync/sync_file_range）无关，建议拆分到独立 PR 保持提交历史清晰- PR 当前处于 dirty merge 状态，需要 rebase 到最新 dev 分支
Powered by glm-5.1
There was a problem hiding this comment.
Review 总结
PR 概述：为 sys_sync_file_range 增加参数校验（负 offset/nbytes 返回 EINVAL、非法 flags 返回 EINVAL），扩展 test-fsync-dir 测试用例覆盖 fsync/fdatasync 错误语义，增加 Memfd fd 支持，修复 shm-deadlock 看门狗竞态。
实现逻辑
-
sys_sync_file_range参数校验：offset < 0 || nbytes < 0→ EINVAL（与 Linuxsync_file_range(2)一致）(flags & !SYNC_FILE_RANGE_ALL) != 0→ EINVAL（仅允许 WAIT_BEFORE=1, WRITE=2, WAIT_AFTER=4 的组合）flags == 0正常通过，与 Linux 行为一致（之前的flags == 0 ||误拒已修复）- 函数保持 advisory no-op 语义不变，仅增加参数前置校验
-
fd 类型校验：
- 使用
get_file_like+downcast_ref模式检查 File / Directory / Memfd - Memfd fd 在最新提交（"Fix Memfd fd"）中已增加
downcast_ref::<Memfd>()支持，参照同文件sys_fadvise64模式 - 非 File/Directory/Memfd 返回 ESPIPE
- 使用
-
shm-deadlock 看门狗竞态修复：
- 新增
g_threads_done变量，仅在所有waitpid完成后设置 - 看门狗从检查
g_running改为检查g_threads_done，避免在工作线程即将正常退出但 waitpid 尚未完成时看门狗过早退出
- 新增
-
测试覆盖（
test-fsync-dir）：- Test 3: 无效 fd (-1) → EBADF
- Test 4: pipe fd fsync/fdatasync → EINVAL
- Test 5: socket fd fsync/fdatasync → EINVAL
- Test 6 扩展: flags=0 → 成功, flags=0x8000 → EINVAL, 负 offset → EINVAL, 负 nbytes → EINVAL
之前 Review 反馈修复情况
- ✅
flags == 0误拒：已修复。当前代码仅检查(flags & !SYNC_FILE_RANGE_ALL) != 0，flags=0正常通过 - ✅ Memfd fd 回归：已修复。最新提交
50b2985b增加了downcast_ref::<Memfd>()检查
本地验证结果
cargo fmt --check→ 通过cargo clippy --package starry-kernel -- -D warnings→ 通过（无警告）
CI 状态
GitHub Actions 检查均为 skipped（fork PR，CI 未自动触发）。本地验证通过。
与 Linux 语义对比
sync_file_rangeflags=0 返回 0 ✅sync_file_range非法 flags 返回 EINVAL ✅sync_file_range负 offset/nbytes 返回 EINVAL ✅fsync/fdatasync对 pipe/socket 返回 EINVAL ✅sync_file_range对 Memfd fd 返回成功 ✅（已修复）
重复/重叠分析
- base 分支
sys_sync_file_range忽略 offset/nbytes 参数（_offset,_nbytes），仅做基本 flags 检查（flags & !0x7） - 搜索
sync_file_range、shm deadlock watchdog、fsync test-fsync相关 open PR，未发现重复或重叠的 PR - PR #930（Axvisor x86_64 Linux guest boot）与本项目完全无关
结论
代码正确，两次 review 反馈均已修复，测试覆盖充分。APPROVE。
💡 建议（非阻塞）
test-shm-deadlock看门狗修复与 fsync/sync_file_range 主题无关，建议后续将无关修改变更拆分到独立 PR，保持提交历史清晰
Powered by glm-5.1
There was a problem hiding this comment.
Review 总结
PR 概述：为 sys_sync_file_range 增加参数校验（负 offset/nbytes 返回 EINVAL、非法 flags 返回 EINVAL），扩展 test-fsync-dir 测试用例覆盖 fsync/fdatasync 错误语义，增加 Memfd fd 支持，优化 sys_shmat 锁持有范围，修复 test-shm-deadlock 看门狗竞态。
✅ 正确的部分
-
sys_sync_file_range参数校验：offset < 0 || nbytes < 0→ EINVAL，与 Linuxsync_file_range(2)一致(flags & !SYNC_FILE_RANGE_ALL) != 0→ EINVAL，用命名常量替换了旧代码的 magic number0x7flags == 0正确接受（前次 review 的flags == 0 ||误拒已修复）- 函数保持 advisory no-op 语义不变
-
Memfd fd 支持：
downcast_ref::<Memfd>()已加入 fd 类型校验，参照同文件sys_fadvise64模式，与之前 review 反馈一致 -
sys_shmat锁持有优化：- 在
SHM_MANAGER下获取shm_inner_arc后立即释放全局锁 - 映射操作只持有
shm_inner和aspace锁 - 映射完成后重新获取
SHM_MANAGER进行insert_shmid_vaddr登记簿操作 - 减少了全局锁的持有时间，避免阻塞其他 shm 操作
- 在
-
test-fsync-dir测试覆盖全面：无效 fd → EBADF、pipe → EINVAL、socket → EINVAL、flags=0 → 成功、非法 flags → EINVAL、负 offset → EINVAL、负 nbytes → EINVAL -
cargo fmt --check和cargo xtask clippy --package starry-kernel均通过
⚠️ 需要修复的阻塞问题
test-shm-deadlock 看门狗循环依赖（见内联评论）：
g_threads_done = 1 被放在 waitpid(tid3) 之后，但 tid3 就是看门狗线程本身，而看门狗通过检查 g_threads_done 来决定是否退出。这形成了循环依赖：
- 主线程
waitpid(tid3)等待看门狗退出 - 看门狗等待
g_threads_done == true才退出 g_threads_done只在看门狗退出后才被设置
结果：看门狗永远看不到 g_threads_done == true，总是超时后设置 g_deadlock_detected = 1 并调用 _exit(1)，导致测试在没有死锁时也会报告失败。
建议修复：将 g_threads_done = 1 移到 worker waitpid 与 watchdog waitpid 之间：
waitpid(tid1, &status, __WALL);
waitpid(tid2, &status, __WALL);
g_threads_done = 1; // 工作线程已收集完毕，通知看门狗
waitpid(tid3, &status, __WALL); // 然后等待看门狗退出💡 建议（非阻塞）
test-shm-deadlock看门狗修复与 PR 标题主题（fsync/sync_file_range）无关，建议后续将无关修改变更拆分到独立 PR
本地验证
cargo fmt --check→ 通过cargo xtask clippy --package starry-kernel→ 通过（12/12 检查通过，0 警告）
重复/重叠分析
- 搜索了
sync_file_range、shm deadlock、fsync test相关 open PR，未发现重复或重叠 - base 分支的
sys_sync_file_range完全忽略 offset/nbytes，且 flags 检查使用 magic number0x7，本 PR 是对 base 的正确改进
CI 状态
所有 GitHub Actions 检查均为 skipped（fork PR，CI 未自动触发）。本地验证通过。
Powered by glm-5.1
There was a problem hiding this comment.
Review 总结
PR 概述：为 sys_sync_file_range 增加参数校验（负 offset/nbytes 返回 EINVAL、非法 flags 返回 EINVAL），扩展 test-fsync-dir 测试用例覆盖 fsync/fdatasync 错误语义，增加 Memfd fd 支持，优化 sys_shmat 锁持有范围，修复 test-shm-deadlock 看门狗竞态。
实现逻辑
-
sys_sync_file_range参数校验：offset < 0 || nbytes < 0→ EINVAL，与 Linuxsync_file_range(2)一致(flags & !SYNC_FILE_RANGE_ALL) != 0→ EINVAL，用命名常量替换旧代码的 magic number0x7flags == 0正确接受（前次 review 的flags == 0 ||误拒已修复）- 函数保持 advisory no-op 语义不变
-
fd 类型校验：
- 使用
get_file_like+downcast_ref模式检查 File / Directory / Memfd - Memfd fd 已增加
downcast_ref::<Memfd>()支持，参照同文件sys_fadvise64模式 - 非 File/Directory/Memfd 返回 ESPIPE，与 Linux 行为一致
- 使用
-
sys_shmat锁持有优化：- 在
SHM_MANAGER下获取shm_inner_arc后立即释放全局锁 - 映射操作只持有
shm_inner和aspace锁 - 映射完成后重新获取
SHM_MANAGER进行insert_shmid_vaddr登记簿操作 - 减少了全局锁的持有时间，避免阻塞其他 shm 操作
- 在
-
test-shm-deadlock看门狗修复：- 新增
g_threads_done变量，在两个 worker 的waitpid完成后设置 kill(tid3, SIGKILL)作为安全网，确保看门狗在g_threads_done未被及时感知时也能退出- 修复了前次 review 指出的循环依赖问题
- 新增
-
test-fsync-dir测试覆盖：- Test 3: 无效 fd (-1) → EBADF
- Test 4: pipe fd fsync/fdatasync → EINVAL
- Test 5: socket fd fsync/fdatasync → EINVAL
- Test 6 扩展: flags=0 → 成功, 非法 flags → EINVAL, 负 offset → EINVAL, 负 nbytes → EINVAL
之前 Review 反馈修复情况
- ✅
flags == 0误拒：已修复，仅检查(flags & !SYNC_FILE_RANGE_ALL) != 0 - ✅ Memfd fd 回归：已修复，增加了
downcast_ref::<Memfd>()检查 - ✅ 看门狗循环依赖：已修复，
g_threads_done在 worker waitpid 之后、watchdog waitpid 之前设置，并增加kill(tid3, SIGKILL)安全网
本地验证结果
cargo fmt --check→ 通过cargo xtask clippy --package starry-kernel→ 12/12 检查通过，0 警告
CI 状态
所有 GitHub Actions 检查均为 skipped（fork PR，CI 未自动触发）。本地验证通过。
与 Linux 语义对比
sync_file_rangeflags=0 返回 0 ✅sync_file_range非法 flags 返回 EINVAL ✅sync_file_range负 offset/nbytes 返回 EINVAL ✅fsync/fdatasync对 pipe/socket 返回 EINVAL ✅sync_file_range对 Memfd fd 返回成功 ✅sync_file_range对 pipe/socket 返回 ESPIPE ✅- 注意：
sync_file_range对目录 fd 返回成功（与fsync保持一致），Linux 返回 EINVAL。这是 base 分支的已有行为，非本 PR 引入
重复/重叠分析
- base 分支
sys_sync_file_range完全忽略 offset/nbytes，flags 检查使用 magic number0x7，本 PR 是正确改进 - 搜索了
sync_file_range、shm deadlock、fsync test、test-fsync-dir相关 open PR，未发现重复或重叠 - PR #895 (sqlite3 测试)、#970 (sched 测试) 等与本项目完全无关
💡 建议（非阻塞）
test-shm-deadlock看门狗修复和sys_shmat锁优化与 PR 标题主题（fsync/sync_file_range）无关，建议后续将无关修改变更拆分到独立 PRshm.rs中新增的info!()日志较详细，建议在合并前评估是否需要降级为debug!()或在发布版本中关闭
结论
代码正确，三次 review 反馈（flags==0 误拒、Memfd 回归、看门狗循环依赖）均已修复。本地验证通过（fmt + clippy）。APPROVE。
Powered by glm-5.1
There was a problem hiding this comment.
Review 总结
PR 概述：为 sys_sync_file_range 增加参数校验（负 offset/nbytes → EINVAL、非法 flags → EINVAL），增加 Memfd fd 支持，优化 sys_shmat 全局锁持有范围，修复 test-shm-deadlock 看门狗竞态，扩展 test-fsync-dir 测试覆盖。
实现逻辑
-
sys_sync_file_range参数校验：offset < 0 || nbytes < 0→ EINVAL，与 Linuxsync_file_range(2)一致(flags & !SYNC_FILE_RANGE_ALL) != 0→ EINVAL，用命名常量替换旧代码 magic number0x7flags == 0正确接受（前次 review 的误拒已修复）- 函数保持 advisory no-op 语义不变，仅增加参数前置校验
-
fd 类型校验：
downcast_ref::<Memfd>()已加入校验链，参照同文件sys_fadvise64模式- 非 File/Directory/Memfd 返回 ESPIPE，与 Linux 行为一致
-
sys_shmat锁持有优化：- 在
SHM_MANAGER下获取shm_inner_arc后立即释放全局锁 - 映射操作只持有
shm_inner和aspace锁 - 映射完成后重新获取
SHM_MANAGER进行insert_shmid_vaddr登记簿操作 - 减少了全局锁的持有时间，降低了多核场景下的锁竞争
- 在
-
test-shm-deadlock看门狗修复：g_threads_done在两个 worker 的waitpid之后设置kill(tid3, SIGKILL)作为安全网确保看门狗退出- 修复了前次 review 指出的循环依赖
-
test-fsync-dir测试覆盖：- Test 3: 无效 fd (-1) → EBADF
- Test 4: pipe fd fsync/fdatasync → EINVAL
- Test 5: socket fd fsync/fdatasync → EINVAL
- Test 6 扩展: flags=0 → 成功, 非法 flags → EINVAL, 负 offset → EINVAL, 负 nbytes → EINVAL
之前 Review 反馈修复情况
- ✅
flags == 0误拒：已修复，仅检查(flags & !SYNC_FILE_RANGE_ALL) != 0 - ✅ Memfd fd 回归：已修复，增加了
downcast_ref::<Memfd>()检查 - ✅ 看门狗循环依赖：已修复，
g_threads_done在 worker waitpid 之后设置，并增加kill(tid3, SIGKILL)安全网
本地验证结果
cargo fmt --check→ 通过（无格式问题）cargo clippy --package starry-kernel -- -D warnings→ 通过（0 警告）
CI 状态
所有 GitHub Actions 检查均为 skipped（fork PR，CI 未自动触发）。本地验证通过。
与 Linux 语义对比
sync_file_rangeflags=0 返回 0 ✅sync_file_range非法 flags 返回 EINVAL ✅sync_file_range负 offset/nbytes 返回 EINVAL ✅fsync/fdatasync对 pipe/socket 返回 EINVAL ✅sync_file_range对 Memfd fd 返回成功 ✅- 注意：
sync_file_range对目录 fd 返回成功（与 fsync 一致），Linux 返回 EINVAL。这是 base 分支的已有行为，非本 PR 引入
重复/重叠分析
- base 分支
sys_sync_file_range完全忽略 offset/nbytes，flags 检查使用 magic number0x7，本 PR 是正确改进 - base 分支
sys_shmat在整个映射过程中持有SHM_MANAGER，本 PR 减小了锁持有范围 - 搜索了
sync_file_range、shm deadlock、fsync test、test-fsync-dir相关 open PR，未发现重复或重叠 - PR #226 修复了原始 SHM AB/BA 死锁，但未优化锁持有范围，本 PR 的锁优化是独立改进
💡 建议（非阻塞）
shm.rs中新增的info!()日志较详细，建议合并前评估是否需要降级为debug!()或在发布版本中关闭test-shm-deadlock看门狗修复和sys_shmat锁优化与 PR 标题主题（fsync/sync_file_range）无关，建议后续将无关修改变更拆分到独立 PR
结论
代码正确，三次 review 反馈均已修复，本地验证通过。APPROVE。
Powered by glm-5.1
There was a problem hiding this comment.
Review 总结
PR 概述：为 sys_sync_file_range 增加参数校验（负 offset/nbytes → EINVAL、非法 flags → EINVAL），增加 Memfd fd 支持，优化 sys_shmat 全局锁持有范围，修复 test-shm-deadlock 看门狗竞态，扩展 test-fsync-dir 测试覆盖。
✅ 正确的部分
-
sys_sync_file_range参数校验：offset < 0 || nbytes < 0→ EINVAL，与 Linuxsync_file_range(2)一致(flags & !SYNC_FILE_RANGE_ALL) != 0→ EINVAL，用命名常量替换了旧代码的 magic number0x7flags == 0正确接受（前几次 review 的误拒已修复）- 函数保持 advisory no-op 语义不变
-
Memfd fd 支持：
downcast_ref::<Memfd>()已加入 fd 类型校验链，与同文件sys_fadvise64模式完全一致 ✅ -
sys_shmat锁持有优化：- 在 SHM_MANAGER 下获取
shm_inner_arc后立即释放全局锁 - 映射操作只持有
shm_inner和aspace锁 - 映射完成后重新获取 SHM_MANAGER 进行
insert_shmid_vaddr登记簿操作 - 有效减少了全局锁的持有时间，降低多核场景下锁竞争
- 在 SHM_MANAGER 下获取
-
test-fsync-dir测试覆盖全面：无效 fd → EBADF、pipe → EINVAL、socket → EINVAL、flags=0 → 成功、非法 flags → EINVAL、负 offset → EINVAL、负 nbytes → EINVAL -
test-shm-deadlock看门狗修复正确：g_threads_done在 worker waitpid 之后、watchdog waitpid 之前设置，解决了循环依赖kill(tid3, SIGKILL)作为安全网确保看门狗退出
-
本地验证：
cargo fmt --check和cargo clippy --package starry-kernel -- -D warnings均通过，0 警告
💡 建议（非阻塞）
shm.rs中新增了大量info!()日志（shmat/shmdt 各约 5 条），在正常负载下会产生大量输出。建议合并前将大部分降级为debug!()，只保留关键路径的info!()，或通过 feature gate 控制test-shm-deadlock看门狗修复、sys_shmat锁优化与 PR 标题主题（fsync/sync_file_range）无关，建议后续将无关修改拆分到独立 PR，保持提交历史清晰
与 Linux 语义对比
sync_file_rangeflags=0 返回 0 ✅sync_file_range非法 flags 返回 EINVAL ✅sync_file_range负 offset/nbytes 返回 EINVAL ✅sync_file_range对 Memfd fd 返回成功 ✅fsync/fdatasync对 pipe/socket 返回 EINVAL ✅- 注意：
sync_file_range对目录 fd 返回成功（Linux 返回 EINVAL），这是 base 分支已有行为
之前 Review 反馈修复情况
- ✅
flags == 0误拒：已修复 - ✅ Memfd fd 回归：已修复
- ✅ 看门狗循环依赖：已修复
结论
代码正确，三次 review 反馈均已修复，本地验证通过（fmt + clippy），测试覆盖充分。APPROVE。
Powered by glm-5.1
|
@ZR233 老师您好，PR 已经根据 review 反馈完成了修复，CI 已通过，麻烦您抽空 review 一下🙏 |
…-os#823) * fsync first commit * fsync second commit * Parameter Validation * fsync third commit * fsync fourth commit * harden shm-deadlock watchdog * Resolve the conflict * Fix Memfd fd * Avoid holding SHM_MANAGER during shmat mapping * Avoid false shm deadlock timeout * chore: rerun CI * chore: rerun CI
2 participants
第一次测试提交：补充 StarryOS fsync/sync_file_range 边界用例与内核参数校验修复。
覆盖
fsync/fdatasync对目录、pipe、socket 的错误语义，以及sync_file_range的非法 flags 和负 offset；同时修复内核对sync_file_range的参数校验逻辑。Summary
fsync/fdatasync的错误语义覆盖（非法 fd、pipe、socket）sync_file_range增加非法 flags 与负 offset 语义测试sys_sync_file_range：对负 offset/len 与非法 flags 返回EINVAL方案说明
新增测例主要针对 Linux 兼容语义和边界条件，先在用户态明确预期行为，再通过 QEMU grouped syscall case 复现并定位内核语义差异。
sync_file_range在 StarryOS 里当前是 no-op，但仍应遵守 Linux 的参数约束；因此补充了EINVAL语义校验并修复内核实现。Test plan
cargo xtask starry test qemu -t aarch64-unknown-none-softfloat -c syscallcargo xtask clippy --package starry-kernelcargo fmt