Skip to content

Conversation

@fslongjin
Copy link
Member

本次PR集成了多个关键修复和功能增强,主要包括:

  1. 重构页缓存读写机制,解决潜在死锁问题
  2. 扩展mmap系统调用支持到多个文件系统节点
  3. 添加/dev/random随机设备支持
  4. 增强/proc文件系统,新增statm文件
  5. 修复系统调用边界情况,确保POSIX兼容性
  6. 改进x86_64架构的内存错误处理

详细变更内容

  1. 内存管理改进
  • 页缓存读写重构:将页缓存的读写操作改为两阶段模式(持锁收集目标页→解锁后执行拷贝),避免用户空间缺页异常导致的自锁问题
  • mmap错误处理增强:加强mmap系统调用的偏移量检查、内存分配逻辑和内存保护验证
  • x86_64页错误处理优化:改进内核态访问用户地址时的异常修复逻辑,正确区分SIGSEGV和SIGBUS信号
  1. 文件系统mmap支持扩展
  • 为多个文件系统节点添加mmap()方法(682593791):包括ext4、fat、ramfs等文件系统的inode实现
  • /dev/zero设备支持mmap:映射行为等同于匿名零页,符合Linux语义
  1. 设备驱动新增
  • 添加/dev/random随机设备(c208597a3):实现符合预期的随机数生成接口,支持read操作
  • 增强字符设备权限:对字符设备等非普通文件,具备读/写权限时自动允许pread/pwrite操作
  1. procfs功能增强
  • 新增/proc/[pid]/statm文件支持:显示进程虚拟内存使用情况(当前为最小实现,占位返回七个字段)
  • 优化statm文件打开逻辑:正确计算进程虚拟内存页数
  1. arch相关改进
  • x86_64 NX/XD支持完善:正确检测并启用EFER.NXE位,使用户态PROT_EXEC正确受NX约束
  • 内存保护映射优化:完善protection_map()实现

技术亮点

  1. 死锁预防:页缓存读写重构消除了用户空间缺页可能引发的自锁风险
  2. 信号处理精确化:区分不同类型的页错误,发送正确的信号(SIGSEGV/SIGBUS)
  3. POSIX兼容性:系统调用边界情况处理更符合标准
  4. 功能完整性:逐步完善mmap生态,为更多文件类型提供内存映射支持

- 将页缓存读写拆分为两阶段以避免用户缺页时持有锁
- 改进文件系统缺页处理,返回SIGBUS而非panic
- 优化sys_read/sys_write的用户缓冲区访问检查
- 修复mprotect参数对齐检查

Signed-off-by: longjin <[email protected]>
- 新增随机设备模块random_dev,提供随机字节生成能力
- 在DevFS中注册/dev/random设备,确保系统能够访问随机数据
- 更新相关文件以支持新设备的集成

Signed-off-by: longjin <[email protected]>
- 为LockedZeroInode、LockedExt4Inode、LockedFATInode和LockedRamFSInode实现mmap方法,允许内存映射操作。
- 更新相关文件以支持mmap功能,确保与用户空间的交互更加灵活。

Signed-off-by: longjin <[email protected]>
- Enhanced error handling in mmap implementation to return appropriate errors for unsupported operations.
- Added checks for MAP_PRIVATE and MAP_SHARED flags to ensure only one is set.
- Implemented page alignment validation for MAP_FIXED.
- Updated tests to reflect changes in mmap behavior.

Signed-off-by: longjin <[email protected]>
- Updated the `init_xd_rsvd` function to ensure NX support is enabled and correctly handle hardware limitations.
- Improved alignment checks in `sys_mprotect` to prevent overflow and ensure proper memory area verification.
- Removed outdated tests from `mmap_test` to streamline the test suite.

Signed-off-by: longjin <[email protected]>
- Introduced the ProcStatm file type to the ProcFileType enum.
- Implemented the open_statm function to return a placeholder response for the statm file.
- Updated the ProcFS inode creation to include the statm file for each process.
- Enhanced the IndexNode implementation to handle the new ProcStatm file type.

Signed-off-by: longjin <[email protected]>
@github-actions github-actions bot added the enhancement New feature or request label Dec 10, 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 10, 2025

Claude finished @fslongjin's task in 4m 12s —— View job


PR审查:内存管理、mmap支持与设备驱动的综合改进

我将对这个PR进行代码审查。以下是我的审查计划:

  • 分析PR的整体结构和变更范围
  • 审查内存管理相关更改(页缓存重构、mmap错误处理)
  • 审查文件系统mmap支持扩展
  • 审查设备驱动新增(/dev/random)
  • 审查procfs功能增强
  • 审查arch相关改进
  • 检查代码质量、潜在问题和改进建议
  • 提供综合审查反馈

综合审查报告

PR概览

本次PR包含了内存管理、文件系统、设备驱动和进程信息等多个方面的综合改进,代码质量整体较高,架构设计合理。主要变更包括:

  1. 页缓存读写重构 - 解决潜在死锁问题
  2. mmap系统调用增强 - 扩展支持更多文件系统,改进错误处理
  3. /dev/random设备支持 - 新增随机数生成接口
  4. procfs功能增强 - 新增/proc/[pid]/statm文件
  5. 系统调用边界情况处理 - 确保POSIX兼容性
  6. x86_64内存错误处理优化 - 改进内核态访问用户地址的异常修复逻辑

详细审查反馈

1. 页缓存重构 (kernel/src/filesystem/page_cache.rs)

优点:

  • 采用两阶段读写模式(持锁收集目标页→解锁后执行拷贝)有效避免了用户空间缺页异常导致的自锁问题
  • 引入CopyItem结构体封装拷贝信息,设计清晰
  • 使用volatile_read/volatile_write预触发用户缓冲区,防止在持页锁时发生缺页

潜在问题:

  • volatile_read!(buf[dst_offset])volatile_write!(buf[dst_offset], byte)的使用可能不够标准,建议确认这种模式是否符合Rust的volatile访问语义
  • 第450-451行:先读取再写入同一位置可能不是必要的,只需读取即可预触发页面

2. mmap系统调用增强 (kernel/src/mm/syscall/sys_mmap.rs)

优点:

  • 偏移量溢出检查完善,符合Linux语义(EOVERFLOW处理)
  • RLIMIT_AS检查增加页面容差,避免误拒边界映射
  • 支持MAP_POPULATEMAP_LOCKED标志的预分配逻辑
  • 参数对齐和边界检查全面

改进建议:

  • 第77行:is_none_or的使用略显晦涩,可考虑更明确的检查逻辑
  • 可考虑添加对MAP_STACK等标志的早期拒绝(如Linux那样)

3. /dev/random设备支持 (kernel/src/filesystem/devfs/random_dev.rs)

优点:

  • 设备号设置正确(1,8),符合Linux惯例
  • 权限设置为0o666,允许所有用户读写
  • 读写接口实现符合预期

严重安全问题:

  • 随机数生成依赖rand_bytes(),而底层使用_rdtsc()时间戳计数器
  • RDTSC不是加密安全的随机数源,容易被预测
  • 对于/dev/random设备,应提供真正的熵源(如硬件RDRAND、熵池等)

建议:

  • 短期:将设备重命名为/dev/urandom(非阻塞随机源),或添加明确文档说明其安全性限制
  • 长期:实现真正的熵收集和混合算法,支持/dev/random(阻塞)和/dev/urandom(非阻塞)的不同语义

4. procfs statm文件支持 (kernel/src/filesystem/procfs/mod.rs)

优点:

  • 最小化实现,返回七个字段占位符,为后续扩展奠定基础
  • 正确计算进程虚拟内存页数(第440-442行)
  • 与进程状态文件结构一致

改进建议:

  • 目前只实现了第一个字段(总虚拟内存页数),其他六个字段为0,需要在文档或注释中说明
  • 可考虑逐步实现RSS、共享页等实际字段

5. 系统调用边界处理 (sys_read.rs, sys_write.rs等)

优点:

  • 正确处理len==0的POSIX语义(成功返回0,不访问缓冲区)
  • 用户缓冲区访问检查更严格,防止无效页访问
  • 区分用户态和内核态的访问模式

6. x86_64架构改进 (kernel/src/arch/x86_64/mm/fault.rs等)

优点:

  • 改进内核态访问用户地址时的异常修复逻辑
  • 正确区分SIGSEGV和SIGBUS信号
  • NX/XD支持完善,正确检测EFER.NXE位

代码质量总体评估

优点:

  1. 架构设计合理,死锁预防措施有效
  2. POSIX兼容性考虑全面
  3. 错误处理更加健壮,边界检查完善
  4. 代码注释较为充分,关键逻辑有解释
  5. 功能完整性逐步完善,为后续开发奠定基础

需要注意的问题:

  1. 安全性/dev/random的随机数源不安全,需紧急处理
  2. 正确性:确认volatile访问模式是否符合预期
  3. 完整性:部分功能为最小实现(如statm文件),需要后续完善

建议的后续步骤

  1. 高优先级:修复/dev/random的安全问题,至少应重命名或添加安全警告
  2. 中优先级:完善procfs statm文件的字段实现
  3. 低优先级:考虑添加更多mmap标志的支持和测试

结论

本次PR的变更质量较高,解决了重要的死锁问题和功能缺失,增强了系统的稳定性和功能性。主要问题集中在/dev/random设备的安全性上,建议在合并前处理此问题。其他改进均为正向,可以提升系统的整体质量。

推荐在解决/dev/random安全问题后合并此PR。


审查完成时间:2025-12-10
审查者:Claude

- Separated the page reclamation into two phases to avoid holding the reclaimer lock for extended periods, reducing the risk of lock order inversion with page_manager/page_cache.
- Updated the `shrink_list` method to handle victim page eviction without holding the reclaimer lock, ensuring safer memory management.
- Improved the `drain_lru` method to efficiently retrieve victim pages for reclamation.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant