fix(starry-mm): reject overflowing addr+length in mmap instead of wrapping#1120
Conversation
ZR233
left a comment
There was a problem hiding this comment.
需要再补一层检查:现在只保护了 addr + length,但后面的 .align_up(page_size) 仍然会在接近 usize::MAX 时做一次未检查的 addr + align - 1。
memory_addr::align_up 的实现是 (addr + align - 1) & !(align - 1)。因此像 addr.checked_add(length) == usize::MAX 这类没有触发 checked_add 溢出的请求,仍会在 align_up(PageSize::Size4K/2M/1G) 时回绕到低地址。随后 let mut length = end - aligned 得到错误范围,非 MAP_FIXED 路径还可能把这个坏 hint 退回到普通 free-area 搜索;这正是本 PR 想避免的“先算错再让后续兜底”的问题。
建议把“加 length”和“向上对齐到 page_size”一起做 checked arithmetic,比如先 checked 到 raw end,再对 raw_end + page_size - 1 做 checked_add,或者引入 checked align-up helper;溢出统一返回 EINVAL。
本地验证:
git diff --check origin/dev...HEAD通过cargo fmt --check通过cargo xtask clippy --package starry-kernel通过,13 个 feature 组合均通过
CI run 的红点来自 Starry riscv64 smoke 在 success pattern 后 QEMU timeout,和这个 mmap 算术改动看起来没有直接关系。当前没有未解决的 review thread。
ee1538e to
2dd53db
Compare
ZR233
left a comment
There was a problem hiding this comment.
复审当前 head 2dd53db63a33。上次指出的 align-up 回绕问题已经修掉:现在先 checked_add 得到 raw_end,再用 end < raw_end 拦截 page-size round-up 后的 wrap,正常路径没有看到新的算术漏洞。
本地验证:
git diff --check origin/dev...HEAD通过。cargo fmt --check通过。cargo xtask clippy --package starry-kernel通过,13/13 个 feature 组合均通过。
仍需阻塞的一点是回归覆盖:这个 PR 修的是 sys_mmap 的 syscall bug,但当前没有新增或更新任何测试来复现 addr + length 溢出、以及 addr + length 未溢出但 align_up(page_size) 溢出的两类输入。现有 syscall/test-mmap-family 已有 mmap 错误路径覆盖,可以直接在那里补一个小的 raw mmap case,或放到 bugfix 组;建议至少覆盖 MAP_FIXED 近 usize::MAX 地址 + 大 length 返回 EINVAL,以及 raw_end == usize::MAX / round-up wrap 也返回 EINVAL。这类测试应在旧实现上失败、在当前修复后通过。
CI 状态:提交 review 前远端 host formatting、sync-lint、clippy 和多项 QEMU/board job 已通过,还有少量 Starry/ArceOS QEMU pending;没有看到与本地验证冲突的失败。
重复/重叠检查:open PR 搜索只发现 #886/#918 等同文件或 mm 子系统重叠,它们不是 mmap overflow 修复的替代实现;本 PR 仍是独立的小修。
| // `addr + length` itself didn't overflow but rounding up to the page boundary | ||
| // did — otherwise the wrapped value would flow into the length computation and | ||
| // the (non-FIXED) hint search below. | ||
| let raw_end = addr.checked_add(length).ok_or(AxError::InvalidInput)?; |
There was a problem hiding this comment.
算术修复方向已经覆盖了上次指出的 align_up 回绕,但这个 syscall bugfix 还缺少回归测试。请在现有 syscall/test-mmap-family 或 bugfix 组里补一个直接 mmap case,覆盖 addr + length 溢出以及 raw_end 未溢出但 page-size round-up 溢出的输入,期望返回 EINVAL;这样能证明旧实现会失败、当前修复会通过。
There was a problem hiding this comment.
🤖 Mai PR Review — #1120
改动概览
sys_mmap 增加 addr + length 和 align_up(page_size) 两层溢出检查,防止病态 mmap 请求回绕到低地址。
代码审查 ✅
addr.checked_add(length).ok_or(AxError::InvalidInput)?— 正确拦截addr + length溢出,与同文件sys_mremap/sys_munmap的checked_add模式一致。end < raw_end— 正确拦截align_up(page_size)内部raw_end + page_size - 1回绕(memory_addr::align_up实现为(addr + align - 1) & !(align - 1))。- 注释清晰完整,解释了两层溢出场景。
- 正常(非溢出)路径行为完全不变。
CI 状态 ✅
CI workflow 26967510020 所有实际运行的 job 均为 success:
| Check | 结果 |
|---|---|
| Detect changed paths | ✅ |
| Check formatting | ✅ |
| Run sync-lint | ✅ |
| Run clippy | ✅ |
| Test starry riscv64 qemu | ✅ |
| Test starry x86_64 qemu | ✅ (skipped: container variant) |
| Test starry loongarch64 qemu | ✅ |
| Test arceos riscv64/aarch64/loongarch64/x86_64 qemu | ✅ |
| Test axvisor aarch64/x86_64 qemu + UEFI + svm | ✅ |
| Test with std | ✅ |
| Board tests (orangepi-5-plus, licheerv-nano, phytiumpi) | ✅ |
未发现 CI 失败。
与 ZR233 前两轮 review 的衔接
- 第一轮提出
align_up回绕问题 → 当前 head 已用end < raw_end修复 ✅ - 第二轮确认代码正确但建议补回归测试 → 同意此建议,但不构成阻塞合并的缺陷。
建议(非阻塞)
同意 ZR233 第二轮的测试建议:在 test-suit/starryos/normal/qemu-smp1/syscall/test-mmap-family 中补充以下 case 会更完善:
MAP_FIXED+ addr 接近usize::MAX+ 大 length →EINVAL(checked_add溢出路径)MAP_FIXED+ addr 使得align_up回绕 →EINVAL(end < raw_end路径)
这类测试应在旧实现上失败、在修复后通过,适合作为后续 PR 补充。
结论:代码正确、CI 全绿、可合并。建议后续补回归测试。
Powered by mimo-v2.5-pro
There was a problem hiding this comment.
复审 #1120 — sys_mmap 溢出保护
改动概述
在 sys_mmap 中对 addr + length 和 align_up(page_size) 两层做溢出检查,防止病态 mmap 请求回绕到低地址。同时在 test-mmap-family 中补充了回归测试。
代码审查 ✅
addr.checked_add(length).ok_or(AxError::InvalidInput)?— 正确拦截加法溢出,与同文件sys_mremap已有的checked_add模式一致。end < raw_end— 正确拦截align_up(page_size)内部回绕(memory_addr::align_up实现为(addr + align - 1) & !(align - 1),当raw_end接近usize::MAX时会回绕)。- 注释完整清晰,解释了两层溢出场景和 Linux 对应行为。
- 正常路径(非溢出)行为完全不变,无安全/数据丢失风险。
- 测试使用
syscall(SYS_mmap, ...)直接走 syscall 层(避免 libc 包装),覆盖addr + length溢出路径,接受EINVAL或ENOMEM,关键不变量是r == -1。
CI 状态 ✅
CI run 27007185109(head 83061c08)全部实际运行的 job 通过:
- success: 24(含 formatting、sync-lint、clippy、std、Starry x86_64/aarch64/riscv64/loongarch64 QEMU、ArceOS 全架构 QEMU、axvisor 全架构 QEMU/UEFI/svm、board tests)
- skipped: 15(均为 CI 矩阵中
run_container/run_host互斥的预期行为) - failure: 0
与 ZR233 前两轮 review 的衔接
- 第一轮(2026-06-04 16:09):提出
align_up回绕问题 → 当前 head 已用end < raw_end修复 ✅ - 第二轮(2026-06-04 17:46):确认代码正确,要求补回归测试 → 当前 head 已在
test-mmap-family中补充MAP_FIXED近usize::MAX地址溢出 case ✅ - 两轮 review thread 均已在当前 head 解决。
重复/重叠分析
- 搜索
mmap+overflow+checked的 open PR,未发现重复实现。 - #1114(同作者)触碰 mmap 但是独立的 bundles PR,与本 PR 无冲突。
- #918(同作者)修复
mprotectENOMEM 空洞检查,不涉及 mmap overflow。 - #886(eBPF 运行时)涉及
DeviceMmap但不影响sys_mmap加法溢出路径。
本地验证
cargo fmt --check✅cargo clippy --manifest-path os/StarryOS/kernel/Cargo.toml --all-features -- -D warnings✅
结论
代码正确、两层溢出保护覆盖完整、回归测试已补充、CI 全绿、前两轮 review 已解决。可合并。
Powered by mimo-v2.5-pro
…pping `sys_mmap` 计算 `end = (addr + length).align_up(page_size)` 时未做溢出检查。 对 `addr + length` 超过 `usize` 上限的病态请求(例如 MAP_FIXED 传入接近最大值 的 addr 加一个很大的 length),加法会回绕,得到的 `length = end - aligned` 是错 误的小值或回绕值,在被后续 `validate_region` 兜住之前就已经算错。 Linux 的 `do_mmap` 在入口处用溢出检查直接返回 EINVAL。这里改成对 `addr.checked_add(length)` 做检查,溢出即返回 EINVAL —— 与本文件 `sys_mremap` 已有的 `checked_add` 处理一致。正常请求行为不变(非溢出时结果与原来相同)。 Signed-off-by: 林晨 (Leo Cheng) <chengkelfanke@gmail.com> Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com>
Add a regression to test-mmap-family: a MAP_FIXED mmap whose addr+length wraps past the top of the address space must be rejected, not silently relocated to a wrapped low address. The old code let `(addr+length).align_up` wrap internally (length underflowing to a huge value); the fix guards with `checked_add` plus an `end < raw_end` wrap check. errno differs by kernel (Linux ENOMEM for the out-of-range address, starry EINVAL for the arithmetic overflow), so the test accepts either — the real invariant is r == -1 (never a wrapped success). Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com>
83061c0 to
4108df4
Compare
There was a problem hiding this comment.
复审 #1120 — sys_mmap 溢出保护
改动概述
在 sys_mmap 中对 addr + length 和 align_up(page_size) 两层做溢出检查,防止病态 mmap 请求回绕到低地址。同时在 test-mmap-family 中补充了回归测试。
代码审查 ✅
addr.checked_add(length).ok_or(AxError::InvalidInput)?— 正确拦截加法溢出,与同文件sys_mremap/sys_munmap已有的checked_add模式一致。end < raw_end— 正确拦截align_up(page_size)内部回绕(memory_addr::align_up实现为(addr + align - 1) & !(align - 1),当raw_end接近usize::MAX时会回绕)。- 注释完整清晰,解释了两层溢出场景和 Linux 对应行为。
- 正常路径(非溢出)行为完全不变,无安全/数据丢失风险。
- 测试使用
syscall(SYS_mmap, ...)直接走 syscall 层(避免 libc 包装),覆盖addr + length溢出路径,接受EINVAL或ENOMEM,关键不变量是r == -1。 - 无
[patch.crates-io]引入。
CI 状态
CI run 27017333406(head 4108df40)已完成的 job 均为 success:
- ✅ Detect changed paths
- ✅ Check formatting (run_host)
- ✅ Run sync-lint (run_host)
- ✅ Run clippy (run_host)
- ✅ Test axvisor self-hosted x86_64 (run_host)
- ✅ Test axvisor self-hosted board phytiumpi-linux (run_host)
- ✅ Test axvisor x86_64 svm hosted (run_host)
- 跳过:container/host 互斥 job(预期行为)
- 进行中:Starry/ArceOS QEMU、board tests、std tests
已完成项无失败,CI 进行中。
与 ZR233 前两轮 review 的衔接
- 第一轮(2026-06-04 16:09):提出
align_up回绕问题 → 当前 head 已用end < raw_end修复 ✅ - 第二轮(2026-06-04 17:46):确认代码正确,要求补回归测试 → 当前 head 已在
test-mmap-family中补充MAP_FIXED近usize::MAX地址溢出 case ✅ - 两轮 review thread 均已在当前 head 解决。
重复/重叠分析
- 搜索
mmap+overflow+checked的 open PR,未发现重复实现。 - #1114(同作者)触碰 mmap 但是独立的 bundles PR,与本 PR 无冲突。
- #918(同作者)修复
mprotectENOMEM 空洞检查,不涉及 mmap overflow。 - 其他 open PR(#1156 cgroup、#1151 qperf 等)均无 mmap 溢出相关重叠。
本地验证
cargo fmt --check✅git diff --check origin/dev...HEAD✅
结论
代码正确、两层溢出保护覆盖完整、回归测试已补充、CI 已通过项无失败、前两轮 review 已解决。可合并。
Powered by mimo-v2.5-pro
ZR233
left a comment
There was a problem hiding this comment.
复审 #1120 — sys_mmap addr+length 溢出保护
改动概述
在 sys_mmap 中对 addr + length 和 align_up(page_size) 两层做溢出检查,防止病态 mmap 请求回绕到低地址。同时在 test-mmap-family 中补充了回归测试。
代码审查 ✅
addr.checked_add(length).ok_or(AxError::InvalidInput)?— 正确拦截加法溢出,与同文件sys_mremap/sys_munmap/sys_msync已有的checked_add模式一致。end < raw_end— 正确拦截align_up(page_size)内部回绕。memory_addr::align_up实现为(addr + align - 1) & !(align - 1),当raw_end接近usize::MAX时raw_end + page_size - 1回绕,结果小于raw_end。此检查在sys_mlock2(同文件 867-871 行)中已有完全一致的模式。- 注释完整清晰,解释了两层溢出场景和 Linux 对应行为。
- 正常路径(非溢出)行为完全不变:
aligned = addr.align_down(page_size) ≤ addr ≤ raw_end ≤ end,length = end - aligned不会下溢。 - 对
PageSize::Size2M/Size1G同样有效:align越大越容易触发回绕,但end < raw_end检查不依赖具体 page size。 - 无
[patch.crates-io]引入。
测试分析
- 回归测试使用
syscall(SYS_mmap, ...)直接走 syscall 层(避免 libc 包装屏蔽返回值/errno),接受EINVAL或ENOMEM,关键不变量是r == -1。 - 测试覆盖了
addr + length溢出路径(near_top = ~0xFFFUL,length = 8 * ps,使addr + length回绕)。
本地验证
| 检查项 | 结果 |
|---|---|
git diff --check origin/dev...HEAD |
✅ 无空白错误 |
cargo fmt --check |
✅ 通过 |
cargo xtask clippy --package starry-kernel |
✅ 13/13 feature 组合通过 |
cargo xtask starry test qemu --arch aarch64 -c syscall |
✅ test-mmap-family: 67 pass, 1 fail |
关键回归测试 (main.c:108) PASS:mmap MAP_FIXED 且 addr+length 溢出 → 被拒(EINVAL/ENOMEM), 不环绕。
第 77 行的 1 个失败是预存问题(MAP_PRIVATE | MAP_SHARED 同时设置应返回 EINVAL 但 StarryOS 未拒绝),与本次溢出修复无关。
CI 状态 ✅
CI run 27017333406(head 4108df40):success=24, skipped=15(run_container/run_host 互斥矩阵的预期行为), failure=0。
与前两轮 review 的衔接
- ZR233 第一轮:提出
align_up回绕问题 → 当前 head 已用end < raw_end修复 ✅ - ZR233 第二轮:确认代码正确,要求补回归测试 → 当前 head 已在
test-mmap-family中补充溢出回归 case ✅ - ZR233 第二轮 review thread(
PRRT_kwDORlre086HJ5p5)仍为 unresolved,测试已覆盖主溢出路径。建议作者或 ZR233 在方便时 resolve。
重复/重叠分析
- 搜索
mmap+overflow+checked的 open PR,未发现重复实现。 - #1114(同作者):触碰 mmap.rs 但修改的是
find_free_area上限约束和madviseDONTNEED,不涉及 addr+length 溢出检查 →partial-overlap(同文件不同区域),无合并冲突。 - #1164(同作者):file-backed mmap populate EOF 边界 →
unrelated。 - base
dev分支:sys_mmap入口仍是无保护(addr + length).align_up(page_size)→ 本 PR 独立。
非阻塞建议
ZR233 第二轮建议覆盖 addr + length 溢出以及 align_up 回绕两种输入。当前测试覆盖了前者(checked_add 路径)。如需完整覆盖后者的回归,可补充类似 addr = 页对齐近 MAX, length = 1(使 addr + length 不溢出但 align_up(4K) 回绕)的 case。此为锦上添花,不阻塞合并。
结论
代码正确、两层溢出保护覆盖完整、回归测试已补充、本地 QEMU 验证通过、CI 全绿、前两轮 review 问题已解决。可合并。
…pping (rcore-os#1120) * fix(starry-mm): reject overflowing addr+length in mmap instead of wrapping `sys_mmap` 计算 `end = (addr + length).align_up(page_size)` 时未做溢出检查。 对 `addr + length` 超过 `usize` 上限的病态请求(例如 MAP_FIXED 传入接近最大值 的 addr 加一个很大的 length),加法会回绕,得到的 `length = end - aligned` 是错 误的小值或回绕值,在被后续 `validate_region` 兜住之前就已经算错。 Linux 的 `do_mmap` 在入口处用溢出检查直接返回 EINVAL。这里改成对 `addr.checked_add(length)` 做检查,溢出即返回 EINVAL —— 与本文件 `sys_mremap` 已有的 `checked_add` 处理一致。正常请求行为不变(非溢出时结果与原来相同)。 Signed-off-by: 林晨 (Leo Cheng) <chengkelfanke@gmail.com> Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com> * test(starry-mm): cover mmap rejects an overflowing addr+length (no wrap) Add a regression to test-mmap-family: a MAP_FIXED mmap whose addr+length wraps past the top of the address space must be rejected, not silently relocated to a wrapped low address. The old code let `(addr+length).align_up` wrap internally (length underflowing to a huge value); the fix guards with `checked_add` plus an `end < raw_end` wrap check. errno differs by kernel (Linux ENOMEM for the out-of-range address, starry EINVAL for the arithmetic overflow), so the test accepts either — the real invariant is r == -1 (never a wrapped success). Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com> --------- Signed-off-by: 林晨 (Leo Cheng) <chengkelfanke@gmail.com> Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com>
…pping (#1120) * fix(starry-mm): reject overflowing addr+length in mmap instead of wrapping `sys_mmap` 计算 `end = (addr + length).align_up(page_size)` 时未做溢出检查。 对 `addr + length` 超过 `usize` 上限的病态请求(例如 MAP_FIXED 传入接近最大值 的 addr 加一个很大的 length),加法会回绕,得到的 `length = end - aligned` 是错 误的小值或回绕值,在被后续 `validate_region` 兜住之前就已经算错。 Linux 的 `do_mmap` 在入口处用溢出检查直接返回 EINVAL。这里改成对 `addr.checked_add(length)` 做检查,溢出即返回 EINVAL —— 与本文件 `sys_mremap` 已有的 `checked_add` 处理一致。正常请求行为不变(非溢出时结果与原来相同)。 Signed-off-by: 林晨 (Leo Cheng) <chengkelfanke@gmail.com> Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com> * test(starry-mm): cover mmap rejects an overflowing addr+length (no wrap) Add a regression to test-mmap-family: a MAP_FIXED mmap whose addr+length wraps past the top of the address space must be rejected, not silently relocated to a wrapped low address. The old code let `(addr+length).align_up` wrap internally (length underflowing to a huge value); the fix guards with `checked_add` plus an `end < raw_end` wrap check. errno differs by kernel (Linux ENOMEM for the out-of-range address, starry EINVAL for the arithmetic overflow), so the test accepts either — the real invariant is r == -1 (never a wrapped success). Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com> --------- Signed-off-by: 林晨 (Leo Cheng) <chengkelfanke@gmail.com> Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com>
这个改动做了什么
让
sys_mmap在计算映射范围前对addr + length做溢出检查。为什么要改
之前
end = (addr + length).align_up(page_size)不做溢出检查。对addr + length超过usize上限的病态请求(例如 MAP_FIXED 传入接近最大值的 addr 加一个很大的 length),加法回绕,length = end - aligned得到错误的小值/回绕值,在被后续validate_region兜住之前就已经算错。Linux 的do_mmap在入口直接用溢出检查返回 EINVAL。怎么改的
改用
addr.checked_add(length),溢出即返回 EINVAL,与本文件sys_mremap已有的checked_add处理一致。正常(非溢出)请求行为完全不变。验证