feat(vfs): support bind mount reconfiguration

[sparkzky]

Allow MS_BIND|MS_REMOUNT to update per-mount flags on existing bind
mounts, including readonly enforcement on wrapped mount inodes.

Also preserve subtree roots when creating bind mounts and add C unit
tests covering readonly bind remount and subdirectory bind behavior.

[sparkzky]

@claude review

[dragonos-community-ai-pr-reviewer]

PR Review: feat(vfs): support bind mount reconfiguration

I've reviewed the changes in this PR. Overall, the implementation looks solid and adds important functionality for bind mount reconfiguration. Below is my detailed review.

Todo list:

• Read repository's CLAUDE.md for guidelines
• Analyze PR changes by reading modified files
• Review the new test file test_mount_reconfigure.c
• Check for potential bugs, security issues, and code quality
• Provide comprehensive review feedback in GitHub comment

---

Summary of Changes

The PR adds support for MS_BIND|MS_REMOUNT to update per-mount flags on existing bind mounts, enforces readonly on wrapped mount inodes, preserves subtree roots when creating bind mounts, and adds C unit tests.

Key Changes:

1. mount.rs:

   • Added root_inner_inode field to MountFS to preserve subtree roots
   • Changed mount_flags from MountFlags to RwSem<MountFlags> for dynamic updates
   • Added ensure_mount_writable() checks in MountFSInode write operations
   • Added mount_subtree() method for bind mounts with preserved subtree roots
   • Updated mount() to use mount_subtree()

2. sys_mount.rs:

   • Added do_reconfigure_bind_mount() for MS_BIND|MS_REMOUNT
   • Updated do_bind_mount() and do_recursive_bind_mount() to use mount_subtree() with preserved subtree roots

3. Small updates to vcore.rs, initram.rs, mnt.rs to pass None for new root_inner_inode parameter

4. New test file test_mount_reconfigure.c with three comprehensive tests

Positive Aspects

✅ Good architectural design – preserving subtree roots via root_inner_inode is clean
✅ Proper synchronization – using RwSem for mount flags allows safe concurrent updates
✅ Comprehensive test coverage – tests cover readonly remount, self-bind subdir remount, and subtree root preservation
✅ Readonly enforcement – ensure_mount_writable() added to all relevant IndexNode operations
✅ Backward compatibility – existing call sites updated with None for new parameter

Potential Issues and Suggestions

1. Missing Validation in do_reconfigure_bind_mount()

The function doesn't verify that the target mount is actually a bind mount. While MS_BIND|MS_REMOUNT should only be used on bind mounts, it's worth adding a sanity check (e.g., verifying the mount has a source path). This would help catch misuse.

Fix this →

2. Incomplete Flag Enforcement

The ensure_mount_writable() only checks RDONLY. Other mount flags like NOEXEC, NODEV, NOSUID, NOATIME, etc., are not enforced in the VFS layer. While this matches the current state before the PR, consider adding enforcement for these flags where appropriate (e.g., NOEXEC on execve path, NODEV on device file access).

Suggestion: Track as future improvement.

3. Potential Downcast Failure in do_bind_mount()

The code assumes target_inode is a MountFSInode:

target_inode
    .clone()
    .downcast_arc::<MountFSInode>()
    .ok_or(SystemError::EINVAL)?

If target_inode is not a MountFSInode (e.g., a raw filesystem inode), this will fail with EINVAL. The previous code used target_inode.mount() which might have handled this differently. Verify that target_inode is always a MountFSInode in bind mount contexts.

Fix this →

4. Error Handling in mount_subtree()

The function calls propagate_mount() but only logs a warning on failure. This matches existing behavior in mount(), but consider whether propagation failure should fail the entire operation (currently it doesn't).

5. Test Cleanup

The tests could be more robust in cleanup on failure. Currently, some error paths may not clean up all temporary directories. Consider using atexit or a cleanup stack.

Minor: Test uses hardcoded /tmp paths; ensure they don't conflict with parallel test runs (they use unique names, so okay).

Code Quality Notes

• Comments: Good Chinese comments explaining the purpose of root_inner_inode
• Naming: mount_subtree clearly indicates its purpose
• Error messages: Could be more descriptive in some places (e.g., "target is not on a mount")
• Consistency: The PR maintains existing code patterns

Recommendations

1. Add validation for bind mount in do_reconfigure_bind_mount()
2. Consider adding a helper to enforce other mount flags (future PR)
3. Verify downcast safety in bind mount code paths
4. Consider adding a comment about why propagation failure is only a warning

Final Verdict

This is a well‑implemented feature that extends DragonOS's VFS capabilities to match Linux behavior for bind mount reconfiguration. The changes are minimal, focused, and come with good test coverage. With the minor improvements suggested above, this PR is ready for merge.

---

Review completed. The PR successfully implements bind mount reconfiguration with proper readonly enforcement and subtree root preservation.

View job run

[fslongjin]

🔍 自动化代码审查报告 — PR #1809

使用 bug-hunt-diff工具对 diff 进行多轮检测 + 验证，Linux 参考实现：linux-6.6.21。

总计发现 5 个问题（high×2, medium×2, low×1）

---

🔴 [HIGH] Missing mount-root check in do_reconfigure_bind_mount

文件: kernel/src/filesystem/vfs/syscall/sys_mount.rs:269-305

Linux do_reconfigure_mnt 要求 path_mounted(path) 检查（path->dentry == path->mnt->mnt_root），即 target 必须是其所在 mount 的根。当前实现无此检查。

复现场景：用户传入 mount 内部的普通子目录路径（非 mountpoint），target_inode.fs() 返回包含它的父 mount，函数会悄无声息地修改错误 mount 的 flags，而不是返回 EINVAL。

// 当前实现 — 对任意 MountFSInode 都会成功 downcast
let target_mfs = target_inode.fs().downcast_arc::<MountFS>().ok_or_else(|| {
    SystemError::EINVAL
})?;
// ← 缺少：该 inode 是否为其所在 mount 的 root 的检查

Linux 参考 (namespace.c:2826):

if (!path_mounted(path))    // path->dentry == path->mnt->mnt_root
    return -EINVAL;

修复建议：is_mountpoint_root 已在同文件 line 7 import 且在 do_new_mount 中使用，直接复用：

fn do_reconfigure_bind_mount(target_inode: Arc<dyn IndexNode>, ...) -> Result<(), SystemError> {
+   if !is_mountpoint_root(&target_inode) {
+       return Err(SystemError::EINVAL);
+   }
    let target_mfs = target_inode.fs().downcast_arc::<MountFS>()...;
    // ...
}

---

🔴 [HIGH] is_mountpoint_root 与新 root_inner_inode 语义不一致

文件: kernel/src/filesystem/vfs/mount.rs (is_mountpoint_root vs mountpoint_root_inode)

PR 将 mountpoint_root_inode() 改为返回 root_inner_inode（子目录），但 is_mountpoint_root() 仍与底层文件系统的全局根 inner_filesystem.root_inode() 比较。

对子目录 bind mount 的根节点，is_mountpoint_root() 始终返回 false，导致：

• umount() 返回 EINVAL — 子目录 bind mount 无法卸载
• do_parent() 跨 mount 向上遍历失败（.. 路径出错）
• absolute_path() 路径生成异常

// mountpoint_root_inode() — PR 已修改为使用 root_inner_inode：
inner_inode: self.root_inner_inode.clone(),   // ← 可能是子目录

// is_mountpoint_root() — PR 未修改，仍比较 FS 全局根：
return Ok(self.inner_inode.fs().root_inode().metadata()?.inode_id  // 始终是 FS 全局根
    == self.inner_inode.metadata()?.inode_id);                     // 子目录 ≠ 全局根 → false

修复建议：

fn is_mountpoint_root(&self) -> Result<bool, SystemError> {
    Ok(self.inner_inode.metadata()?.inode_id
        == self.mount_fs.root_inner_inode().metadata()?.inode_id)
}

---

🟡 [MEDIUM] write_sync 和 setxattr 遗漏 RDONLY 检查

文件: kernel/src/filesystem/vfs/mount.rs (write_sync ~line 1126, setxattr ~line 1134)

PR 为 MountFSInode 的 13 个写操作添加了 ensure_mount_writable() 保护，但遗漏了：

• write_sync — 被 page cache writeback（AsyncPageCacheBackend）调用
• setxattr — 被 xattr 系统调用调用

bind-remount 设为 RDONLY 后：write_at → EROFS ✓，但 page cache 回写和 xattr 写入仍会成功 ✗

修复建议：

fn write_sync(&self, offset: usize, buf: &[u8]) -> Result<usize, SystemError> {
+   self.ensure_mount_writable()?;
    self.inner_inode.write_sync(offset, buf)
}

fn setxattr(&self, name: &str, value: &[u8]) -> Result<usize, SystemError> {
+   self.ensure_mount_writable()?;
    self.inner_inode.setxattr(name, value)
}

---

🟡 [MEDIUM] 并发：do_reconfigure_bind_mount 中 mount_flags 的 RMW 操作非原子

文件: kernel/src/filesystem/vfs/syscall/sys_mount.rs:292-303

当前实现分两步操作，中间没有持锁：

let mut merged_flags = target_mfs.mount_flags();  // 读锁取值后立即释放
// ← 此窗口内其他线程可修改 flags
target_mfs.set_mount_flags(merged_flags);          // 写锁存值 — 可能覆盖并发更新

Linux 通过 lock_mount_hash() 全局自旋锁保护整个 read → compute → write 序列，DragonOS 无等效机制。

修复建议：将整个 RMW 放在单个写锁保护下，或为 MountFS 添加接受 closure 的原子 reconfigure_flags 方法。

---

🔵 [LOW] STRICTATIME 被持久存储为 mount flag（与 Linux 语义不符）

文件: kernel/src/filesystem/vfs/syscall/sys_mount.rs:302

Linux 无 MNT_STRICTATIME 持久标志位（MNT_ATIME_MASK = MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME）。MS_STRICTATIME 只是清除其他 atime 标志的指令，不被存储。

DragonOS 常规 mount 路径（lines 222–225）正确处理：清除 RELATIME/NOATIME，不存储 STRICTATIME。但新 bind-remount 路径直接存储了该 bit，导致 /proc/mounts 输出与 Linux 不一致。

修复建议：

// do_reconfigure_bind_mount 中，计算 merged_flags 后：
if requested_flags.contains(MountFlags::STRICTATIME) {
    merged_flags.remove(MountFlags::NOATIME | MountFlags::NODIRATIME
        | MountFlags::RELATIME | MountFlags::STRICTATIME);
    // 不 insert STRICTATIME 本身
}

---

已排除的误报：mount_subtree 中 add_mount 部分失败回滚问题（pre-existing）、ensure_mount_writable 不检查祖先 mount（Linux 正确行为——各 mount flags 相互独立）、reconfigurable_flags 常量重复定义（当前无分歧，仅代码质量问题）。