Skip to content

Conversation

@fslongjin
Copy link
Member

  • 修复 IoVecs 构造时对零长度缓冲区的验证,确保符合 Linux 语义
  • 修复 scatter 方法在遇到不可访问内存时的错误处理,避免部分写入后返回错误
  • 修复 readv/preadv 等系统调用,使其支持分块读取和部分成功写入
  • 修复页面回收逻辑,避免回收仍被映射的文件页
  • 修复 UserBufferReader/Writer 对空指针的检查,防止未定义行为
  • 调整缓存阈值并添加 gVisor 测试的内存检测逻辑

- 修复 IoVecs 构造时对零长度缓冲区的验证,确保符合 Linux 语义
- 修复 scatter 方法在遇到不可访问内存时的错误处理,避免部分写入后返回错误
- 修复 readv/preadv 等系统调用,使其支持分块读取和部分成功写入
- 修复页面回收逻辑,避免回收仍被映射的文件页
- 修复 UserBufferReader/Writer 对空指针的检查,防止未定义行为
- 调整缓存阈值并添加 gVisor 测试的内存检测逻辑

Signed-off-by: longjin <longjin@DragonOS.org>
@github-actions github-actions bot added the Bug fix A bug is fixed in this pull request label Dec 20, 2025
@fslongjin
Copy link
Member Author

@claude review this PR in Chinese

@dragonos-community-ai-pr-reviewer
Copy link

dragonos-community-ai-pr-reviewer bot commented Dec 20, 2025

Claude finished @fslongjin's task in 6m 27s —— View job


PR 代码审查

我将对这个 PR 进行全面的代码审查。以下是审查任务清单:

  • 分析 PR 描述和变更文件
  • 检查核心修复:IoVecs 构造、scatter 方法错误处理、readv/preadv 系统调用
  • 检查页面回收逻辑修复
  • 检查 UserBufferReader/Writer 空指针检查
  • 检查缓存阈值和 gVisor 测试内存检测逻辑
  • 提供代码审查反馈和建议

总体评价

这个 PR 修复了用户空间内存访问和页面回收的关键问题,主要改进包括:

  1. IoVecs 构造:修复零长度缓冲区的验证,确保符合 Linux 语义
  2. scatter 方法错误处理:避免部分写入后返回错误,支持部分成功
  3. readv/preadv 系统调用:支持分块读取和部分成功写入
  4. 页面回收逻辑:避免回收仍被映射的文件页
  5. UserBufferReader/Writer 空指针检查:防止未定义行为
  6. 缓存阈值调整:降低块缓存阈值到 2MB
  7. gVisor 测试内存检测:根据可用内存动态启用大内存测试

详细审查反馈

1. IoVecs 构造修复 (kernel/src/filesystem/vfs/iov.rs)

改进点

  • 添加了 IOV_MAX 常量(1024)限制,符合 Linux 的 UIO_MAXIOV
  • from_user 中对零长度缓冲区进行地址验证(即使 iov_len == 0 也检查 iov_base
  • 使用 user_accessible_len 进行可访问性检查,支持部分可访问区域
  • 添加了 iovs() 方法提供已验证的 iovec 列表访问

潜在问题

  • 第 102 行重复调用了 verify_area(base, one.iov_len)?(第 94 行已调用过一次)
  • 第 113-117 行:根据 _readv 标志分别创建 UserBufferWriterUserBufferReader,但这两个构造函数内部也会进行验证,可能造成重复检查

建议

  • 移除重复的 verify_area 调用
  • 考虑将第 113-117 行的验证逻辑合并,或者使用更高效的方式

2. scatter 方法错误处理 (kernel/src/filesystem/vfs/iov.rs)

改进点

  • scatter 返回值改为 Result<(), SystemError>,支持错误传播
  • 添加 written_any 标志,支持部分写入成功
  • 使用 user_accessible_len 检查可访问长度,支持部分可访问区域
  • 遇到不可访问区域时,如果有数据已写入则成功返回,否则返回 EFAULT

符合 Linux 语义:当部分数据已成功写入后遇到不可访问内存,Linux 会返回已写入的字节数,这个实现符合该语义。

3. readv/preadv 系统调用改进

sys_read.rs 改进

  • 重构了 read_into_user_buffer 函数,支持分块读取(64KB 块)
  • 使用 copy_to_user_protected 进行异常保护的拷贝
  • 支持部分成功:如果拷贝过程中遇到 EFAULT 但已有数据写入,返回已写入字节数

sys_readv.rs 改进

  • 完全重写了实现,支持分块处理(64KB 块)
  • 添加了 MAX_RW_COUNT 限制(INT_MAX & ~(PAGE_SIZE-1)
  • 逐块检查可访问性,避免遍历大地址范围
  • 支持部分成功和错误处理

sys_preadv.rssys_preadv2.rsnet/syscall.rs 改进

  • 更新 scatter 调用,正确处理 Result 返回值

4. 页面回收逻辑修复 (kernel/src/mm/page.rs)

改进点

  • evict_pages 中添加检查:如果文件页仍被映射(map_count() != 0),则不回收
  • 避免触发 InnerPage::drop 断言失败和用户空间崩溃

关键代码

if guard.map_count() != 0 {
    drop(guard);
    page_reclaimer_lock_irqsave().insert_page(page.phys_address(), &page);
    continue;
}

这个修复非常重要,防止了仍在使用的文件页被错误回收。

5. UserBufferReader/Writer 空指针检查 (kernel/src/syscall/user_access.rs)

改进点

  • UserBufferReader::newUserBufferWriter::new 中添加空指针检查
  • len != 0addr as usize == 0 时返回 EFAULT
  • 符合 Linux 语义:传递空指针给非空缓冲区应失败

copy_to/from_user_protected 改进

  • check_user_access_by_page_table 替换为 user_accessible_len
  • 仅基于 VMA 做权限/范围检查,不要求 PTE 已存在
  • 支持懒分配/按需调页的用户页

6. 缓存阈值调整 (kernel/src/driver/block/cache/mod.rs)

更改CACHE_THRESHOLD 从 64MB 降低到 2MB

这个调整可能为了减少内存占用,但需要确认是否影响性能。建议在 PR 描述中说明调整原因。

7. gVisor 测试改进

新增文件blocklists/readv_test - 包含需要大内存的测试用例

run_tests.sh 改进

  • 添加 adjust_readv_blocklist 函数,根据可用内存动态启用大内存测试
  • 当可用内存 ≥ 2.5GiB 时启用 ReadvTestNoFixture.TruncatedAtMax 测试
  • 使用 /proc/meminfo 检测可用内存

whitelist.txt 改进:添加 readv_test 到白名单

代码质量检查

安全性

  • 所有用户空间指针访问都经过验证
  • 使用异常表保护的用户拷贝函数
  • 防止空指针解引用

性能

  • 分块处理避免大内存分配
  • 逐块可访问性检查避免遍历大地址范围
  • 合理的错误处理避免不必要的中断

兼容性

  • 符合 Linux 语义(部分成功、错误码、限制值)
  • 支持零长度缓冲区验证
  • 支持懒分配内存区域

发现的问题和建议

  1. 重复验证iov.rs 中重复的 verify_area 调用,建议移除第 102 行
  2. 错误处理一致性scatter 方法中 written_any 逻辑正确,但 gather 方法也有类似模式,建议保持一致性
  3. 常量命名IOV_MAX 定义在文件顶部,但其他常量可能分散,建议统一位置
  4. 测试覆盖:需要确保新的部分成功逻辑有充分的测试覆盖

总结

这是一个高质量的 PR,解决了多个关键问题:

  1. ✅ 修复了零长度缓冲区的验证,符合 Linux 语义
  2. ✅ 改进了 scatter 方法的错误处理,支持部分成功
  3. ✅ 实现了 readv/preadv 的分块读取和部分成功写入
  4. ✅ 修复了页面回收逻辑,避免回收映射中的文件页
  5. ✅ 添加了空指针检查,防止未定义行为
  6. ✅ 调整了缓存阈值并添加了智能测试内存检测

建议合并,但建议在合并前解决重复验证的小问题。


审查完成

- 移除冗余的 verify_area 和 UserBufferReader/Writer 检查,统一使用
user_accessible_len 进行访问性验证
- 在 gather 方法中使用 copy_from_user_protected 进行异常保护的拷贝,与 scatter
方法保持一致
- 改进错误处理逻辑,当部分数据已成功读取时返回已读取的数据,否则返回 EFAULT

Signed-off-by: longjin <longjin@DragonOS.org>
@fslongjin fslongjin merged commit a59cc0e into DragonOS-Community:master Dec 22, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug fix A bug is fixed in this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant