feat(prctl): 添加 capability 相关的 prctl 选项

[sparkzky]

实现对 prctl 系统调用的扩展，增加 capability 管理功能：

• PR_SET/GET_KEEPCAPS: 控制 UID/GID 变更时是否保留 capabilities
• PR_CAPBSET_READ: 检查指定 capability 是否在 bounding set 中
• PR_CAPBSET_DROP: 从 bounding set 中移除指定的 capability

这些选项对于实现与 Linux 兼容的 capability 管理机制至关重要，
特别是在容器场景下需要进行精细的权限控制。

[sparkzky]

@claude review

[dragonos-community-ai-pr-reviewer]

Claude finished @sparkzky's task in 4m 39s —— View job

---

PR 审查：feat(prctl): 添加 capability 相关的 prctl 选项

我将审查这个PR，它添加了对prctl系统调用的扩展，增加了capability管理功能。

待办事项列表：

• 创建待办事项列表并更新GitHub评论
• 分析PR上下文和更改的文件
• 读取修改的文件以理解代码更改
• 检查代码质量和潜在问题
• 提供代码审查反馈

PR 摘要：

• 标题： feat(prctl): 添加 capability 相关的 prctl 选项
• 作者： sparkzky
• 分支： feat-prctl-capability-extensions -> master
• 状态： OPEN
• 更改： 91行添加，3行删除，2个文件修改
• 功能： 实现对prctl系统调用的扩展，增加capability管理功能：
  • PR_SET/GET_KEEPCAPS: 控制UID/GID变更时是否保留capabilities
  • PR_CAPBSET_READ: 检查指定capability是否在bounding set中
  • PR_CAPBSET_DROP: 从bounding set中移除指定的capability

代码审查反馈：

总体而言，这是一个很好的功能添加，实现了与Linux兼容的capability管理。代码结构清晰，注释详细。以下是一些改进建议：

主要问题

1. 缺少capability索引范围验证 (kernel/src/process/syscall/sys_prctl.rs:179-225)

   • PR_CAPBSET_READ和PR_CAPBSET_DROP都需要验证arg2（capability索引）是否在有效范围内（0-40，对应41个capabilities）。
   • 当前代码直接使用1usize << arg2，如果arg2 >= 64（在64位系统上）或超过有效capability数量，可能导致未定义行为。
   • 建议： 添加范围检查，返回EINVAL对于无效的capability索引。

2. 缺少权限检查 (kernel/src/process/syscall/sys_prctl.rs:190-225)

   • 根据Linux文档，PR_CAPBSET_DROP需要CAP_SETPCAP能力。
   • 当前实现没有进行权限检查，任何进程都可以从bounding set中删除capabilities。
   • 建议： 在PR_CAPBSET_DROP中添加权限检查：if !cred.has_capability(CAPFlags::CAP_SETPCAP) { return Err(SystemError::EPERM); }

3. Cred克隆代码冗余且易出错 (kernel/src/process/syscall/sys_prctl.rs:198-223)

   • 当前代码手动构造新的Cred结构体，列出了所有字段。这容易出错，难以维护，并且在Cred结构体更改时可能遗漏更新。
   • Cred已经实现了Clone trait（#[derive(Debug, Clone)]）。
   • 建议： 使用Cred::new_arc()方法或先克隆再修改的方式：

     let mut new_cred = (*old_cred).clone();
     new_cred.cap_bset = new_bset;
     let new_cred = Cred::new_arc(new_cred);

次要问题

4. Capability标志处理 (kernel/src/process/syscall/sys_prctl.rs:183-185, 194-196)

   • 使用CAPFlags::from_bits(cap_bit as u64).unwrap_or(CAPFlags::CAP_EMPTY_SET)处理无效的capability索引。
   • 对于无效索引，这会将cap_flag设置为CAP_EMPTY_SET，而不是返回错误。
   • 建议： 结合范围检查，对于无效索引直接返回EINVAL。

5. 线程安全性考虑

   • PR_CAPBSET_DROP操作涉及完整的cred替换，这应该是线程安全的（通过current.set_cred()）。
   • 确保set_cred方法正确处理竞态条件。

6. 与Linux行为的一致性

   • PR_SET_KEEPCAPS: Linux允许任何非零值设置为true，0设置为false，当前实现正确。
   • PR_GET_KEEPCAPS: 返回1表示true，0表示false，当前实现正确。
   • 需要验证PR_CAPBSET_READ的返回值：Linux返回1表示在bounding set中，0表示不在，当前实现正确。

代码质量建议

7. 提取公共逻辑

   • PR_CAPBSET_READ和PR_CAPBSET_DROP中都有将索引转换为CAPFlags的逻辑，可以考虑提取为辅助函数：

     fn capability_from_index(idx: usize) -> Result<CAPFlags, SystemError> {
         if idx >= 41 { // 41个capabilities
             return Err(SystemError::EINVAL);
         }
         let cap_bit = 1u64 << idx;
         CAPFlags::from_bits(cap_bit).ok_or(SystemError::EINVAL)
     }

8. 注释改进

   • 现有注释很好，但可以添加更多关于Linux兼容性的说明。
   • 特别是PR_CAPBSET_DROP的权限要求应该文档化。

测试建议

9. 建议添加测试
   • 考虑添加单元测试或集成测试来验证这些新的prctl选项的行为。
   • 测试应该包括边界情况（无效的capability索引，权限检查等）。

总结

这是一个重要的功能添加，为DragonOS提供了更好的capability管理能力。在解决上述问题后，代码将更加健壮和安全。

需要修复的关键问题：

1. 添加capability索引范围验证
2. 为PR_CAPBSET_DROP添加CAP_SETPCAP权限检查
3. 简化Cred克隆逻辑

可选改进：

• 提取公共的capability索引转换逻辑
• 增强错误处理和边界情况测试

CI 状态

CI 正在运行：

• Test x86_64 - ⏳ 进行中
• Build Check - ⏳ 进行中

[sparkzky]

@claude review

[dragonos-community-ai-pr-reviewer]

Claude Code is working…

I'll analyze this and get back to you.

View job run