From dfd96d0752aaedf5b50f77b27b50dff013c4daea Mon Sep 17 00:00:00 2001 From: wangyining Date: Fri, 10 Jul 2026 10:01:35 +0800 Subject: [PATCH 1/4] Align VFS namespace locking with Linux --- fs/kvfs/docs/design.md | 77 +++++- fs/kvfs/docs/security.md | 61 +++-- fs/kvfs/src/lib.rs | 3 +- fs/kvfs/src/mount.rs | 10 +- fs/kvfs/src/namei.rs | 21 +- fs/kvfs/src/node/dentry.rs | 493 ++++++++++++++++++++++++++++--------- fs/kvfs/src/node/inode.rs | 19 +- fs/kvfs/src/node/mod.rs | 3 +- fs/kvfs/src/super_block.rs | 12 +- 9 files changed, 541 insertions(+), 158 deletions(-) diff --git a/fs/kvfs/docs/design.md b/fs/kvfs/docs/design.md index cd4f8c829..1bf874fca 100644 --- a/fs/kvfs/docs/design.md +++ b/fs/kvfs/docs/design.md @@ -4,7 +4,9 @@ `kvfs` 提供 Linux 风格的 VFS 对象模型，包括 superblock、mount、dentry、inode、 open file、路径解析和通用文件系统 helper。具体文件系统通过 inode/file operation -traits 接入，POSIX 层负责把 syscall ABI 参数转换成 VFS 语义对象。 +traits 接入，POSIX 层负责把 syscall ABI 参数转换成 VFS 语义对象。`kvfs` 同时拥有 +namespace validation、lock ordering、dcache identity、类型化 operation flags，以及 +`VfsInode` 上的 page-cache attachment。 权限模型与 Linux 保持同一层次：`kcred` 定义 `Cred`，syscall 层从当前 task 取得一次 credential snapshot，`kvfs` 接收显式 `&Cred` 并完成路径遍历和通用 DAC。`kvfs` 不 @@ -32,12 +34,23 @@ current task kcred::Cred POSIX syscall ABI --> Filename --> Nameidata methods --> Path/VfsInode::permission | | +-- open --> OpenHow --> OpenParams ---+--> VfsFile { f_cred: Arc } - | - +-- flags: OpenFlags + | | + | +-- flags: OpenFlags | - +-- rename --> RenameFlags --> Path/Dentry --> InodeDirOperations + +-- rename --> RenameFlags --> Path / Dentry --> InodeDirOperations | +-- statfs <---------------- StatFsFlags <---- filesystem + +Mount / Path + | + v +Dentry ---- namespace location (parent, name) + | | + v v +VfsInode child cache + | + v +filesystem operation traits ``` 原始整数只存在于 ABI 或兼容入口。进入 VFS 后，不同 flags 家族由不同 bitflags @@ -53,6 +66,10 @@ POSIX syscall ABI --> Filename --> Nameidata methods --> Path/VfsInode::permissi 文件系统私有状态，不重复保存 root；这对应 Linux 中 `super_block.s_root` 的所有权 边界，也避免私有状态和 root inode 之间形成引用环。 +`Dentry` 是可移动的 namespace 对象。rename 保留 source dentry 和 inode identity，只 +改变 dentry 的位置和 cache membership。inode 持有文件状态和 address space，因此 +rename 不会 flush 文件数据，也不会替换 PageCache identity。 + 每个 live backing inode number 通过 filesystem `InodeCache` 复用同一个 `VfsInode`；hard link、rename 和重复 lookup 因此共享 AddressSpace/PageCache。具体文件系统完成 mutation 后，operation callback 已持有 `VfsInode` 时可用 `update_metadata_after_backing_change()` 或 @@ -67,13 +84,17 @@ credential 的生命周期由 syscall 持有的 `Arc` 保证；对象字段不 ## 调用约束 / 执行上下文 -路径操作会获取 mutex、分配对象并调用具体文件系统，可能阻塞，不适用于中断上下文。 -这些 API 依赖分配器和正常内核运行环境。POSIX 路径通常需要当前进程的 mount、root -和 cwd；纯 VFS 对象方法只依赖显式传入的对象。 +路径和 namespace 操作会获取 sleepable lock、分配对象并调用具体文件系统，可能阻塞， +不适用于中断上下文，也不能在持有 spinlock 时调用。这些 API 依赖调度器、分配器和 +正常内核运行环境。POSIX 路径通常需要当前进程的 mount、root 和 cwd；纯 VFS 对象 +方法只依赖显式传入的对象。 一次完整 pathname 操作必须复用同一个 credential snapshot。调用者不能在每个路径 组件重新查询 current task，否则并发 credential commit 可能让同一次解析混用身份。 +文件系统 callback 可在 I/O 上阻塞，但不能在持有同一组 VFS inode namespace lock 时 +重新进入这些 VFS namespace 操作，否则会形成自锁。 + `init_anon_inodefs()` 必须在 boot/runtime 初始化阶段调用，早于普通任务和并行单元测试 创建匿名 inode 文件。`AnonInodeFs::global()` 只读取已经初始化的 singleton，不会在 运行时首次访问路径中构造 VFS 对象；未初始化时会 panic 暴露启动顺序错误。 @@ -84,6 +105,23 @@ open 在入口清理 legacy flags，校验已知位，生成 access mode、open flags。namei 使用这些语义执行查找、创建和最终 open，不再直接组合 `O_CREAT` 与 `O_EXCL`。 +VFS 采用 Linux directory-locking ownership model： + +- slow lookup 对父目录 inode 加 shared namespace lock； +- create 对父目录 inode 加 exclusive namespace lock； +- unlink 和 rmdir 先锁父目录，再锁 victim inode； +- link 先锁新父目录，再锁非目录 source inode； +- same-directory rename 只锁一次父目录； +- cross-directory rename 先获取 superblock topology mutex，再按拓扑顺序锁父目录。 + +rename 在两个父目录稳定后解析 source 和 target，然后先锁参与的子目录，再锁非目录 +inode。子目录按 source 到 target 顺序，非目录 inode 按指针值排序。类型、空目录、 +flag 和祖先关系检查都在所需锁持有期间完成；文件系统 callback 成功后，VFS 再提交 +dentry cache move 或 exchange。 + +open-create 在同一个父目录 exclusive lock 下完成最终 lookup 和可能的 create，避免 +`O_EXCL` 与 lookup/create 竞争。 + ### 路径遍历与 DAC namei 在进入每个目录并查找下一组件前检查目录 search permission (`MAY_EXEC`)；最终 @@ -173,6 +211,24 @@ Namespace callback 在 inode namespace lock 下运行。文件系统应在回调 mutation result 同步到受影响的 live inode；dentry cache 的删除/rebind 仍由 KVFS 在成功 返回后完成。 +全局 namespace lock 顺序为： + +```text +superblock rename mutex + -> parent-directory namespace locks + -> child-directory namespace locks + -> non-directory namespace locks (pointer order) + -> dentry cache and location locks +``` + +`SuperBlock::rename_mutex` 对应 Linux `s_vfs_rename_mutex`，不是 dcache rename +seqlock；它只在 cross-directory rename 中获取。`VfsInode::namespace_lock` 表达 +Linux `inode->i_rwsem` 中和 namespace 相关的子集。`DentryLocation` 用一个 `RwLock` +同时保护 `parent` 和 `name`，读者不会观察到混合位置。 + +child cache 仍使用 mutex。RCU、seqcount lookup 和 lock-free dcache traversal 在当前 +锁模型和回归覆盖稳定前保持在范围外。 + ## 设计决策 - ABI carrier 与内核语义类型分离，转换尽量靠近边界。 @@ -187,6 +243,10 @@ mutation result 同步到受影响的 live inode；dentry cache 的删除/rebind 复杂 VFS 全局对象的生命周期：启动时构造，运行时只复用。 - PageCache resize 顺序由 AddressSpace operation 契约表达，不允许文件系统绕过 inode-owned Mapping 建立第二套数据 cache。 +- 锁由语义 VFS 对象持有，而不是由 filesystem bridge 持有。 +- `RenameData` 对应 Linux `struct renamedata`，在 VFS orchestration 中携带 rename + participants。 +- namespace lock 使用 blocking lock，因为文件系统 callback 可以执行 I/O。 ## Drop / 资源释放 @@ -205,3 +265,6 @@ parent children 弱索引与 superblock dentry cache 中移除 dentry；仍有 P - FAT 等不能原生表达 Unix UID/GID 的后端不能完整持久化创建者身份。 - 当前 POSIX rename 路径不支持 `RENAME_WHITEOUT`。 - superblock dentry cache 尚无 Linux 风格 LRU/shrinker。 +- fast lookup 仍是 mutex-based，没有 RCU 或 rename sequence validation。 +- layered filesystem 的跨文件系统 lock rank 尚未建模。 +- mount topology 同步与 superblock rename mutex 仍是不同机制。 diff --git a/fs/kvfs/docs/security.md b/fs/kvfs/docs/security.md index a4495d1aa..54995509c 100644 --- a/fs/kvfs/docs/security.md +++ b/fs/kvfs/docs/security.md @@ -4,7 +4,8 @@ 用户提供的路径、open flags、rename flags 和 mount flags 不可信。POSIX syscall 层 负责复制用户内存并完成 ABI 初步校验；`kvfs` 接收内核所有的字符串和类型化 flags。 -具体文件系统返回的目录项与元数据也必须视为可能失败的外部输入。 +具体文件系统返回的目录项与元数据也必须视为可能失败的外部输入。`kvfs` 在提交 +namespace 状态前校验 name、mount relationship、类型、topology 和 operation flags。 `Cred` 来自可信 task 状态，但其 UID/GID 不是“特权保证”，只能作为 DAC 输入。 调用者负责在操作入口取得一个稳定 `Arc`；`kvfs` 负责让整次路径遍历、最终检查 @@ -12,7 +13,8 @@ ## 外部边界 / 攻击面 -- `Filename::open_with_flags_at` 和 `dentry_open` 是保留 raw `O_*` 的兼容入口。 +- `Filename::open_with_flags_at` 和 `dentry_open` 是保留 raw `O_*` 的兼容入口，会把 + 原始位规范化为 `OpenParams` 与 `OpenFlags`。 - `sys_renameat2` 将 raw rename bits 转换为 `RenameFlags` 后才进入 VFS。 - 文件系统 operation traits 可返回磁盘、网络或设备后端产生的错误与元数据。 - 所有 pathname 与 namespace mutation API 的 `&Cred` 是权限边界；省略或替换它会改变 @@ -24,15 +26,19 @@ - 文件系统 mutation result 会进入 VFS cached inode attributes；错误 identity 或 immutable geometry 不可信。 -`kvfs` 不直接解引用用户指针，不直接访问 MMIO、PIO 或 DMA。 +`kvfs` 不直接解引用用户指针，不直接访问 MMIO、PIO、DMA 或 architecture FFI。 ## unsafe 代码清单 -当前 `fs/kvfs/src` 没有 `unsafe` block。内存安全依赖 Rust 所有权以及 operation -trait 的 `Send + Sync` 约束。 +当前 `fs/kvfs/src` 没有 `unsafe` block。namespace model 由 safe Rust lock、 +`Arc` ownership 和 operation trait 的 `Send + Sync` 约束维护内存安全。 ## 内存安全不变量 +- `LockedDentry` 的 location guard 限定借用的 dentry name 生命周期。 +- `parent` 和 `name` 在同一个 location write lock 下同时替换。 +- rename 期间 source dentry 和对应 `VfsInode` 保持存活。 +- filesystem callback 不能直接修改 VFS cache internals。 - 每个 raw flags 家族必须在边界转换为对应 bitflags 类型。 - 未知 open/rename 位不得进入内部 namespace 或 open 算法。 - `AtomicU32` 中的 `f_flags` 只通过 `OpenFlags` API 读写。 @@ -67,12 +73,20 @@ trait 的 `Send + Sync` 约束。 children map 与 superblock dcache 不嵌套持锁；namespace 操作先更新 parent 弱索引， 再更新 dcache 强所有权。类型化 flags 是不可变值快照，不提供共享可变状态。 +所有 namespace lock 都是 sleepable lock。全局顺序为 superblock topology、父目录、 +子目录、非目录 inode，最后才是 dentry cache/location。cross-directory 父目录锁按 +ancestor-first 顺序；互不为祖先时先锁 source parent。非目录 inode 按指针值排序。 + 匿名 inode pseudo fs 的 singleton 由 `Once` 发布，但不允许普通运行时路径触发初始化； 并发创建匿名文件只共享已经发布的 mount/inode，不竞争初始化闭包。 Credential 本身是不可变 `Arc` 快照。权限检查期间不持有 task credential 锁，也不 重新读取 current task，因此并发 `commit_creds()` 只能影响下一次操作。 +后端可以在持有 VFS lock 时 sleep。对同一 namespace 对象重新进入 VFS namespace +操作不受支持，可能导致 deadlock。layered filesystem 只能在 upper VFS lock 之后获取 +lower filesystem lock；在推广此类嵌套前还需要明确的跨文件系统 lock rank。 + ## 威胁分析 | 编号 | 威胁描述 | 影响等级 | 触发条件 | 应对措施 | @@ -91,9 +105,13 @@ Credential 本身是不可变 `Arc` 快照。权限检查期间不持有 task cr | T-12 | `ftruncate` 因当前 pathname DAC 被错误拒绝或绕过写模式 | 中 | fd 操作复用 pathname truncate | `VfsFile::truncate` 先验证 `FMode::WRITE`，再走 opened truncate | | T-13 | 非 owner 直接修改 inode owner、mode 或显式时间 | 高 | syscall 或调用者直接进入后端 `setattr` | `Path` metadata API 在 mount write check 后统一执行 Linux owner/group/write policy | | T-14 | pathname socket 或其它 special inode 固定为 root | 高 | simple filesystem 动态 mknod 绕过 owner helper | `SimpleDir` mknod 使用 `inode_init_owner()`，仅支持持久插入的目录实现开放创建 | -| T-06 | core mutation result 污染错误的 live inode identity | 高 | bridge 把一个 inode 的 metadata 写入另一个 `VfsInode` | cached metadata refresh 校验 identity、node type、block geometry 和 `rdev` | -| T-07 | truncate 先释放磁盘 block、后失效 PageCache/mmap，造成 stale access | 高 | 文件系统没有保持 split truncate 顺序 | 文件系统 `set_len()` 执行 backing prepare → `truncate_pagecache()`/view invalidation → backing finish | -| T-08 | unlink 时过早触发磁盘 inode 回收 | 高 | dentry removal 与最后 open-file 引用混为一谈 | `Arc` inode identity 延迟 final teardown，磁盘回收只在 superblock `evict_inode()` hook 中执行 | +| T-15 | core mutation result 污染错误的 live inode identity | 高 | bridge 把一个 inode 的 metadata 写入另一个 `VfsInode` | cached metadata refresh 校验 identity、node type、block geometry 和 `rdev` | +| T-16 | truncate 先释放磁盘 block、后失效 PageCache/mmap，造成 stale access | 高 | 文件系统没有保持 split truncate 顺序 | 文件系统 `set_len()` 执行 backing prepare -> `truncate_pagecache()`/view invalidation -> backing finish | +| T-17 | unlink 时过早触发磁盘 inode 回收 | 高 | dentry removal 与最后 open-file 引用混为一谈 | `Arc` inode identity 延迟 final teardown，磁盘回收只在 superblock `evict_inode()` hook 中执行 | +| T-18 | 并发 rename 创建目录环 | 高 | cross-directory rename 未序列化 topology 或未在锁内检查祖先关系 | topology mutex、稳定 parent lock 和 ancestry check 拒绝该操作 | +| T-19 | create 与 lookup、rmdir 或 replacement 竞争 | 高 | final lookup 和 create 分离持锁 | final lookup 和 mutation 在父目录 exclusive lock 下执行，victim lock 保护删除 | +| T-20 | 反向 cross-directory rename 死锁 | 高 | 两个线程按相反顺序锁父目录 | 一个 topology mutex 串行化 topology mutation，父目录按拓扑顺序加锁 | +| T-21 | dentry name 和 parent 不一致 | 中 | parent/name 分开更新或读者观察中间状态 | 两个字段在同一个 location write lock 下替换 | ## 故障模式与影响分析（FMEA） @@ -106,7 +124,11 @@ Credential 本身是不可变 `Arc` 快照。权限检查期间不持有 task cr | F-05 | 匿名 inode fs 未初始化即使用 | boot 初始化顺序缺失 | 当前调用 panic | 暴露启动顺序回归 | 3 | `fs_boot::prepare_namespace()` 显式初始化，测试覆盖启动路径 | | F-06 | 权限检查失败 | mode、owner 或组不允许请求 | 当前 VFS 操作返回 `PermissionDenied` | namespace 和 inode 状态保持不变 | 3 | 在后端 mutation 前完成通用检查并传播错误 | | F-07 | 后端不能保存 Unix owner | 磁盘格式没有 UID/GID | getattr 无法完整反映创建身份 | DAC 语义受文件系统能力限制 | 3 | 文档明确后端限制；支持 owner 的后端必须持久化 helper 结果 | -| F-06 | split truncate 在 backing prepare 后 cache invalidation 失败 | 分配或 mapping invalidation 错误 | 当前 truncate 返回失败 | backing inode 可能保持 orphan/recovery state | 2 | 由具体文件系统的持久化 recovery protocol 收敛，禁止静默执行 finish | +| F-08 | split truncate 在 backing prepare 后 cache invalidation 失败 | 分配或 mapping invalidation 错误 | 当前 truncate 返回失败 | backing inode 可能保持 orphan/recovery state | 2 | 由具体文件系统的持久化 recovery protocol 收敛，禁止静默执行 finish | + +校验失败会在 filesystem callback 前返回 typed VFS error。后端失败时 dentry cache +location 保持不变，因为 cache commit 只在 callback 成功后执行；cache commit 是内存内 +不可失败步骤，持久化操作的 rollback 与 logging 仍由后端负责。 ## 已知限制 @@ -116,8 +138,11 @@ Credential 本身是不可变 `Arc` 快照。权限检查期间不持有 task cr 建立，但尚未定义额外语义位；当前调用使用 empty flags。 - superblock dentry cache 尚未实现 Linux 风格的 LRU/shrinker，当前依赖 namespace 删除和卸载路径主动驱逐。 -KVFS 提供 Mapping view -invalidation 通知，但各文件系统仍需用 live mmap/truncate case 验证自身接线。 +- KVFS 提供 Mapping view invalidation 通知，但各文件系统仍需用 live mmap/truncate + case 验证自身接线。 +- fast lookup 仍是 mutex-based，没有 RCU 或 rename sequence validation。 +- layered filesystem 的 lock ordering 尚未建模。 +- mount topology synchronization 与 superblock rename mutex 是不同机制。 ## 审计清单 @@ -134,7 +159,13 @@ invalidation 通知，但各文件系统仍需用 live mmap/truncate case 验证 - 中间目录 search、最终 inode 和父目录 mutation 权限是否分别在正确阶段检查。 - 新建 inode 是否使用 `inode_init_owner()`，后端是否持久化 UID/GID。 - fd-based 操作是否使用 open file mode/`f_cred`，而不是重新执行 pathname 授权。 -- backing mutation 后是否刷新所有受影响的 live inode，而不是只返回新 dentry？ -- truncate 是否通过 `set_len()` 与 `truncate_pagecache()` 保持 backing prepare、Mapping invalidation 和 - backing finish 顺序？ -- `release()` 与 final `evict_inode()` 是否保持为两个不同生命周期阶段？ +- backing mutation 后是否刷新所有受影响的 live inode，而不是只返回新 dentry。 +- truncate 是否通过 `set_len()` 与 `truncate_pagecache()` 保持 backing prepare、Mapping + invalidation 和 backing finish 顺序。 +- `release()` 与 final `evict_inode()` 是否保持为两个不同生命周期阶段。 +- 每个 namespace mutation 是否获取父目录 exclusive lock。 +- unlink/rmdir 和 rename replacement 是否锁住 victim inode。 +- cross-directory rename 是否先获取 topology mutex，再获取 inode lock。 +- directory lock 是否先于 non-directory lock 获取。 +- callback 是否避免重新进入同一组 VFS namespace 对象。 +- cache commit 是否只在 backend success 后执行。 diff --git a/fs/kvfs/src/lib.rs b/fs/kvfs/src/lib.rs index 90576f8ab..7a9d14183 100644 --- a/fs/kvfs/src/lib.rs +++ b/fs/kvfs/src/lib.rs @@ -51,7 +51,8 @@ pub use node::{ VfsInodeInit, WeakVfsInode, bdev_add, bdev_del, cdev_add, cdev_del, inode_init_owner, }; pub(crate) use node::{ - DentryKey, d_inode, d_is_dir, d_is_negative, d_is_symlink, d_really_is_positive, + DentryKey, LookupCreateResult, d_inode, d_is_dir, d_is_negative, d_is_symlink, + d_really_is_positive, }; pub use open_flags::OpenFlags; pub(crate) use open_flags::{AccMode, OpenHow, OpenParams}; diff --git a/fs/kvfs/src/mount.rs b/fs/kvfs/src/mount.rs index 26b082e33..60e4b7336 100644 --- a/fs/kvfs/src/mount.rs +++ b/fs/kvfs/src/mount.rs @@ -848,12 +848,6 @@ impl Path { new_dir.check_sticky(&new_path, cred)?; new_path.check_not_mountpoint()?; } - if !self.ptr_eq(new_dir) - && old_path.is_dir() - && old_path.dentry.is_ancestor_of(&new_dir.dentry)? - { - return Err(VfsError::InvalidInput); - } self.dentry .as_dir()? .rename(old_name, new_dir.dentry.as_dir()?, new_name, flags) @@ -869,7 +863,7 @@ impl Path { } self.check_sticky(&path, cred)?; path.check_not_mountpoint()?; - self.dentry.as_dir()?.unlink(name, false) + self.dentry.as_dir()?.unlink(name) } /// Remove a directory entry. @@ -882,7 +876,7 @@ impl Path { } self.check_sticky(&path, cred)?; path.check_not_mountpoint()?; - self.dentry.as_dir()?.unlink(name, true) + self.dentry.as_dir()?.rmdir(name) } /// Mount a filesystem at this path. diff --git a/fs/kvfs/src/namei.rs b/fs/kvfs/src/namei.rs index 920c81fd3..27b3542c4 100644 --- a/fs/kvfs/src/namei.rs +++ b/fs/kvfs/src/namei.rs @@ -701,6 +701,9 @@ impl<'a> Nameidata<'a> { .ok_or(VfsError::InvalidInput)?; let dir = self.path.dentry().as_dir()?; match dir.lookup(name) { + Ok(_) if flags.is_exclusive_create() => { + return Err(VfsError::AlreadyExists); + } Ok(entry) => return Ok(self.path.with_dentry(entry)), Err(err) if err.canonicalize() == VfsError::NotFound => {} Err(err) => return Err(err), @@ -717,12 +720,18 @@ impl<'a> Nameidata<'a> { cred, )?; - let inode = dir.vfs_inode(); - let entry = - inode.create_with_mode(dir, name, flags.mode(), flags.is_exclusive_create(), cred)?; - dir.insert_cache(name.to_owned(), entry.clone()); - file.mark_created(); - Ok(self.path.with_dentry(entry)) + match dir.lookup_or_create_with_mode( + name, + flags.mode(), + flags.is_exclusive_create(), + cred, + )? { + crate::LookupCreateResult::Existing(entry) => Ok(self.path.with_dentry(entry)), + crate::LookupCreateResult::Created(entry) => { + file.mark_created(); + Ok(self.path.with_dentry(entry)) + } + } } fn open_last_lookups( diff --git a/fs/kvfs/src/node/dentry.rs b/fs/kvfs/src/node/dentry.rs index 8ea439187..02ea9e2c7 100644 --- a/fs/kvfs/src/node/dentry.rs +++ b/fs/kvfs/src/node/dentry.rs @@ -25,7 +25,7 @@ use hashbrown::HashMap; use super::{DirEntrySink, NodeFlags, VfsInode}; use crate::{ DeviceId, Metadata, MetadataUpdate, Mutex, NodePermission, NodeType, RenameFlags, RwLock, - RwLockReadGuard, SuperBlock, VfsError, VfsResult, + RwLockReadGuard, SuperBlock, Umode, VfsError, VfsResult, path::{DOT, DOTDOT, MAX_NAME_LEN, PathBuf}, }; @@ -221,6 +221,20 @@ pub struct LockedDentry<'a> { location: RwLockReadGuard<'a, DentryLocation>, } +#[derive(Clone, Copy)] +struct RenameData<'a> { + old_parent: &'a Dentry, + old_name: &'a str, + new_parent: &'a Dentry, + new_name: &'a str, + flags: RenameFlags, +} + +pub(crate) enum LookupCreateResult { + Existing(Dentry), + Created(Dentry), +} + impl LockedDentry<'_> { /// Returns the dentry name within its parent directory. pub fn name(&self) -> &str { @@ -567,6 +581,14 @@ impl Dentry { Ok(false) } + fn tree_root(&self) -> Self { + let mut current = self.clone(); + while let Some(parent) = current.parent() { + current = parent; + } + current + } + pub(crate) fn collect_absolute_path(&self, components: &mut Vec) { let mut current = self.clone(); loop { @@ -700,14 +722,7 @@ impl Dentry { *self.0.location.write() = DentryLocation { parent, name }; } - fn commit_move( - &self, - src_name: &str, - src: &Dentry, - dst_dir: &Self, - dst_name: &str, - dst: &Dentry, - ) { + fn d_move(&self, src_name: &str, src: &Dentry, dst_dir: &Self, dst_name: &str, dst: &Dentry) { self.remove_cache_entry(src_name); if dst.is_really_positive() { dst_dir.forget_cache_entry(dst_name); @@ -719,7 +734,7 @@ impl Dentry { dst_dir.insert_cache(dst_name.to_owned(), src.clone()); } - fn commit_exchange( + fn d_exchange( &self, src_name: &str, src: &Dentry, @@ -784,6 +799,29 @@ impl Dentry { Ok(entry) } + pub(crate) fn lookup_or_create_with_mode( + &self, + name: &str, + mode: Umode, + exclusive: bool, + cred: &kcred::Cred, + ) -> VfsResult { + self.as_dir()?; + Self::verify_child_name(name)?; + let dir_inode = self.vfs_inode(); + let _namespace_guard = dir_inode.lock_namespace_exclusive(); + match self.lookup_no_namespace_lock(&dir_inode, name) { + Ok(_) if exclusive => Err(VfsError::AlreadyExists), + Ok(entry) => Ok(LookupCreateResult::Existing(entry)), + Err(err) if err.canonicalize() == VfsError::NotFound => { + let entry = dir_inode.create_with_mode(self, name, mode, exclusive, cred)?; + self.insert_cache(name.to_owned(), entry.clone()); + Ok(LookupCreateResult::Created(entry)) + } + Err(err) => Err(err), + } + } + /// Creates a directory child dentry below this directory. pub fn mkdir( &self, @@ -833,31 +871,55 @@ impl Dentry { pub fn link(&self, name: &str, node: &Dentry) -> VfsResult { self.as_dir()?; Self::verify_child_name(name)?; + if node.is_dir() { + return Err(VfsError::OperationNotPermitted); + } let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); + let source_inode = node.vfs_inode(); + let _source_guard = source_inode.lock_namespace_exclusive(); let entry = dir_inode.link(self, name, node)?; self.insert_cache(name.to_owned(), entry.clone()); Ok(entry) } - /// Unlinks a child dentry by name. - pub fn unlink(&self, name: &str, is_dir: bool) -> VfsResult<()> { + /// Unlinks a non-directory child by name. + pub fn unlink(&self, name: &str) -> VfsResult<()> { self.as_dir()?; Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); let entry = self.lookup_no_namespace_lock(&dir_inode, name)?; - match (entry.is_dir(), is_dir) { - (true, false) => return Err(VfsError::IsADirectory), - (false, true) => return Err(VfsError::NotADirectory), - _ => {} + let victim_inode = entry.vfs_inode(); + let _victim_guard = victim_inode.lock_namespace_exclusive(); + if entry.is_dir() { + return Err(VfsError::IsADirectory); } - dir_inode.unlink(&entry)?; self.forget_cache_entry(name); Ok(()) } + /// Removes an empty directory child by name. + pub fn rmdir(&self, name: &str) -> VfsResult<()> { + self.as_dir()?; + Self::verify_child_name(name)?; + let dir_inode = self.vfs_inode(); + let _namespace_guard = dir_inode.lock_namespace_exclusive(); + let entry = self.lookup_no_namespace_lock(&dir_inode, name)?; + let victim_inode = entry.vfs_inode(); + let _victim_guard = victim_inode.lock_namespace_exclusive(); + if !entry.is_dir() { + return Err(VfsError::NotADirectory); + } + if entry.has_children()? { + return Err(VfsError::DirectoryNotEmpty); + } + dir_inode.rmdir(&entry)?; + self.forget_cache_entry(name); + Ok(()) + } + /// Returns whether the directory contains children. pub fn has_children(&self) -> VfsResult { self.as_dir()?; @@ -894,101 +956,14 @@ impl Dentry { if !supported_flags.contains(flags) { return Err(VfsError::InvalidInput); } - let super_block = self.super_block(); - let _rename_guard = super_block - .as_ref() - .map(|super_block| super_block.lock_rename()); - let old_dir_inode = self.vfs_inode(); - let new_dir_inode = dst_dir.vfs_inode(); - - if Arc::ptr_eq(&old_dir_inode, &new_dir_inode) { - let _old_guard = old_dir_inode.lock_namespace_exclusive(); - return self.rename_no_namespace_lock( - &old_dir_inode, - src_name, - dst_dir, - &new_dir_inode, - dst_name, - flags, - ); - } - - if (Arc::as_ptr(&old_dir_inode) as usize) <= (Arc::as_ptr(&new_dir_inode) as usize) { - let _old_guard = old_dir_inode.lock_namespace_exclusive(); - let _new_guard = new_dir_inode.lock_namespace_exclusive(); - self.rename_no_namespace_lock( - &old_dir_inode, - src_name, - dst_dir, - &new_dir_inode, - dst_name, - flags, - ) - } else { - let _new_guard = new_dir_inode.lock_namespace_exclusive(); - let _old_guard = old_dir_inode.lock_namespace_exclusive(); - self.rename_no_namespace_lock( - &old_dir_inode, - src_name, - dst_dir, - &new_dir_inode, - dst_name, - flags, - ) - } - } - - fn rename_no_namespace_lock( - &self, - old_dir_inode: &VfsInode, - src_name: &str, - dst_dir: &Self, - new_dir_inode: &VfsInode, - dst_name: &str, - flags: RenameFlags, - ) -> VfsResult<()> { - let src = self.lookup_no_namespace_lock(old_dir_inode, src_name)?; - let dst = match dst_dir.lookup_no_namespace_lock(new_dir_inode, dst_name) { - Ok(dst) => { - if src.is_same_inode(&dst) { - return Ok(()); - } - if !flags.contains(RenameFlags::EXCHANGE) { - if src.node_type() == NodeType::Directory { - if dst.node_type() != NodeType::Directory { - return Err(VfsError::NotADirectory); - } - if dst.has_children()? { - return Err(VfsError::DirectoryNotEmpty); - } - } else if dst.node_type() == NodeType::Directory { - return Err(VfsError::IsADirectory); - } - if flags.contains(RenameFlags::NOREPLACE) { - return Err(VfsError::AlreadyExists); - } - } - dst - } - Err(err) - if err.canonicalize() == VfsError::NotFound - && flags.contains(RenameFlags::EXCHANGE) => - { - return Err(VfsError::NotFound); - } - Err(err) if err.canonicalize() == VfsError::NotFound => { - Dentry::new_negative(Some(dst_dir.clone()), dst_name.to_owned()) - } - Err(err) => return Err(err), - }; - - old_dir_inode.rename(&src, new_dir_inode, &dst, flags)?; - if flags.contains(RenameFlags::EXCHANGE) { - self.commit_exchange(src_name, &src, dst_dir, dst_name, &dst); - } else { - self.commit_move(src_name, &src, dst_dir, dst_name, &dst); + RenameData { + old_parent: self, + old_name: src_name, + new_parent: dst_dir, + new_name: dst_name, + flags, } - Ok(()) + .execute() } /// Emits entries currently present in this dentry's child cache. @@ -1062,6 +1037,196 @@ impl Dentry { } } +impl RenameData<'_> { + fn execute(self) -> VfsResult<()> { + let old_dir_inode = self.old_parent.vfs_inode(); + let new_dir_inode = self.new_parent.vfs_inode(); + + if Arc::ptr_eq(&old_dir_inode, &new_dir_inode) { + let _parent_guard = old_dir_inode.lock_namespace_exclusive(); + return self.execute_with_parents_locked(&old_dir_inode, &new_dir_inode, false); + } + + let old_super_block = self + .old_parent + .super_block() + .ok_or(VfsError::CrossesDevices)?; + let new_super_block = self + .new_parent + .super_block() + .ok_or(VfsError::CrossesDevices)?; + if !Arc::ptr_eq(&old_super_block, &new_super_block) { + return Err(VfsError::CrossesDevices); + } + + let _topology_guard = old_super_block.lock_rename_topology(); + let old_parent_first = self.old_parent_first()?; + if old_parent_first { + let _old_parent_guard = old_dir_inode.lock_namespace_exclusive(); + let _new_parent_guard = new_dir_inode.lock_namespace_exclusive(); + self.execute_with_parents_locked(&old_dir_inode, &new_dir_inode, true) + } else { + let _new_parent_guard = new_dir_inode.lock_namespace_exclusive(); + let _old_parent_guard = old_dir_inode.lock_namespace_exclusive(); + self.execute_with_parents_locked(&old_dir_inode, &new_dir_inode, true) + } + } + + fn old_parent_first(self) -> VfsResult { + if self.old_parent.is_ancestor_of(self.new_parent)? { + return Ok(true); + } + if self.new_parent.is_ancestor_of(self.old_parent)? { + return Ok(false); + } + if !self + .old_parent + .tree_root() + .ptr_eq(&self.new_parent.tree_root()) + { + return Err(VfsError::CrossesDevices); + } + Ok(true) + } + + fn execute_with_parents_locked( + self, + old_dir_inode: &VfsInode, + new_dir_inode: &VfsInode, + is_cross_directory: bool, + ) -> VfsResult<()> { + let source = self + .old_parent + .lookup_no_namespace_lock(old_dir_inode, self.old_name)?; + let target = match self + .new_parent + .lookup_no_namespace_lock(new_dir_inode, self.new_name) + { + Ok(target) => target, + Err(err) + if err.canonicalize() == VfsError::NotFound + && self.flags.contains(RenameFlags::EXCHANGE) => + { + return Err(VfsError::NotFound); + } + Err(err) if err.canonicalize() == VfsError::NotFound => { + Dentry::new_negative(Some(self.new_parent.clone()), self.new_name.to_owned()) + } + Err(err) => return Err(err), + }; + if target.is_really_positive() && source.is_same_inode(&target) { + return Ok(()); + } + + self.validate_topology(&source, &target)?; + let participant_inodes = self.participant_inodes(&source, &target, is_cross_directory); + let _participant_guards: Vec<_> = participant_inodes + .iter() + .map(|inode| inode.lock_namespace_exclusive()) + .collect(); + + self.validate_locked(&source, &target)?; + old_dir_inode.rename(&source, new_dir_inode, &target, self.flags)?; + if self.flags.contains(RenameFlags::EXCHANGE) { + self.old_parent.d_exchange( + self.old_name, + &source, + self.new_parent, + self.new_name, + &target, + ); + } else { + self.old_parent.d_move( + self.old_name, + &source, + self.new_parent, + self.new_name, + &target, + ); + } + Ok(()) + } + + fn validate_topology(self, source: &Dentry, target: &Dentry) -> VfsResult<()> { + if source.is_dir() && source.is_ancestor_of(self.new_parent)? { + return Err(VfsError::InvalidInput); + } + if target.is_really_positive() + && target.is_dir() + && target.is_ancestor_of(self.old_parent)? + { + return if self.flags.contains(RenameFlags::EXCHANGE) { + Err(VfsError::InvalidInput) + } else { + Err(VfsError::DirectoryNotEmpty) + }; + } + Ok(()) + } + + fn validate_locked(self, source: &Dentry, target: &Dentry) -> VfsResult<()> { + if self.flags.contains(RenameFlags::EXCHANGE) { + return Ok(()); + } + if self.flags.contains(RenameFlags::NOREPLACE) && target.is_really_positive() { + return Err(VfsError::AlreadyExists); + } + if !target.is_really_positive() { + return Ok(()); + } + + match (source.is_dir(), target.is_dir()) { + (true, false) => return Err(VfsError::NotADirectory), + (false, true) => return Err(VfsError::IsADirectory), + _ => {} + } + if target.is_dir() && target.has_children()? { + return Err(VfsError::DirectoryNotEmpty); + } + Ok(()) + } + + fn participant_inodes( + self, + source: &Dentry, + target: &Dentry, + is_cross_directory: bool, + ) -> Vec> { + let is_exchange = self.flags.contains(RenameFlags::EXCHANGE); + let mut directories = Vec::new(); + let mut non_directories = Vec::new(); + + if source.is_dir() { + if is_cross_directory { + directories.push(source.vfs_inode()); + } + } else { + non_directories.push(source.vfs_inode()); + } + + if target.is_really_positive() { + if target.is_dir() { + if is_cross_directory || !is_exchange { + directories.push(target.vfs_inode()); + } + } else { + non_directories.push(target.vfs_inode()); + } + } + + non_directories.sort_by_key(|inode| Arc::as_ptr(inode) as usize); + for inode in non_directories { + if !directories + .iter() + .any(|existing| Arc::ptr_eq(existing, &inode)) + { + directories.push(inode); + } + } + directories + } +} + #[cfg(unittest)] mod tests_dentry { use alloc::{string::String, sync::Arc, vec::Vec}; @@ -1240,6 +1405,9 @@ mod tests_dentry { struct MockDirOps { inode: u64, can_rename: bool, + can_remove: bool, + unlink_count: AtomicUsize, + rmdir_count: AtomicUsize, } impl MockDirOps { @@ -1247,6 +1415,9 @@ mod tests_dentry { Self { inode, can_rename: false, + can_remove: false, + unlink_count: AtomicUsize::new(0), + rmdir_count: AtomicUsize::new(0), } } @@ -1254,6 +1425,19 @@ mod tests_dentry { Self { inode, can_rename: true, + can_remove: false, + unlink_count: AtomicUsize::new(0), + rmdir_count: AtomicUsize::new(0), + } + } + + fn new_removable(inode: u64) -> Self { + Self { + inode, + can_rename: false, + can_remove: true, + unlink_count: AtomicUsize::new(0), + rmdir_count: AtomicUsize::new(0), } } } @@ -1329,7 +1513,21 @@ mod tests_dentry { } fn unlink(&self, _dir: &VfsInode, _dentry: &LockedDentry<'_>) -> VfsResult<()> { - Err(VfsError::OperationNotSupported) + self.unlink_count.fetch_add(1, Ordering::Relaxed); + if self.can_remove { + Ok(()) + } else { + Err(VfsError::OperationNotSupported) + } + } + + fn rmdir(&self, _dir: &VfsInode, _dentry: &LockedDentry<'_>) -> VfsResult<()> { + self.rmdir_count.fetch_add(1, Ordering::Relaxed); + if self.can_remove { + Ok(()) + } else { + Err(VfsError::OperationNotSupported) + } } fn rename( @@ -1413,6 +1611,22 @@ mod tests_dentry { Dentry::new_dir_from_inode(inode, parent, String::from(name)) } + fn make_removable_dir_entry( + inode: u64, + parent: Option, + name: &str, + ) -> (Dentry, Arc) { + let operations = Arc::new(MockDirOps::new_removable(inode)); + let inode = VfsInode::new_openable_dir( + operations.clone(), + inode_init(inode, NodeType::Directory, 0), + ); + ( + Dentry::new_dir_from_inode(inode, parent, String::from(name)), + operations, + ) + } + fn inode_init(inode: u64, node_type: NodeType, size: u64) -> VfsInodeInit { VfsInodeInit::new( inode, @@ -1609,6 +1823,61 @@ mod tests_dentry { assert!(root.lookup_cache("right").unwrap().ptr_eq(&right)); } + #[def_test] + fn test_rmdir_uses_directory_removal_callback() { + let root_operations = Arc::new(MockDirOps::new_removable(44)); + let root_inode = VfsInode::new_openable_dir( + root_operations.clone(), + inode_init(44, NodeType::Directory, 0), + ); + let root = Dentry::new_dir_from_inode(root_inode, None, String::new()); + let (victim, _) = make_removable_dir_entry(45, Some(root.clone()), "victim"); + root.insert_cache(String::from("victim"), victim); + + root.rmdir("victim").unwrap(); + + assert_eq!(root_operations.unlink_count.load(Ordering::Relaxed), 0); + assert_eq!(root_operations.rmdir_count.load(Ordering::Relaxed), 1); + assert!(root.lookup_cache("victim").is_none()); + } + + #[def_test] + fn test_rename_rejects_directory_move_into_descendant() { + let root = make_renamable_dir_entry(46, None, ""); + let source = make_renamable_dir_entry(47, Some(root.clone()), "source"); + let child = make_renamable_dir_entry(48, Some(source.clone()), "child"); + root.insert_cache(String::from("source"), source.clone()); + source.insert_cache(String::from("child"), child.clone()); + let _super_block = SuperBlock::new(Arc::new(MockFilesystem), root.clone()); + + assert_eq!( + root.rename("source", &child, "moved", RenameFlags::empty()), + Err(VfsError::InvalidInput) + ); + assert!(root.lookup_cache("source").unwrap().ptr_eq(&source)); + } + + #[def_test] + fn test_cross_directory_rename_moves_source_dentry() { + let fs = Arc::new(MockFilesystem); + let root = make_renamable_dir_entry(49, None, ""); + let old_parent = make_renamable_dir_entry(50, Some(root.clone()), "old"); + let new_parent = make_renamable_dir_entry(51, Some(root.clone()), "new"); + let (source, _) = make_file_entry(fs, 52, Some(old_parent.clone()), "source"); + root.insert_cache(String::from("old"), old_parent.clone()); + root.insert_cache(String::from("new"), new_parent.clone()); + old_parent.insert_cache(String::from("source"), source.clone()); + let _super_block = SuperBlock::new(Arc::new(MockFilesystem), root.clone()); + + old_parent + .rename("source", &new_parent, "target", RenameFlags::empty()) + .unwrap(); + + assert!(old_parent.lookup_cache("source").is_none()); + assert!(new_parent.lookup_cache("target").unwrap().ptr_eq(&source)); + assert_eq!(source.absolute_path().unwrap().as_str(), "/new/target"); + } + #[def_test] fn test_distinct_entries_create_distinct_inode_identities() { let fs = Arc::new(MockFilesystem); diff --git a/fs/kvfs/src/node/inode.rs b/fs/kvfs/src/node/inode.rs index 6e1e71d11..7b9ca9712 100644 --- a/fs/kvfs/src/node/inode.rs +++ b/fs/kvfs/src/node/inode.rs @@ -112,10 +112,15 @@ bitflags! { /// Directory inode operations. /// -/// Namespace-mutating entry points are called by the VFS while holding the -/// relevant parent-directory namespace locks. Dentry arguments that carry the -/// operation name are passed as [`LockedDentry`], allowing filesystem callbacks -/// to read `dentry.name()` as a borrowed `&str` without cloning. +/// Namespace entry points are called by the VFS while holding the locks needed +/// for the operation. Slow lookup holds the parent lock shared. Mutation holds +/// the parent lock exclusive and also locks source or victim inodes when Linux +/// directory-locking rules require it. Dentry arguments that carry operation +/// names are passed as [`LockedDentry`], allowing callbacks to borrow +/// `dentry.name()` without cloning. +/// +/// Implementations may sleep, but must not re-enter namespace operations on the +/// same VFS objects while these locks are held. pub trait InodeDirOperations: Send + Sync { fn lookup( &self, @@ -1514,6 +1519,12 @@ impl VfsInode { self.require_directory_operations()?.unlink(self, &dentry) } + /// Remove a directory child below this directory inode. + pub fn rmdir(&self, dentry: &Dentry) -> VfsResult<()> { + let dentry = dentry.lock_location(); + self.require_directory_operations()?.rmdir(self, &dentry) + } + /// Rename a child from this directory inode to another directory inode. pub fn rename( &self, diff --git a/fs/kvfs/src/node/mod.rs b/fs/kvfs/src/node/mod.rs index 7177da1d7..f24b90a2b 100644 --- a/fs/kvfs/src/node/mod.rs +++ b/fs/kvfs/src/node/mod.rs @@ -10,7 +10,8 @@ mod inode; pub use dentry::{Dentry, DentryOperations, LockedDentry}; pub(crate) use dentry::{ - DentryKey, d_inode, d_is_dir, d_is_negative, d_is_symlink, d_really_is_positive, + DentryKey, LookupCreateResult, d_inode, d_is_dir, d_is_negative, d_is_symlink, + d_really_is_positive, }; pub use device::{DeviceFileOps, MmapMapper, bdev_add, bdev_del, cdev_add, cdev_del}; pub use dir::{DirContext, DirEntrySink}; diff --git a/fs/kvfs/src/super_block.rs b/fs/kvfs/src/super_block.rs index 0e6923317..c74aecf16 100644 --- a/fs/kvfs/src/super_block.rs +++ b/fs/kvfs/src/super_block.rs @@ -191,12 +191,15 @@ pub struct StatFs { /// reaches page cache state through those inodes, matching Linux's /// `super_block` -> `inode` -> `address_space` layering. It also retains hashed /// dentries until namespace eviction, matching Linux dcache lifetime semantics. +/// Cross-directory rename also uses the superblock's topology mutex, +/// corresponding to Linux `s_vfs_rename_mutex`; same-directory rename does not +/// take that mutex. pub struct SuperBlock { ops: Arc, root: Dentry, dentry_cache: Mutex>, max_file_size: u64, - rename_lock: Mutex<()>, + rename_mutex: Mutex<()>, inodes: Mutex>>, } @@ -224,7 +227,7 @@ impl SuperBlock { root, dentry_cache: Mutex::default(), max_file_size, - rename_lock: Mutex::default(), + rename_mutex: Mutex::default(), inodes: Mutex::default(), }); super_block.root.bind_super_block(&super_block); @@ -251,8 +254,9 @@ impl SuperBlock { self.max_file_size } - pub(crate) fn lock_rename(&self) -> MutexGuard<'_, ()> { - self.rename_lock.lock() + /// Serializes directory-tree topology changes across different parents. + pub(crate) fn lock_rename_topology(&self) -> MutexGuard<'_, ()> { + self.rename_mutex.lock() } /// Tracks an inode attached to this superblock. -- Gitee From 5cbd00d2c6354cf002f3d1a639e3e38a466c5c95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B1=E5=AE=81?= Date: Tue, 21 Jul 2026 15:13:07 +0800 Subject: [PATCH 2/4] Optimize VFS namespace locking and permission checks to enhance the security and consistency of directory operations. --- fs/kvfs/docs/design.md | 27 ++- fs/kvfs/docs/security.md | 17 +- fs/kvfs/src/lib.rs | 3 +- fs/kvfs/src/mount.rs | 65 +++---- fs/kvfs/src/namei.rs | 74 ++++++-- fs/kvfs/src/node/dentry.rs | 349 ++++++++++++++++++++++++++----------- 6 files changed, 374 insertions(+), 161 deletions(-) diff --git a/fs/kvfs/docs/design.md b/fs/kvfs/docs/design.md index 1bf874fca..237a7163c 100644 --- a/fs/kvfs/docs/design.md +++ b/fs/kvfs/docs/design.md @@ -111,16 +111,21 @@ VFS 采用 Linux directory-locking ownership model： - create 对父目录 inode 加 exclusive namespace lock； - unlink 和 rmdir 先锁父目录，再锁 victim inode； - link 先锁新父目录，再锁非目录 source inode； -- same-directory rename 只锁一次父目录； +- same-directory rename 按 parent dentry identity 判定并只锁一次父目录； - cross-directory rename 先获取 superblock topology mutex，再按拓扑顺序锁父目录。 rename 在两个父目录稳定后解析 source 和 target，然后先锁参与的子目录，再锁非目录 -inode。子目录按 source 到 target 顺序，非目录 inode 按指针值排序。类型、空目录、 -flag 和祖先关系检查都在所需锁持有期间完成；文件系统 callback 成功后，VFS 再提交 -dentry cache move 或 exchange。 +inode。两个 parent dentry 即使引用同一个目录 inode，也仍属于 cross-directory topology +mutation；对应 inode lock 会去重。participant 最多是 source 和 target 两个 inode，直接按 +目录/非目录组合获取，不构造临时 `Vec`；两个非目录 inode 按指针值排序。类型、flag、 +祖先关系以及针对最终 source/target 的 Path policy 都在同一个 namespace transaction 内 +完成；文件系统 callback 成功后，VFS 再提交 dentry cache move 或 exchange。目录是否为空 +由 filesystem rename/rmdir callback 判定，通用层不把 dentry child cache 当作后端目录内容。 open-create 在同一个父目录 exclusive lock 下完成最终 lookup 和可能的 create，避免 -`O_EXCL` 与 lookup/create 竞争。 +`O_EXCL` 与 lookup/create 竞争。read-only mount 和创建权限属于 create-only 错误：只有锁内 +最终 lookup 仍为 negative 时才检查；若名称已经变为 positive，普通 `O_CREAT` 打开现有 +对象，`O_CREAT | O_EXCL` 返回 `AlreadyExists`。 ### 路径遍历与 DAC @@ -131,8 +136,10 @@ open 再按访问模式检查目标 inode。默认 `InodeOperations::permission` 仍要求至少一个 execute bit，目录 search 可越过 execute bit。 namespace 修改由 `Path` 在调用文件系统回调前统一检查：create、mkdir、mknod、symlink、 -link、unlink、rmdir 和 rename 要求父目录 `MAY_WRITE | MAY_EXEC`。sticky 目录中的删除 -和替换还要求 root、目录所有者或 victim 所有者身份之一。 +link、unlink、rmdir 和 rename 要求父目录 `MAY_WRITE | MAY_EXEC`。unlink、rmdir 和 rename +把 Path policy 作为 validator 传入 Dentry 操作；validator 在父目录锁、所需 participant +锁和最终 lookup 结果仍然有效时执行。sticky 目录中的删除和替换还要求 root、目录所有者 +或该最终 victim 的所有者身份之一，mountpoint 检查也针对同一个 dentry。 inode metadata 修改也由 `Path` 统一授权，再进入同一个后端 `setattr` callback： @@ -244,8 +251,10 @@ child cache 仍使用 mutex。RCU、seqcount lookup 和 lock-free dcache travers - PageCache resize 顺序由 AddressSpace operation 契约表达，不允许文件系统绕过 inode-owned Mapping 建立第二套数据 cache。 - 锁由语义 VFS 对象持有，而不是由 filesystem bridge 持有。 -- `RenameData` 对应 Linux `struct renamedata`，在 VFS orchestration 中携带 rename - participants。 +- `RenameData` 对应 Linux `struct renamedata`，其一次性 `execute` 会消费操作对象并驱动 + VFS orchestration；辅助方法只借用该对象，不为它提供 `Clone`/`Copy`。 +- validator closure 是 Path policy 与锁内 Dentry transaction 之间的窄接口，不新增持久 + 状态，也不在锁外保留 lookup 结果。 - namespace lock 使用 blocking lock，因为文件系统 callback 可以执行 I/O。 ## Drop / 资源释放 diff --git a/fs/kvfs/docs/security.md b/fs/kvfs/docs/security.md index 54995509c..f23a0ddb5 100644 --- a/fs/kvfs/docs/security.md +++ b/fs/kvfs/docs/security.md @@ -50,8 +50,9 @@ namespace 状态前校验 name、mount relationship、类型、topology 和 oper - `Nameidata` 不持有 credential；一次操作的调用者必须在整个方法链中传递同一个 `&Cred`。 - 路径中间目录在 lookup 下一组件前必须通过 `MAY_EXEC`。 - 最终 open 必须按 access mode 检查目标 inode，不能只检查路径是否存在。 -- namespace mutation 必须先检查相关父目录 `MAY_WRITE | MAY_EXEC`；sticky 目录必须额外 - 检查目录所有者、victim 所有者或 root。 +- namespace mutation 必须检查相关父目录 `MAY_WRITE | MAY_EXEC`；unlink、rmdir 和 rename + 的 sticky 与 mountpoint policy 必须针对父目录锁内最终 lookup 得到的 victim 执行，不能 + 复用锁外预查对象。 - 创建 inode 的初始 UID/GID 必须由 `inode_init_owner()` 基于 `fsuid/fsgid` 和父目录 setgid 状态导出，不能使用固定 root owner。 - `mkdir` 必须先清除用户 mode 中的 set-user-ID/set-group-ID 位；只有 setgid 父目录可由 @@ -74,8 +75,9 @@ children map 与 superblock dcache 不嵌套持锁；namespace 操作先更新 p 再更新 dcache 强所有权。类型化 flags 是不可变值快照，不提供共享可变状态。 所有 namespace lock 都是 sleepable lock。全局顺序为 superblock topology、父目录、 -子目录、非目录 inode，最后才是 dentry cache/location。cross-directory 父目录锁按 -ancestor-first 顺序；互不为祖先时先锁 source parent。非目录 inode 按指针值排序。 +子目录、非目录 inode，最后才是 dentry cache/location。cross-directory 由 parent dentry +identity 决定，父目录锁按 ancestor-first 顺序；互不为祖先时先锁 source parent。多个 +dentry alias 引用同一 inode 时对应 inode lock 去重，非目录 inode 按指针值排序。 匿名 inode pseudo fs 的 singleton 由 `Once` 发布，但不允许普通运行时路径触发初始化； 并发创建匿名文件只共享已经发布的 mount/inode，不竞争初始化闭包。 @@ -98,8 +100,8 @@ lower filesystem lock；在推广此类嵌套前还需要明确的跨文件系 | T-05 | 运行时并发首次访问匿名 inode fs 导致初始化卡住 | 中 | 复杂 VFS 对象放在 lazy 首次访问路径中 | boot 阶段调用 `init_anon_inodefs()`，`global()` 只读取已发布对象 | | T-06 | 路径只检查最终 inode，绕过不可搜索目录 | 高 | namei 未对中间目录检查 execute/search | 每次 lookup 下一组件前调用 `Path::permission(MAY_EXEC, cred)` | | T-07 | owner class 缺位后错误退回 group/other | 高 | DAC 把三类权限当作可任选集合 | `generic_permission` 按 owner、group、other 互斥顺序只选择一类 | -| T-08 | namespace 修改绕过父目录权限 | 高 | 后端 callback 被直接调用或 VFS wrapper 漏检 | `Path` mutation API 统一执行父目录 `MAY_WRITE | MAY_EXEC` | -| T-09 | sticky 目录删除其它用户文件 | 高 | unlink/rename 只检查目录 mode | VFS 在回调前执行 sticky owner policy | +| T-08 | namespace 修改绕过父目录权限 | 高 | 后端 callback 被直接调用或 VFS wrapper 漏检 | `Path` mutation API 在最终对象仍受 namespace lock 保护时执行父目录 `MAY_WRITE | MAY_EXEC` | +| T-09 | sticky 目录删除其它用户文件 | 高 | 锁外检查的名称在 callback 前被替换 | unlink/rmdir/rename 的 validator 对锁内最终 victim 执行 sticky owner 和 mountpoint policy | | T-10 | 创建对象固定为 root 或错误组 | 高 | 后端自行填写 UID/GID | 创建 callback 接收 `&Cred` 并使用 `inode_init_owner()` | | T-11 | 一次 namei 混用 credential | 高 | 每个组件反向调用 current helper | syscall 捕获一次 `Arc`，VFS 显式传递引用 | | T-12 | `ftruncate` 因当前 pathname DAC 被错误拒绝或绕过写模式 | 中 | fd 操作复用 pathname truncate | `VfsFile::truncate` 先验证 `FMode::WRITE`，再走 opened truncate | @@ -109,9 +111,10 @@ lower filesystem lock；在推广此类嵌套前还需要明确的跨文件系 | T-16 | truncate 先释放磁盘 block、后失效 PageCache/mmap，造成 stale access | 高 | 文件系统没有保持 split truncate 顺序 | 文件系统 `set_len()` 执行 backing prepare -> `truncate_pagecache()`/view invalidation -> backing finish | | T-17 | unlink 时过早触发磁盘 inode 回收 | 高 | dentry removal 与最后 open-file 引用混为一谈 | `Arc` inode identity 延迟 final teardown，磁盘回收只在 superblock `evict_inode()` hook 中执行 | | T-18 | 并发 rename 创建目录环 | 高 | cross-directory rename 未序列化 topology 或未在锁内检查祖先关系 | topology mutex、稳定 parent lock 和 ancestry check 拒绝该操作 | -| T-19 | create 与 lookup、rmdir 或 replacement 竞争 | 高 | final lookup 和 create 分离持锁 | final lookup 和 mutation 在父目录 exclusive lock 下执行，victim lock 保护删除 | +| T-19 | create 与 lookup、删除或 replacement 竞争 | 高 | final lookup、对象校验和 mutation 分离持锁 | final lookup、validator、participant lock 和 callback 位于同一父目录 exclusive transaction；create-only 错误仅在最终 negative 时返回 | | T-20 | 反向 cross-directory rename 死锁 | 高 | 两个线程按相反顺序锁父目录 | 一个 topology mutex 串行化 topology mutation，父目录按拓扑顺序加锁 | | T-21 | dentry name 和 parent 不一致 | 中 | parent/name 分开更新或读者观察中间状态 | 两个字段在同一个 location write lock 下替换 | +| T-22 | 目录 alias 绕过 topology 序列化或重复锁 inode | 高 | 用 parent inode identity 判定 same-directory | topology 按 parent dentry identity 判定；parent 和 participant inode lock 独立去重 | ## 故障模式与影响分析（FMEA） diff --git a/fs/kvfs/src/lib.rs b/fs/kvfs/src/lib.rs index 7a9d14183..90576f8ab 100644 --- a/fs/kvfs/src/lib.rs +++ b/fs/kvfs/src/lib.rs @@ -51,8 +51,7 @@ pub use node::{ VfsInodeInit, WeakVfsInode, bdev_add, bdev_del, cdev_add, cdev_del, inode_init_owner, }; pub(crate) use node::{ - DentryKey, LookupCreateResult, d_inode, d_is_dir, d_is_negative, d_is_symlink, - d_really_is_positive, + DentryKey, d_inode, d_is_dir, d_is_negative, d_is_symlink, d_really_is_positive, }; pub use open_flags::OpenFlags; pub(crate) use open_flags::{AccMode, OpenHow, OpenParams}; diff --git a/fs/kvfs/src/mount.rs b/fs/kvfs/src/mount.rs index 60e4b7336..bcf4635c6 100644 --- a/fs/kvfs/src/mount.rs +++ b/fs/kvfs/src/mount.rs @@ -749,6 +749,7 @@ impl Path { } } + #[cfg(unittest)] fn lookup_child_in_mount(&self, name: &str) -> VfsResult { Ok(Self::new( self.mnt.clone(), @@ -834,49 +835,53 @@ impl Path { if !Arc::ptr_eq(&self.mnt, &new_dir.mnt) { return Err(VfsError::CrossesDevices); } - self.may_modify_directory(cred)?; - if !self.ptr_eq(new_dir) { - new_dir.may_modify_directory(cred)?; - } self.check_writable_mount()?; new_dir.check_writable_mount()?; - let old_path = self.lookup_child_in_mount(old_name)?; - self.check_sticky(&old_path, cred)?; - old_path.check_not_mountpoint()?; - if let Ok(new_path) = new_dir.lookup_child_in_mount(new_name) { - new_dir.check_sticky(&new_path, cred)?; - new_path.check_not_mountpoint()?; - } - self.dentry - .as_dir()? - .rename(old_name, new_dir.dentry.as_dir()?, new_name, flags) + self.dentry.as_dir()?.rename_with( + old_name, + new_dir.dentry.as_dir()?, + new_name, + flags, + |source, target| { + self.may_modify_directory(cred)?; + if !self.ptr_eq(new_dir) { + new_dir.may_modify_directory(cred)?; + } + + let old_path = self.with_dentry(source.clone()); + self.check_sticky(&old_path, cred)?; + old_path.check_not_mountpoint()?; + if target.is_really_positive() { + let new_path = new_dir.with_dentry(target.clone()); + new_dir.check_sticky(&new_path, cred)?; + new_path.check_not_mountpoint()?; + } + Ok(()) + }, + ) } /// Remove a non-directory entry. pub fn unlink(&self, name: &str, cred: &Cred) -> VfsResult<()> { - self.may_modify_directory(cred)?; self.check_writable_mount()?; - let path = self.lookup_child_in_mount(name)?; - if path.is_dir() { - return Err(VfsError::IsADirectory); - } - self.check_sticky(&path, cred)?; - path.check_not_mountpoint()?; - self.dentry.as_dir()?.unlink(name) + self.dentry.as_dir()?.unlink_with(name, |victim| { + self.may_modify_directory(cred)?; + let victim_path = self.with_dentry(victim.clone()); + self.check_sticky(&victim_path, cred)?; + victim_path.check_not_mountpoint() + }) } /// Remove a directory entry. pub fn rmdir(&self, name: &str, cred: &Cred) -> VfsResult<()> { - self.may_modify_directory(cred)?; self.check_writable_mount()?; - let path = self.lookup_child_in_mount(name)?; - if !path.is_dir() { - return Err(VfsError::NotADirectory); - } - self.check_sticky(&path, cred)?; - path.check_not_mountpoint()?; - self.dentry.as_dir()?.rmdir(name) + self.dentry.as_dir()?.rmdir_with(name, |victim| { + self.may_modify_directory(cred)?; + let victim_path = self.with_dentry(victim.clone()); + self.check_sticky(&victim_path, cred)?; + victim_path.check_not_mountpoint() + }) } /// Mount a filesystem at this path. diff --git a/fs/kvfs/src/namei.rs b/fs/kvfs/src/namei.rs index 27b3542c4..dbd566809 100644 --- a/fs/kvfs/src/namei.rs +++ b/fs/kvfs/src/namei.rs @@ -15,7 +15,8 @@ use kcred::Cred; use crate::{ AccMode, FMode, Filename, LookupFlags, LookupIntent, MountFlags, NodePermission, NodeType, OpenFlags, OpenHow, OpenParams, Path, ResolvedObject, VfsError, VfsFile, VfsFileBuilder, - VfsInode, VfsResult, d_inode, d_is_dir, d_is_negative, d_is_symlink, path::PathBuf, + VfsInode, VfsResult, d_inode, d_is_dir, d_is_negative, d_is_symlink, node::LookupCreateResult, + path::PathBuf, }; /// Deferred cleanup context used while resolving symbolic-link targets. @@ -712,22 +713,24 @@ impl<'a> Nameidata<'a> { if !flags.will_create() { return Err(VfsError::NotFound); } - if !got_write { - return Err(VfsError::ReadOnlyFilesystem); - } - self.path.permission( - crate::Permission::MAY_WRITE | crate::Permission::MAY_EXEC, - cred, - )?; match dir.lookup_or_create_with_mode( name, flags.mode(), flags.is_exclusive_create(), cred, + || { + if !got_write { + return Err(VfsError::ReadOnlyFilesystem); + } + self.path.permission( + crate::Permission::MAY_WRITE | crate::Permission::MAY_EXEC, + cred, + ) + }, )? { - crate::LookupCreateResult::Existing(entry) => Ok(self.path.with_dentry(entry)), - crate::LookupCreateResult::Created(entry) => { + LookupCreateResult::Existing(entry) => Ok(self.path.with_dentry(entry)), + LookupCreateResult::Created(entry) => { file.mark_created(); Ok(self.path.with_dentry(entry)) } @@ -847,6 +850,9 @@ pub fn dentry_open(path: Path, flags: u32, cred: Arc) -> VfsResult>, + miss_once: crate::Mutex>, } impl TestDir { @@ -958,12 +965,17 @@ mod tests { Self { inode, children: crate::Mutex::default(), + miss_once: crate::Mutex::new(None), } } fn insert(&self, name: &str, entry: Dentry) { self.children.lock().insert(String::from(name), entry); } + + fn miss_next_lookup(&self, name: &str) { + *self.miss_once.lock() = Some(String::from(name)); + } } impl InodeOperations for TestDir { @@ -998,6 +1010,12 @@ mod tests { dentry: &LockedDentry<'_>, _flags: crate::InodeLookupFlags, ) -> VfsResult { + let mut miss_once = self.miss_once.lock(); + if miss_once.as_deref() == Some(dentry.name()) { + *miss_once = None; + return Err(VfsError::NotFound); + } + drop(miss_once); self.children .lock() .get(dentry.name()) @@ -1222,9 +1240,11 @@ mod tests { } struct TestTree { + fs: Arc, root: Path, base: Path, target: Path, + root_ops: Arc, magic_file: Arc, magic_dir: Arc, } @@ -1334,9 +1354,11 @@ mod tests { root_ops.insert("magicdir", magic_dir_entry); TestTree { + fs, root: root_location.clone(), base: root_location, target: target_location, + root_ops, magic_file, magic_dir, } @@ -1426,6 +1448,38 @@ mod tests { assert_eq!(name, "missing"); } + #[def_test] + fn open_create_relookup_precedes_create_only_errors() { + let tree = test_tree(); + tree.root_ops.miss_next_lookup("target"); + let mount = Mount::new_root_with_flags(&tree.fs, crate::MountFlags::RDONLY); + let root = mount.root_path(); + + let existing = Filename::new("/target").open_with_flags_at( + &root, + &root, + linux_raw_sys::general::O_CREAT | linux_raw_sys::general::O_RDONLY, + NodePermission::empty(), + kcred::initial_cred(), + ); + assert!(existing.is_ok()); + + let tree = test_tree(); + tree.root_ops.miss_next_lookup("target"); + let mount = Mount::new_root_with_flags(&tree.fs, crate::MountFlags::RDONLY); + let root = mount.root_path(); + let exclusive = Filename::new("/target").open_with_flags_at( + &root, + &root, + linux_raw_sys::general::O_CREAT + | linux_raw_sys::general::O_EXCL + | linux_raw_sys::general::O_RDONLY, + NodePermission::empty(), + kcred::initial_cred(), + ); + assert!(matches!(exclusive, Err(VfsError::AlreadyExists))); + } + #[def_test] fn final_symlink_follow_policy_is_typed() { let tree = test_tree(); diff --git a/fs/kvfs/src/node/dentry.rs b/fs/kvfs/src/node/dentry.rs index 02ea9e2c7..e55f70017 100644 --- a/fs/kvfs/src/node/dentry.rs +++ b/fs/kvfs/src/node/dentry.rs @@ -221,7 +221,6 @@ pub struct LockedDentry<'a> { location: RwLockReadGuard<'a, DentryLocation>, } -#[derive(Clone, Copy)] struct RenameData<'a> { old_parent: &'a Dentry, old_name: &'a str, @@ -581,14 +580,6 @@ impl Dentry { Ok(false) } - fn tree_root(&self) -> Self { - let mut current = self.clone(); - while let Some(parent) = current.parent() { - current = parent; - } - current - } - pub(crate) fn collect_absolute_path(&self, components: &mut Vec) { let mut current = self.clone(); loop { @@ -799,13 +790,17 @@ impl Dentry { Ok(entry) } - pub(crate) fn lookup_or_create_with_mode( + pub(crate) fn lookup_or_create_with_mode( &self, name: &str, mode: Umode, exclusive: bool, cred: &kcred::Cred, - ) -> VfsResult { + may_create_fn: F, + ) -> VfsResult + where + F: FnOnce() -> VfsResult<()>, + { self.as_dir()?; Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); @@ -814,6 +809,7 @@ impl Dentry { Ok(_) if exclusive => Err(VfsError::AlreadyExists), Ok(entry) => Ok(LookupCreateResult::Existing(entry)), Err(err) if err.canonicalize() == VfsError::NotFound => { + may_create_fn()?; let entry = dir_inode.create_with_mode(self, name, mode, exclusive, cred)?; self.insert_cache(name.to_owned(), entry.clone()); Ok(LookupCreateResult::Created(entry)) @@ -885,6 +881,13 @@ impl Dentry { /// Unlinks a non-directory child by name. pub fn unlink(&self, name: &str) -> VfsResult<()> { + self.unlink_with(name, |_| Ok(())) + } + + pub(crate) fn unlink_with(&self, name: &str, may_unlink_fn: F) -> VfsResult<()> + where + F: FnOnce(&Dentry) -> VfsResult<()>, + { self.as_dir()?; Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); @@ -895,13 +898,24 @@ impl Dentry { if entry.is_dir() { return Err(VfsError::IsADirectory); } + may_unlink_fn(&entry)?; dir_inode.unlink(&entry)?; self.forget_cache_entry(name); Ok(()) } - /// Removes an empty directory child by name. + /// Removes a directory child by name. + /// + /// The filesystem `rmdir` callback is authoritative for whether the + /// directory is empty. pub fn rmdir(&self, name: &str) -> VfsResult<()> { + self.rmdir_with(name, |_| Ok(())) + } + + pub(crate) fn rmdir_with(&self, name: &str, may_rmdir_fn: F) -> VfsResult<()> + where + F: FnOnce(&Dentry) -> VfsResult<()>, + { self.as_dir()?; Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); @@ -912,20 +926,12 @@ impl Dentry { if !entry.is_dir() { return Err(VfsError::NotADirectory); } - if entry.has_children()? { - return Err(VfsError::DirectoryNotEmpty); - } + may_rmdir_fn(&entry)?; dir_inode.rmdir(&entry)?; self.forget_cache_entry(name); Ok(()) } - /// Returns whether the directory contains children. - pub fn has_children(&self) -> VfsResult { - self.as_dir()?; - Ok(self.has_positive_children()) - } - pub(crate) fn has_positive_children(&self) -> bool { self.0.children.lock().iter().any(|(name, entry)| { name != DOT @@ -938,6 +944,10 @@ impl Dentry { } /// Renames a child dentry from this directory to `dst_dir`. + /// + /// The filesystem callback is authoritative for target-directory + /// emptiness; the generic namespace layer only validates common topology, + /// type, and flag rules. pub fn rename( &self, src_name: &str, @@ -945,6 +955,20 @@ impl Dentry { dst_name: &str, flags: RenameFlags, ) -> VfsResult<()> { + self.rename_with(src_name, dst_dir, dst_name, flags, |_, _| Ok(())) + } + + pub(crate) fn rename_with( + &self, + src_name: &str, + dst_dir: &Self, + dst_name: &str, + flags: RenameFlags, + may_rename_fn: F, + ) -> VfsResult<()> + where + F: FnOnce(&Dentry, &Dentry) -> VfsResult<()>, + { if flags.has_conflicting_modes() { return Err(VfsError::InvalidInput); } @@ -963,7 +987,7 @@ impl Dentry { new_name: dst_name, flags, } - .execute() + .execute(may_rename_fn) } /// Emits entries currently present in this dentry's child cache. @@ -1038,13 +1062,21 @@ impl Dentry { } impl RenameData<'_> { - fn execute(self) -> VfsResult<()> { + fn execute(self, may_rename_fn: F) -> VfsResult<()> + where + F: FnOnce(&Dentry, &Dentry) -> VfsResult<()>, + { let old_dir_inode = self.old_parent.vfs_inode(); let new_dir_inode = self.new_parent.vfs_inode(); - if Arc::ptr_eq(&old_dir_inode, &new_dir_inode) { + if self.old_parent.ptr_eq(self.new_parent) { let _parent_guard = old_dir_inode.lock_namespace_exclusive(); - return self.execute_with_parents_locked(&old_dir_inode, &new_dir_inode, false); + return self.execute_with_parents_locked( + &old_dir_inode, + &new_dir_inode, + false, + may_rename_fn, + ); } let old_super_block = self @@ -1061,40 +1093,56 @@ impl RenameData<'_> { let _topology_guard = old_super_block.lock_rename_topology(); let old_parent_first = self.old_parent_first()?; + if Arc::ptr_eq(&old_dir_inode, &new_dir_inode) { + let _parent_guard = old_dir_inode.lock_namespace_exclusive(); + return self.execute_with_parents_locked( + &old_dir_inode, + &new_dir_inode, + true, + may_rename_fn, + ); + } if old_parent_first { let _old_parent_guard = old_dir_inode.lock_namespace_exclusive(); let _new_parent_guard = new_dir_inode.lock_namespace_exclusive(); - self.execute_with_parents_locked(&old_dir_inode, &new_dir_inode, true) + self.execute_with_parents_locked(&old_dir_inode, &new_dir_inode, true, may_rename_fn) } else { let _new_parent_guard = new_dir_inode.lock_namespace_exclusive(); let _old_parent_guard = old_dir_inode.lock_namespace_exclusive(); - self.execute_with_parents_locked(&old_dir_inode, &new_dir_inode, true) + self.execute_with_parents_locked(&old_dir_inode, &new_dir_inode, true, may_rename_fn) } } - fn old_parent_first(self) -> VfsResult { - if self.old_parent.is_ancestor_of(self.new_parent)? { - return Ok(true); - } - if self.new_parent.is_ancestor_of(self.old_parent)? { - return Ok(false); + fn old_parent_first(&self) -> VfsResult { + let mut old_ancestor = self.old_parent.clone(); + while let Some(parent) = old_ancestor.parent() { + if parent.ptr_eq(self.new_parent) { + return Ok(false); + } + old_ancestor = parent; } - if !self - .old_parent - .tree_root() - .ptr_eq(&self.new_parent.tree_root()) - { - return Err(VfsError::CrossesDevices); + + let old_root = old_ancestor; + let mut new_ancestor = self.new_parent.clone(); + while let Some(parent) = new_ancestor.parent() { + if parent.ptr_eq(self.old_parent) || parent.ptr_eq(&old_root) { + return Ok(true); + } + new_ancestor = parent; } - Ok(true) + Err(VfsError::CrossesDevices) } - fn execute_with_parents_locked( - self, - old_dir_inode: &VfsInode, - new_dir_inode: &VfsInode, + fn execute_with_parents_locked( + &self, + old_dir_inode: &Arc, + new_dir_inode: &Arc, is_cross_directory: bool, - ) -> VfsResult<()> { + may_rename_fn: F, + ) -> VfsResult<()> + where + F: FnOnce(&Dentry, &Dentry) -> VfsResult<()>, + { let source = self .old_parent .lookup_no_namespace_lock(old_dir_inode, self.old_name)?; @@ -1119,35 +1167,94 @@ impl RenameData<'_> { } self.validate_topology(&source, &target)?; - let participant_inodes = self.participant_inodes(&source, &target, is_cross_directory); - let _participant_guards: Vec<_> = participant_inodes - .iter() - .map(|inode| inode.lock_namespace_exclusive()) - .collect(); + let is_exchange = self.flags.contains(RenameFlags::EXCHANGE); + let target_is_positive = target.is_really_positive(); + let target_is_directory = target_is_positive && target.is_dir(); + let source_inode = source.vfs_inode(); + let target_inode = target_is_positive.then(|| target.vfs_inode()); + let is_parent_inode = |inode: &Arc| { + Arc::ptr_eq(inode, old_dir_inode) || Arc::ptr_eq(inode, new_dir_inode) + }; + let (first_participant, second_participant) = match (source.is_dir(), target_is_directory) { + (true, _) => { + let source_participant = if is_cross_directory && !is_parent_inode(&source_inode) { + Some(source_inode) + } else { + None + }; + let should_lock_target = target_is_positive + && (!target_is_directory || is_cross_directory || !is_exchange); + let target_participant = + target_inode.filter(|inode| should_lock_target && !is_parent_inode(inode)); + (source_participant, target_participant) + } + (false, true) => { + let target_participant = target_inode.filter(|inode| { + (is_cross_directory || !is_exchange) && !is_parent_inode(inode) + }); + (target_participant, Some(source_inode)) + } + (false, false) => match target_inode { + None => (Some(source_inode), None), + Some(target_inode) if Arc::ptr_eq(&source_inode, &target_inode) => { + (Some(source_inode), None) + } + Some(target_inode) if Arc::as_ptr(&source_inode) < Arc::as_ptr(&target_inode) => { + (Some(source_inode), Some(target_inode)) + } + Some(target_inode) => (Some(target_inode), Some(source_inode)), + }, + }; + let _first_guard = first_participant + .as_ref() + .map(|inode| inode.lock_namespace_exclusive()); + let _second_guard = second_participant + .as_ref() + .map(|inode| inode.lock_namespace_exclusive()); + self.execute_locked( + old_dir_inode, + new_dir_inode, + &source, + &target, + may_rename_fn, + ) + } - self.validate_locked(&source, &target)?; - old_dir_inode.rename(&source, new_dir_inode, &target, self.flags)?; + fn execute_locked( + &self, + old_dir_inode: &Arc, + new_dir_inode: &Arc, + source: &Dentry, + target: &Dentry, + may_rename_fn: F, + ) -> VfsResult<()> + where + F: FnOnce(&Dentry, &Dentry) -> VfsResult<()>, + { + self.validate_locked(source, target)?; + may_rename_fn(source, target)?; + old_dir_inode.rename(source, new_dir_inode, target, self.flags)?; if self.flags.contains(RenameFlags::EXCHANGE) { self.old_parent.d_exchange( self.old_name, - &source, + source, self.new_parent, self.new_name, - &target, + target, ); } else { self.old_parent.d_move( self.old_name, - &source, + source, self.new_parent, self.new_name, - &target, + target, ); } Ok(()) } - fn validate_topology(self, source: &Dentry, target: &Dentry) -> VfsResult<()> { + fn validate_topology(&self, source: &Dentry, target: &Dentry) -> VfsResult<()> { if source.is_dir() && source.is_ancestor_of(self.new_parent)? { return Err(VfsError::InvalidInput); } @@ -1164,7 +1271,7 @@ impl RenameData<'_> { Ok(()) } - fn validate_locked(self, source: &Dentry, target: &Dentry) -> VfsResult<()> { + fn validate_locked(&self, source: &Dentry, target: &Dentry) -> VfsResult<()> { if self.flags.contains(RenameFlags::EXCHANGE) { return Ok(()); } @@ -1180,51 +1287,8 @@ impl RenameData<'_> { (false, true) => return Err(VfsError::IsADirectory), _ => {} } - if target.is_dir() && target.has_children()? { - return Err(VfsError::DirectoryNotEmpty); - } Ok(()) } - - fn participant_inodes( - self, - source: &Dentry, - target: &Dentry, - is_cross_directory: bool, - ) -> Vec> { - let is_exchange = self.flags.contains(RenameFlags::EXCHANGE); - let mut directories = Vec::new(); - let mut non_directories = Vec::new(); - - if source.is_dir() { - if is_cross_directory { - directories.push(source.vfs_inode()); - } - } else { - non_directories.push(source.vfs_inode()); - } - - if target.is_really_positive() { - if target.is_dir() { - if is_cross_directory || !is_exchange { - directories.push(target.vfs_inode()); - } - } else { - non_directories.push(target.vfs_inode()); - } - } - - non_directories.sort_by_key(|inode| Arc::as_ptr(inode) as usize); - for inode in non_directories { - if !directories - .iter() - .any(|existing| Arc::ptr_eq(existing, &inode)) - { - directories.push(inode); - } - } - directories - } } #[cfg(unittest)] @@ -1832,7 +1896,10 @@ mod tests_dentry { ); let root = Dentry::new_dir_from_inode(root_inode, None, String::new()); let (victim, _) = make_removable_dir_entry(45, Some(root.clone()), "victim"); - root.insert_cache(String::from("victim"), victim); + let (cached_child, _) = + make_file_entry(Arc::new(MockFilesystem), 46, Some(victim.clone()), "cached"); + victim.insert_cache(String::from("cached"), cached_child.clone()); + root.insert_cache(String::from("victim"), victim.clone()); root.rmdir("victim").unwrap(); @@ -1841,6 +1908,47 @@ mod tests_dentry { assert!(root.lookup_cache("victim").is_none()); } + #[def_test] + fn test_unlink_validator_observes_callback_victim() { + let root_operations = Arc::new(MockDirOps::new_removable(60)); + let root_inode = VfsInode::new_openable_dir( + root_operations.clone(), + inode_init(60, NodeType::Directory, 0), + ); + let root = Dentry::new_dir_from_inode(root_inode, None, String::new()); + let (victim, _) = + make_file_entry(Arc::new(MockFilesystem), 61, Some(root.clone()), "victim"); + root.insert_cache(String::from("victim"), victim.clone()); + + assert_eq!( + root.unlink_with("victim", |checked| { + if !checked.ptr_eq(&victim) { + return Err(VfsError::InvalidInput); + } + Err(VfsError::OperationNotPermitted) + }), + Err(VfsError::OperationNotPermitted) + ); + assert_eq!(root_operations.unlink_count.load(Ordering::Relaxed), 0); + assert!(root.lookup_cache("victim").unwrap().ptr_eq(&victim)); + } + + #[def_test] + fn test_rename_directory_emptiness_is_filesystem_owned() { + let root = make_renamable_dir_entry(62, None, ""); + let source = make_renamable_dir_entry(63, Some(root.clone()), "source"); + let target = make_renamable_dir_entry(64, Some(root.clone()), "target"); + let (cached_child, _) = + make_file_entry(Arc::new(MockFilesystem), 65, Some(target.clone()), "cached"); + root.insert_cache(String::from("source"), source.clone()); + root.insert_cache(String::from("target"), target.clone()); + target.insert_cache(String::from("cached"), cached_child.clone()); + + root.rename("source", &root, "target", RenameFlags::empty()) + .unwrap(); + assert!(root.lookup_cache("target").unwrap().ptr_eq(&source)); + } + #[def_test] fn test_rename_rejects_directory_move_into_descendant() { let root = make_renamable_dir_entry(46, None, ""); @@ -1878,6 +1986,41 @@ mod tests_dentry { assert_eq!(source.absolute_path().unwrap().as_str(), "/new/target"); } + #[def_test] + fn test_rename_distinct_parent_aliases_deduplicates_inode_lock() { + let root = make_renamable_dir_entry(66, None, ""); + let parent_inode = VfsInode::new_openable_dir( + Arc::new(MockDirOps::new_renamable(67)), + inode_init(67, NodeType::Directory, 0), + ); + let old_parent = Dentry::new_dir_from_inode( + parent_inode.clone(), + Some(root.clone()), + String::from("old"), + ); + let new_parent = + Dentry::new_dir_from_inode(parent_inode, Some(root.clone()), String::from("new")); + let (source, _) = make_file_entry( + Arc::new(MockFilesystem), + 68, + Some(old_parent.clone()), + "source", + ); + root.insert_cache(String::from("old"), old_parent.clone()); + root.insert_cache(String::from("new"), new_parent.clone()); + old_parent.insert_cache(String::from("source"), source.clone()); + let _super_block = SuperBlock::new(Arc::new(MockFilesystem), root); + + assert!(!old_parent.ptr_eq(&new_parent)); + assert!(old_parent.is_same_inode(&new_parent)); + old_parent + .rename("source", &new_parent, "target", RenameFlags::empty()) + .unwrap(); + + assert!(old_parent.lookup_cache("source").is_none()); + assert!(new_parent.lookup_cache("target").unwrap().ptr_eq(&source)); + } + #[def_test] fn test_distinct_entries_create_distinct_inode_identities() { let fs = Arc::new(MockFilesystem); @@ -1967,7 +2110,7 @@ mod tests_dentry { drop(child); - assert!(directory.has_children().unwrap()); + assert!(directory.has_positive_children()); } #[def_test] -- Gitee From 37cbb7114829ecdf4e35bc73281ca9160dd7a003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B1=E5=AE=81?= Date: Tue, 21 Jul 2026 18:01:51 +0800 Subject: [PATCH 3/4] Optimize VFS namespace locking and lookup logic, and simplify the directory entry lookup and creation process. --- fs/kvfs/docs/design.md | 18 ++-- fs/kvfs/docs/security.md | 2 +- fs/kvfs/src/mount.rs | 75 ++++++++-------- fs/kvfs/src/namei.rs | 12 ++- fs/kvfs/src/node/dentry.rs | 172 ++++++++++++++++++++++++++----------- fs/kvfs/src/node/inode.rs | 24 +++--- 6 files changed, 189 insertions(+), 114 deletions(-) diff --git a/fs/kvfs/docs/design.md b/fs/kvfs/docs/design.md index 237a7163c..bf1445a01 100644 --- a/fs/kvfs/docs/design.md +++ b/fs/kvfs/docs/design.md @@ -117,15 +117,19 @@ VFS 采用 Linux directory-locking ownership model： rename 在两个父目录稳定后解析 source 和 target，然后先锁参与的子目录，再锁非目录 inode。两个 parent dentry 即使引用同一个目录 inode，也仍属于 cross-directory topology mutation；对应 inode lock 会去重。participant 最多是 source 和 target 两个 inode，直接按 -目录/非目录组合获取，不构造临时 `Vec`；两个非目录 inode 按指针值排序。类型、flag、 -祖先关系以及针对最终 source/target 的 Path policy 都在同一个 namespace transaction 内 -完成；文件系统 callback 成功后，VFS 再提交 dentry cache move 或 exchange。目录是否为空 -由 filesystem rename/rmdir callback 判定，通用层不把 dentry child cache 当作后端目录内容。 +目录/非目录组合获取，不构造临时 `Vec`；两个非目录 inode 按指针值排序。父目录拓扑遍历 +同时给出锁顺序和 Linux `lock_two_directories()` 语义中的 `trap`，最终 source/target 直接与 +`trap` 比较，不再次遍历父链。类型、flag、祖先关系以及针对最终 source/target 的 Path +policy 都在同一个 namespace transaction 内完成；文件系统 callback 成功后，VFS 再提交 +dentry cache move 或 exchange。目录是否为空由 filesystem rename/rmdir callback 判定， +通用层不把 dentry child cache 当作后端目录内容。 open-create 在同一个父目录 exclusive lock 下完成最终 lookup 和可能的 create，避免 -`O_EXCL` 与 lookup/create 竞争。read-only mount 和创建权限属于 create-only 错误：只有锁内 -最终 lookup 仍为 negative 时才检查；若名称已经变为 positive，普通 `O_CREAT` 打开现有 -对象，`O_CREAT | O_EXCL` 返回 `AlreadyExists`。 +`O_EXCL` 与 lookup/create 竞争。`O_CREAT | O_EXCL` 跳过 speculative lookup，直接执行锁内 +最终 lookup；lookup 得到的同一个 negative dentry 会传给 filesystem create callback，不再 +按名称构造第二个对象。read-only mount 和创建权限属于 create-only 错误：只有锁内最终 +lookup 仍为 negative 时才检查；若名称已经变为 positive，普通 `O_CREAT` 打开现有对象， +`O_CREAT | O_EXCL` 返回 `AlreadyExists`。 ### 路径遍历与 DAC diff --git a/fs/kvfs/docs/security.md b/fs/kvfs/docs/security.md index f23a0ddb5..f81c378d6 100644 --- a/fs/kvfs/docs/security.md +++ b/fs/kvfs/docs/security.md @@ -111,7 +111,7 @@ lower filesystem lock；在推广此类嵌套前还需要明确的跨文件系 | T-16 | truncate 先释放磁盘 block、后失效 PageCache/mmap，造成 stale access | 高 | 文件系统没有保持 split truncate 顺序 | 文件系统 `set_len()` 执行 backing prepare -> `truncate_pagecache()`/view invalidation -> backing finish | | T-17 | unlink 时过早触发磁盘 inode 回收 | 高 | dentry removal 与最后 open-file 引用混为一谈 | `Arc` inode identity 延迟 final teardown，磁盘回收只在 superblock `evict_inode()` hook 中执行 | | T-18 | 并发 rename 创建目录环 | 高 | cross-directory rename 未序列化 topology 或未在锁内检查祖先关系 | topology mutex、稳定 parent lock 和 ancestry check 拒绝该操作 | -| T-19 | create 与 lookup、删除或 replacement 竞争 | 高 | final lookup、对象校验和 mutation 分离持锁 | final lookup、validator、participant lock 和 callback 位于同一父目录 exclusive transaction；create-only 错误仅在最终 negative 时返回 | +| T-19 | create 与 lookup、删除或 replacement 竞争 | 高 | final lookup、对象校验和 mutation 分离持锁 | final lookup、validator、participant lock 和 callback 位于同一父目录 exclusive transaction；create callback 复用最终 negative dentry，create-only 错误仅在该对象仍为 negative 时返回 | | T-20 | 反向 cross-directory rename 死锁 | 高 | 两个线程按相反顺序锁父目录 | 一个 topology mutex 串行化 topology mutation，父目录按拓扑顺序加锁 | | T-21 | dentry name 和 parent 不一致 | 中 | parent/name 分开更新或读者观察中间状态 | 两个字段在同一个 location write lock 下替换 | | T-22 | 目录 alias 绕过 topology 序列化或重复锁 inode | 高 | 用 parent inode identity 判定 same-directory | topology 按 parent dentry identity 判定；parent 和 participant inode lock 独立去重 | diff --git a/fs/kvfs/src/mount.rs b/fs/kvfs/src/mount.rs index bcf4635c6..2c15a85b9 100644 --- a/fs/kvfs/src/mount.rs +++ b/fs/kvfs/src/mount.rs @@ -749,14 +749,6 @@ impl Path { } } - #[cfg(unittest)] - fn lookup_child_in_mount(&self, name: &str) -> VfsResult { - Ok(Self::new( - self.mnt.clone(), - self.dentry.as_dir()?.lookup(name)?, - )) - } - /// Create a regular file under this directory path. pub fn create(&self, name: &str, permission: NodePermission, cred: &Cred) -> VfsResult { self.may_modify_directory(cred)?; @@ -836,7 +828,6 @@ impl Path { return Err(VfsError::CrossesDevices); } self.check_writable_mount()?; - new_dir.check_writable_mount()?; self.dentry.as_dir()?.rename_with( old_name, @@ -885,11 +876,6 @@ impl Path { } /// Mount a filesystem at this path. - #[cfg(unittest)] - fn mount_filesystem(&self, fs: &Arc) -> VfsResult> { - self.mount_filesystem_with_flags_and_devname(fs, MountFlags::empty(), None) - } - fn mount_filesystem_with_flags_and_devname( &self, fs: &Arc, @@ -1004,18 +990,6 @@ impl Path { } } -/// Look up a child path without following symlinks. -#[cfg(unittest)] -fn lookup_no_follow(path: &Path, name: &str) -> VfsResult { - use crate::path::{DOT, DOTDOT}; - - Ok(match name { - DOT => path.clone(), - DOTDOT => path.parent().unwrap_or_else(|| path.clone()), - _ => path.lookup_child_in_mount(name)?.resolve_final_mount(), - }) -} - fn collect_mount_tree(root: &Arc) -> Vec> { let mut mounts = vec![root.clone()]; let mut index = 0; @@ -1077,6 +1051,27 @@ mod tests { SuperBlockOperations, VfsError, VfsFile, VfsInode, VfsInodeInit, VfsResult, }; + fn lookup_child_in_mount(path: &Path, name: &str) -> VfsResult { + path.dentry + .as_dir()? + .lookup(name) + .map(|dentry| path.with_dentry(dentry)) + } + + fn lookup_no_follow(path: &Path, name: &str) -> VfsResult { + use crate::path::{DOT, DOTDOT}; + + Ok(match name { + DOT => path.clone(), + DOTDOT => path.parent().unwrap_or_else(|| path.clone()), + _ => lookup_child_in_mount(path, name)?.resolve_final_mount(), + }) + } + + fn mount_filesystem(path: &Path, fs: &Arc) -> VfsResult> { + path.mount_filesystem_with_flags_and_devname(fs, MountFlags::empty(), None) + } + struct MockFilesystem { mount_flags: StatFsFlags, } @@ -1390,7 +1385,7 @@ mod tests { let root_mount = Mount::new_root(&root_fs); let mnt_loc = lookup_no_follow(&root_mount.root_path(), "mnt").unwrap(); - let mount_a = mnt_loc.mount_filesystem(&fs_a).unwrap(); + let mount_a = mount_filesystem(&mnt_loc, &fs_a).unwrap(); let loc_a = lookup_no_follow(&root_mount.root_path(), "mnt").unwrap(); assert!(Arc::ptr_eq(loc_a.mount(), &mount_a)); @@ -1412,7 +1407,7 @@ mod tests { let child_fs = mock_filesystem(StatFsFlags::empty()); let root_mount = Mount::new_root(&root_fs); let mount_dir = lookup_no_follow(&root_mount.root_path(), "mnt").unwrap(); - let child_mount = mount_dir.mount_filesystem(&child_fs).unwrap(); + let child_mount = mount_filesystem(&mount_dir, &child_fs).unwrap(); let child_root = child_mount.root_path(); let child_path = child_root.absolute_path().unwrap(); @@ -1438,9 +1433,9 @@ mod tests { let root_mount = Mount::new_root(&root_fs); let first_mount_dir = lookup_no_follow(&root_mount.root_path(), "mnt").unwrap(); - let child_mount = first_mount_dir.mount_filesystem(&child_fs).unwrap(); + let child_mount = mount_filesystem(&first_mount_dir, &child_fs).unwrap(); let second_mount_dir = lookup_no_follow(&child_mount.root_path(), "mnt").unwrap(); - let grandchild_mount = second_mount_dir.mount_filesystem(&grandchild_fs).unwrap(); + let grandchild_mount = mount_filesystem(&second_mount_dir, &grandchild_fs).unwrap(); let grandchild_root = grandchild_mount.root_path(); let grandchild_path = grandchild_root.absolute_path().unwrap(); @@ -1464,7 +1459,7 @@ mod tests { assert_eq!(root_mount.children().len(), 0); let mount_dir = lookup_no_follow(&root_mount.root_path(), "mnt").unwrap(); - let child_mount = mount_dir.mount_filesystem(&child_fs).unwrap(); + let child_mount = mount_filesystem(&mount_dir, &child_fs).unwrap(); let children = root_mount.children(); assert_eq!(children.len(), 1); @@ -1482,7 +1477,7 @@ mod tests { let namespace = MntNamespace::new_root(&root_fs, kcred::initial_user_namespace()); let root = namespace.root_path(); - let mountpoint = root.lookup_child_in_mount("mnt").unwrap(); + let mountpoint = lookup_child_in_mount(&root, "mnt").unwrap(); let child_mount = namespace.attach(&mountpoint, &child_fs).unwrap(); let pwd = lookup_no_follow(&root, "mnt").unwrap(); @@ -1525,9 +1520,9 @@ mod tests { let root_mount = Mount::new_root(&root_fs); let first_mount_dir = lookup_no_follow(&root_mount.root_path(), "mnt").unwrap(); - let child_mount = first_mount_dir.mount_filesystem(&child_fs).unwrap(); + let child_mount = mount_filesystem(&first_mount_dir, &child_fs).unwrap(); let second_mount_dir = lookup_no_follow(&child_mount.root_path(), "mnt").unwrap(); - let grandchild_mount = second_mount_dir.mount_filesystem(&grandchild_fs).unwrap(); + let grandchild_mount = mount_filesystem(&second_mount_dir, &grandchild_fs).unwrap(); assert_eq!( child_mount.root_path().unmount(), @@ -1549,9 +1544,9 @@ mod tests { let root_mount = Mount::new_root(&root_fs); let first_mount_dir = lookup_no_follow(&root_mount.root_path(), "mnt").unwrap(); - let child_mount = first_mount_dir.mount_filesystem(&child_fs).unwrap(); + let child_mount = mount_filesystem(&first_mount_dir, &child_fs).unwrap(); let second_mount_dir = lookup_no_follow(&child_mount.root_path(), "mnt").unwrap(); - let grandchild_mount = second_mount_dir.mount_filesystem(&grandchild_fs).unwrap(); + let grandchild_mount = mount_filesystem(&second_mount_dir, &grandchild_fs).unwrap(); child_mount.root_path().unmount_tree().unwrap(); @@ -1574,8 +1569,8 @@ mod tests { let root_mount = Mount::new_root(&root_fs); let mount_dir = lookup_no_follow(&root_mount.root_path(), "mnt").unwrap(); - let mount_a = mount_dir.mount_filesystem(&fs_a).unwrap(); - let mount_b = mount_dir.mount_filesystem(&fs_b).unwrap(); + let mount_a = mount_filesystem(&mount_dir, &fs_a).unwrap(); + let mount_b = mount_filesystem(&mount_dir, &fs_b).unwrap(); assert_eq!(mount_a.root_path().unmount(), Err(VfsError::InvalidInput)); @@ -1591,7 +1586,7 @@ mod tests { let root_mount = Mount::new_root(&root_fs); let mount_dir = lookup_no_follow(&root_mount.root_path(), "mnt").unwrap(); - let child_mount = mount_dir.mount_filesystem(&child_fs).unwrap(); + let child_mount = mount_filesystem(&mount_dir, &child_fs).unwrap(); let child_root = child_mount.root_path(); let root_metadata = root_mount.root_path().getattr().unwrap(); @@ -1709,7 +1704,7 @@ mod tests { ..Default::default() }) .unwrap(); - let victim = root.lookup_child_in_mount("mnt").unwrap(); + let victim = lookup_child_in_mount(&root, "mnt").unwrap(); victim .dentry .update_metadata(MetadataUpdate { diff --git a/fs/kvfs/src/namei.rs b/fs/kvfs/src/namei.rs index dbd566809..e3b32cc12 100644 --- a/fs/kvfs/src/namei.rs +++ b/fs/kvfs/src/namei.rs @@ -701,13 +701,12 @@ impl<'a> Nameidata<'a> { .map(Qstr::as_str) .ok_or(VfsError::InvalidInput)?; let dir = self.path.dentry().as_dir()?; - match dir.lookup(name) { - Ok(_) if flags.is_exclusive_create() => { - return Err(VfsError::AlreadyExists); + if !flags.is_exclusive_create() { + match dir.lookup(name) { + Ok(entry) => return Ok(self.path.with_dentry(entry)), + Err(err) if err.canonicalize() == VfsError::NotFound => {} + Err(err) => return Err(err), } - Ok(entry) => return Ok(self.path.with_dentry(entry)), - Err(err) if err.canonicalize() == VfsError::NotFound => {} - Err(err) => return Err(err), } if !flags.will_create() { @@ -1465,7 +1464,6 @@ mod tests { assert!(existing.is_ok()); let tree = test_tree(); - tree.root_ops.miss_next_lookup("target"); let mount = Mount::new_root_with_flags(&tree.fs, crate::MountFlags::RDONLY); let root = mount.root_path(); let exclusive = Filename::new("/target").open_with_flags_at( diff --git a/fs/kvfs/src/node/dentry.rs b/fs/kvfs/src/node/dentry.rs index e55f70017..d3ccead7a 100644 --- a/fs/kvfs/src/node/dentry.rs +++ b/fs/kvfs/src/node/dentry.rs @@ -754,19 +754,27 @@ impl Dentry { self.lookup_no_namespace_lock(&dir_inode, name) } - fn lookup_no_namespace_lock(&self, dir_inode: &VfsInode, name: &str) -> VfsResult { + fn lookup_dentry_no_namespace_lock( + &self, + dir_inode: &VfsInode, + name: &str, + ) -> VfsResult { if let Some(entry) = self.lookup_cache(name) { - return if entry.is_negative() { - Err(VfsError::NotFound) - } else { - Ok(entry) - }; + return Ok(entry); } - let entry = dir_inode.lookup(self, name)?; + let candidate = Dentry::new_negative(Some(self.clone()), name.to_owned()); + let Some(entry) = dir_inode.lookup_child(&candidate)? else { + return Ok(candidate); + }; if self.can_cache_children() && entry.can_cache_as_child() { self.insert_cache(name.to_owned(), entry.clone()); } + Ok(entry) + } + + fn lookup_no_namespace_lock(&self, dir_inode: &VfsInode, name: &str) -> VfsResult { + let entry = self.lookup_dentry_no_namespace_lock(dir_inode, name)?; if entry.is_negative() { Err(VfsError::NotFound) } else { @@ -805,17 +813,19 @@ impl Dentry { Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); - match self.lookup_no_namespace_lock(&dir_inode, name) { - Ok(_) if exclusive => Err(VfsError::AlreadyExists), - Ok(entry) => Ok(LookupCreateResult::Existing(entry)), - Err(err) if err.canonicalize() == VfsError::NotFound => { - may_create_fn()?; - let entry = dir_inode.create_with_mode(self, name, mode, exclusive, cred)?; - self.insert_cache(name.to_owned(), entry.clone()); - Ok(LookupCreateResult::Created(entry)) - } - Err(err) => Err(err), + let candidate = self.lookup_dentry_no_namespace_lock(&dir_inode, name)?; + if candidate.is_really_positive() { + return if exclusive { + Err(VfsError::AlreadyExists) + } else { + Ok(LookupCreateResult::Existing(candidate)) + }; } + + may_create_fn()?; + let entry = dir_inode.create_with_mode(&candidate, mode, exclusive, cred)?; + self.insert_cache(name.to_owned(), entry.clone()); + Ok(LookupCreateResult::Created(entry)) } /// Creates a directory child dentry below this directory. @@ -1075,6 +1085,7 @@ impl RenameData<'_> { &old_dir_inode, &new_dir_inode, false, + None, may_rename_fn, ); } @@ -1092,32 +1103,45 @@ impl RenameData<'_> { } let _topology_guard = old_super_block.lock_rename_topology(); - let old_parent_first = self.old_parent_first()?; + let (old_parent_first, trap) = self.parent_lock_order()?; if Arc::ptr_eq(&old_dir_inode, &new_dir_inode) { let _parent_guard = old_dir_inode.lock_namespace_exclusive(); return self.execute_with_parents_locked( &old_dir_inode, &new_dir_inode, true, + trap.as_ref(), may_rename_fn, ); } if old_parent_first { let _old_parent_guard = old_dir_inode.lock_namespace_exclusive(); let _new_parent_guard = new_dir_inode.lock_namespace_exclusive(); - self.execute_with_parents_locked(&old_dir_inode, &new_dir_inode, true, may_rename_fn) + self.execute_with_parents_locked( + &old_dir_inode, + &new_dir_inode, + true, + trap.as_ref(), + may_rename_fn, + ) } else { let _new_parent_guard = new_dir_inode.lock_namespace_exclusive(); let _old_parent_guard = old_dir_inode.lock_namespace_exclusive(); - self.execute_with_parents_locked(&old_dir_inode, &new_dir_inode, true, may_rename_fn) + self.execute_with_parents_locked( + &old_dir_inode, + &new_dir_inode, + true, + trap.as_ref(), + may_rename_fn, + ) } } - fn old_parent_first(&self) -> VfsResult { + fn parent_lock_order(&self) -> VfsResult<(bool, Option)> { let mut old_ancestor = self.old_parent.clone(); while let Some(parent) = old_ancestor.parent() { if parent.ptr_eq(self.new_parent) { - return Ok(false); + return Ok((false, Some(old_ancestor))); } old_ancestor = parent; } @@ -1125,8 +1149,11 @@ impl RenameData<'_> { let old_root = old_ancestor; let mut new_ancestor = self.new_parent.clone(); while let Some(parent) = new_ancestor.parent() { - if parent.ptr_eq(self.old_parent) || parent.ptr_eq(&old_root) { - return Ok(true); + if parent.ptr_eq(self.old_parent) { + return Ok((true, Some(new_ancestor))); + } + if parent.ptr_eq(&old_root) { + return Ok((true, None)); } new_ancestor = parent; } @@ -1138,6 +1165,7 @@ impl RenameData<'_> { old_dir_inode: &Arc, new_dir_inode: &Arc, is_cross_directory: bool, + trap: Option<&Dentry>, may_rename_fn: F, ) -> VfsResult<()> where @@ -1146,27 +1174,21 @@ impl RenameData<'_> { let source = self .old_parent .lookup_no_namespace_lock(old_dir_inode, self.old_name)?; - let target = match self + let target = self .new_parent - .lookup_no_namespace_lock(new_dir_inode, self.new_name) - { - Ok(target) => target, - Err(err) - if err.canonicalize() == VfsError::NotFound - && self.flags.contains(RenameFlags::EXCHANGE) => - { - return Err(VfsError::NotFound); - } - Err(err) if err.canonicalize() == VfsError::NotFound => { - Dentry::new_negative(Some(self.new_parent.clone()), self.new_name.to_owned()) - } - Err(err) => return Err(err), - }; + .lookup_dentry_no_namespace_lock(new_dir_inode, self.new_name)?; + if target.is_negative() && self.flags.contains(RenameFlags::EXCHANGE) { + return Err(VfsError::NotFound); + } + if target.is_really_positive() && self.flags.contains(RenameFlags::NOREPLACE) { + return Err(VfsError::AlreadyExists); + } + + self.validate_topology(&source, &target, trap)?; if target.is_really_positive() && source.is_same_inode(&target) { return Ok(()); } - self.validate_topology(&source, &target)?; let is_exchange = self.flags.contains(RenameFlags::EXCHANGE); let target_is_positive = target.is_really_positive(); let target_is_directory = target_is_positive && target.is_dir(); @@ -1254,14 +1276,19 @@ impl RenameData<'_> { Ok(()) } - fn validate_topology(&self, source: &Dentry, target: &Dentry) -> VfsResult<()> { - if source.is_dir() && source.is_ancestor_of(self.new_parent)? { + fn validate_topology( + &self, + source: &Dentry, + target: &Dentry, + trap: Option<&Dentry>, + ) -> VfsResult<()> { + let Some(trap) = trap else { + return Ok(()); + }; + if source.ptr_eq(trap) { return Err(VfsError::InvalidInput); } - if target.is_really_positive() - && target.is_dir() - && target.is_ancestor_of(self.old_parent)? - { + if target.ptr_eq(trap) { return if self.flags.contains(RenameFlags::EXCHANGE) { Err(VfsError::InvalidInput) } else { @@ -1275,9 +1302,6 @@ impl RenameData<'_> { if self.flags.contains(RenameFlags::EXCHANGE) { return Ok(()); } - if self.flags.contains(RenameFlags::NOREPLACE) && target.is_really_positive() { - return Err(VfsError::AlreadyExists); - } if !target.is_really_positive() { return Ok(()); } @@ -1846,6 +1870,28 @@ mod tests_dentry { assert!(root.lookup_cache("final").unwrap().ptr_eq(&target)); } + #[def_test] + fn test_rename_noreplace_rejects_target_with_source_inode() { + let fs = Arc::new(MockFilesystem); + let root = make_renamable_dir_entry(69, None, ""); + let (source, _) = make_file_entry(fs, 70, Some(root.clone()), "tmp"); + let target = Dentry::new_file_from_inode( + source.vfs_inode(), + Some(root.clone()), + String::from("final"), + ); + + root.insert_cache(String::from("tmp"), source.clone()); + root.insert_cache(String::from("final"), target.clone()); + + assert_eq!( + root.rename("tmp", &root, "final", RenameFlags::NOREPLACE), + Err(VfsError::AlreadyExists) + ); + assert!(root.lookup_cache("tmp").unwrap().ptr_eq(&source)); + assert!(root.lookup_cache("final").unwrap().ptr_eq(&target)); + } + #[def_test] fn test_rename_exchange_swaps_cache_entries_without_changing_inode_identity() { let fs = Arc::new(MockFilesystem); @@ -1965,6 +2011,34 @@ mod tests_dentry { assert!(root.lookup_cache("source").unwrap().ptr_eq(&source)); } + #[def_test] + fn test_rename_target_ancestor_uses_linux_trap_errors() { + let root = make_renamable_dir_entry(71, None, ""); + let ancestor = make_renamable_dir_entry(72, Some(root.clone()), "ancestor"); + let old_parent = make_renamable_dir_entry(73, Some(ancestor.clone()), "old"); + let (source, _) = make_file_entry( + Arc::new(MockFilesystem), + 74, + Some(old_parent.clone()), + "source", + ); + root.insert_cache(String::from("ancestor"), ancestor.clone()); + ancestor.insert_cache(String::from("old"), old_parent.clone()); + old_parent.insert_cache(String::from("source"), source.clone()); + let _super_block = SuperBlock::new(Arc::new(MockFilesystem), root.clone()); + + assert_eq!( + old_parent.rename("source", &root, "ancestor", RenameFlags::empty()), + Err(VfsError::DirectoryNotEmpty) + ); + assert_eq!( + old_parent.rename("source", &root, "ancestor", RenameFlags::EXCHANGE), + Err(VfsError::InvalidInput) + ); + assert!(old_parent.lookup_cache("source").unwrap().ptr_eq(&source)); + assert!(root.lookup_cache("ancestor").unwrap().ptr_eq(&ancestor)); + } + #[def_test] fn test_cross_directory_rename_moves_source_dentry() { let fs = Arc::new(MockFilesystem); diff --git a/fs/kvfs/src/node/inode.rs b/fs/kvfs/src/node/inode.rs index 7b9ca9712..118481bcc 100644 --- a/fs/kvfs/src/node/inode.rs +++ b/fs/kvfs/src/node/inode.rs @@ -1421,13 +1421,11 @@ impl VfsInode { pub(crate) fn create_with_mode( &self, - dir: &Dentry, - name: &str, + dentry: &Dentry, mode: Umode, exclusive: bool, cred: &Cred, ) -> VfsResult { - let dentry = Dentry::new_negative(Some(dir.clone()), name.to_owned()); let dentry = dentry.lock_location(); self.require_directory_operations()?.create( &MountIdmap, @@ -1439,12 +1437,17 @@ impl VfsInode { ) } - /// Look up a child below this directory inode. - pub fn lookup(&self, dir: &Dentry, name: &str) -> VfsResult { - let dentry = Dentry::new_negative(Some(dir.clone()), name.to_owned()); - let dentry = dentry.lock_location(); - self.require_directory_operations()? - .lookup(self, &dentry, InodeLookupFlags::empty()) + pub(crate) fn lookup_child(&self, dentry: &Dentry) -> VfsResult> { + let result = { + let dentry = dentry.lock_location(); + self.require_directory_operations()? + .lookup(self, &dentry, InodeLookupFlags::empty()) + }; + match result { + Ok(entry) => Ok(Some(entry)), + Err(err) if err.canonicalize() == VfsError::NotFound => Ok(None), + Err(err) => Err(err), + } } /// Create a regular-file child below this directory inode. @@ -1456,7 +1459,8 @@ impl VfsInode { cred: &Cred, ) -> VfsResult { let mode = Umode::new(NodeType::RegularFile, permission); - self.create_with_mode(dir, name, mode, false, cred) + let dentry = Dentry::new_negative(Some(dir.clone()), name.to_owned()); + self.create_with_mode(&dentry, mode, false, cred) } /// Create a directory child below this directory inode. -- Gitee From 9f2d3be07fc97ae441076a98179061f77e343e7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B1=E5=AE=81?= Date: Wed, 22 Jul 2026 12:46:05 +0800 Subject: [PATCH 4/4] Refactor VFS directory entry and inode operations for improved caching and lookup handling - Removed unnecessary state tracking in TestDir and simplified lookup logic. - Updated Dentry and LockedDentry to support parallel lookups and caching mechanisms. - Enhanced inode operations to return Option for lookups, allowing for more flexible handling of misses. - Modified create, mkdir, mknod, symlink, and link methods to instantiate entries directly without returning Dentry. - Introduced new methods in SuperBlock for managing cached dentry movements and exchanges. - Updated tests to reflect changes in lookup behavior and ensure correct instantiation of cached entries. --- fs/bridges/kext4_vfs/src/fs.rs | 14 - fs/bridges/kext4_vfs/src/inode.rs | 77 ++--- fs/bridges/rsext4_vfs/src/inode.rs | 78 ++--- fs/bridges/v9fs/src/inode.rs | 109 ++---- fs/filesystems/bpffs/src/lib.rs | 66 ++-- fs/filesystems/fat/src/dir.rs | 76 ++-- fs/filesystems/memfs/src/lib.rs | 62 ++-- fs/kvfs/docs/design.md | 37 +- fs/kvfs/docs/security.md | 26 +- fs/kvfs/src/mount.rs | 34 +- fs/kvfs/src/namei.rs | 37 +- fs/kvfs/src/node/dentry.rs | 536 +++++++++++++++++++++++------ fs/kvfs/src/node/inode.rs | 98 +++--- fs/kvfs/src/nullfs.rs | 4 +- fs/kvfs/src/simple_dir.rs | 23 +- fs/kvfs/src/super_block.rs | 35 ++ 16 files changed, 776 insertions(+), 536 deletions(-) diff --git a/fs/bridges/kext4_vfs/src/fs.rs b/fs/bridges/kext4_vfs/src/fs.rs index a5ae0d453..8753c3ac3 100644 --- a/fs/bridges/kext4_vfs/src/fs.rs +++ b/fs/bridges/kext4_vfs/src/fs.rs @@ -192,20 +192,6 @@ impl Ext4Filesystem { Ok(vfs_inode) } - pub(crate) fn make_dentry( - fs: &Arc, - parent: Option, - name: String, - inode: Ext4Inode, - ) -> VfsResult { - let inode = Self::iget_from_core_inode(fs, inode)?; - if inode.is_dir() { - Ok(Dentry::new_dir_from_inode(inode, parent, name)) - } else { - Ok(Dentry::new_file_from_inode(inode, parent, name)) - } - } - pub(crate) fn sync_to_disk(&self) -> VfsResult<()> { self.lock().sync_filesystem().map_err(into_vfs_err) } diff --git a/fs/bridges/kext4_vfs/src/inode.rs b/fs/bridges/kext4_vfs/src/inode.rs index da71a2cb6..a3d456131 100644 --- a/fs/bridges/kext4_vfs/src/inode.rs +++ b/fs/bridges/kext4_vfs/src/inode.rs @@ -4,7 +4,7 @@ //! KExt4 inode operations. -use alloc::{borrow::ToOwned, collections::BTreeSet, string::String, sync::Arc, vec, vec::Vec}; +use alloc::{collections::BTreeSet, string::String, sync::Arc, vec, vec::Vec}; use iov_iter::{IovIterDest, IovIterSource}; use kext4::{ @@ -59,7 +59,7 @@ impl Inode { self.number } - fn lookup_child(&self, parent: &Dentry, name: &str) -> VfsResult { + fn lookup_child(&self, name: &str) -> VfsResult> { let entry = { let fs = self.fs.lock(); let directory = fs.inode(self.number).map_err(into_vfs_err)?; @@ -67,7 +67,7 @@ impl Inode { } .ok_or(VfsError::NotFound)?; let inode = self.fs.load_inode(entry.inode())?; - Ext4Filesystem::make_dentry(&self.fs, Some(parent.clone()), name.to_owned(), inode) + Ext4Filesystem::iget_from_core_inode(&self.fs, inode) } fn block_size(&self) -> u64 { @@ -344,16 +344,14 @@ impl InodeDirOperations for Inode { _dir: &VfsInode, dentry: &LockedDentry<'_>, _flags: kvfs::InodeLookupFlags, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult> { let name = dentry.name(); - if name == "." { - return Ok(parent.clone()); - } - if name == ".." { - return parent.parent().ok_or(VfsError::NotFound); - } - self.lookup_child(&parent, name) + let inode = match self.lookup_child(name) { + Ok(inode) => inode, + Err(err) if err.canonicalize() == VfsError::NotFound => return Ok(None), + Err(err) => return Err(err), + }; + dentry.instantiate_or_alias(inode) } fn create( @@ -364,8 +362,7 @@ impl InodeDirOperations for Inode { mode: kvfs::Umode, _exclusive: bool, cred: &kcred::Cred, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); if mode.node_type() != NodeType::RegularFile { return Err(VfsError::InvalidInput); @@ -385,12 +382,8 @@ impl InodeDirOperations for Inode { .map_err(into_vfs_err)? }; self.fs.sync_vfs_directory(dir, created.parent())?; - Ext4Filesystem::make_dentry( - &self.fs, - Some(parent), - name.to_owned(), - created.child().clone(), - ) + let inode = Ext4Filesystem::iget_from_core_inode(&self.fs, created.child().clone())?; + dentry.instantiate(inode) } fn mkdir( @@ -400,8 +393,7 @@ impl InodeDirOperations for Inode { dentry: &LockedDentry<'_>, mode: kvfs::Umode, cred: &kcred::Cred, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let (mode, uid, gid) = inode_init_owner(dir, mode, cred); let created = { @@ -418,12 +410,8 @@ impl InodeDirOperations for Inode { .map_err(into_vfs_err)? }; self.fs.sync_vfs_directory(dir, created.parent())?; - Ext4Filesystem::make_dentry( - &self.fs, - Some(parent), - name.to_owned(), - created.child().clone(), - ) + let inode = Ext4Filesystem::iget_from_core_inode(&self.fs, created.child().clone())?; + dentry.instantiate(inode) } fn mknod( @@ -434,8 +422,7 @@ impl InodeDirOperations for Inode { mode: kvfs::Umode, device: DeviceId, cred: &kcred::Cred, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let kind = vfs_type_to_inode_kind(mode.node_type()).ok_or(VfsError::InvalidInput)?; let device = match mode.node_type() { @@ -459,12 +446,8 @@ impl InodeDirOperations for Inode { .map_err(into_vfs_err)? }; self.fs.sync_vfs_directory(dir, created.parent())?; - Ext4Filesystem::make_dentry( - &self.fs, - Some(parent), - name.to_owned(), - created.child().clone(), - ) + let inode = Ext4Filesystem::iget_from_core_inode(&self.fs, created.child().clone())?; + dentry.instantiate(inode) } fn symlink( @@ -474,8 +457,7 @@ impl InodeDirOperations for Inode { dentry: &LockedDentry<'_>, target: &str, cred: &kcred::Cred, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let (_, uid, gid) = inode_init_owner( dir, @@ -499,12 +481,8 @@ impl InodeDirOperations for Inode { .map_err(into_vfs_err)? }; self.fs.sync_vfs_directory(dir, created.parent())?; - Ext4Filesystem::make_dentry( - &self.fs, - Some(parent), - name.to_owned(), - created.child().clone(), - ) + let inode = Ext4Filesystem::iget_from_core_inode(&self.fs, created.child().clone())?; + dentry.instantiate(inode) } fn link( @@ -512,8 +490,7 @@ impl InodeDirOperations for Inode { old_dentry: &Dentry, dir: &VfsInode, dentry: &LockedDentry<'_>, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let target: Arc = old_dentry.downcast()?; let linked = { @@ -530,12 +507,8 @@ impl InodeDirOperations for Inode { }; self.fs.sync_vfs_directory(dir, linked.parent())?; sync_dentry_link_state(old_dentry, linked.target())?; - Ext4Filesystem::make_dentry( - &self.fs, - Some(parent), - name.to_owned(), - linked.target().clone(), - ) + let inode = Ext4Filesystem::iget_from_core_inode(&self.fs, linked.target().clone())?; + dentry.instantiate(inode) } fn unlink(&self, dir: &VfsInode, dentry: &LockedDentry<'_>) -> VfsResult<()> { diff --git a/fs/bridges/rsext4_vfs/src/inode.rs b/fs/bridges/rsext4_vfs/src/inode.rs index 6b6b52553..276486914 100644 --- a/fs/bridges/rsext4_vfs/src/inode.rs +++ b/fs/bridges/rsext4_vfs/src/inode.rs @@ -17,7 +17,7 @@ use kvfs::{ AddressSpace, AddressSpaceOperations, Dentry, DeviceId, DirContext, FileDirOperations, FileOperations, InodeDirOperations, InodeOperations, InodeSymlinkOperations, Kiocb, LockedDentry, Metadata, MetadataUpdate, NodeType, ReadaheadControl, VfsError, VfsFile, - VfsResult, WriteBeginRequest, WriteEndRequest, WritebackControl, inode_init_owner, + VfsInode, VfsResult, WriteBeginRequest, WriteEndRequest, WritebackControl, inode_init_owner, }; use rsext4::{BLOCK_SIZE, Jbd2Dev}; @@ -41,33 +41,13 @@ impl Inode { Arc::new(Self { fs, ino, node_type }) } - fn create_entry( - &self, - parent: &Dentry, - ino: u32, - inode: &rsext4::disknode::Ext4Inode, - name: impl Into, - ) -> Dentry { - let name = name.into(); - let inode = Ext4Filesystem::iget_from_disk_inode(&self.fs, ino, inode); - if inode.is_dir() { - Dentry::new_dir_from_inode(inode, Some(parent.clone()), name) - } else { - Dentry::new_file_from_inode(inode, Some(parent.clone()), name) - } - } - - fn lookup_locked(&self, parent: &Dentry, name: &str) -> VfsResult { - let parent_ino: u32 = parent - .inode() - .try_into() - .map_err(|_| VfsError::InvalidInput)?; + fn lookup_locked(&self, name: &str) -> VfsResult> { let mut state = self.fs.lock(); let (fs, dev) = state.split(); - let (ino, inode) = rsext4::dir::get_inode_by_name(fs, dev, parent_ino, name) + let (ino, inode) = rsext4::dir::get_inode_by_name(fs, dev, self.ino, name) .map_err(into_vfs_err)? .ok_or(VfsError::NotFound)?; - Ok(self.create_entry(parent, ino, &inode, name)) + Ok(Ext4Filesystem::iget_from_disk_inode(&self.fs, ino, &inode)) } fn update_ctime_with( @@ -208,16 +188,14 @@ impl InodeDirOperations for Inode { _dir: &kvfs::VfsInode, dentry: &LockedDentry<'_>, _flags: kvfs::InodeLookupFlags, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult> { let name = dentry.name(); - if name == "." { - return Ok(parent.clone()); - } - if name == ".." { - return parent.parent().ok_or(VfsError::NotFound); - } - self.lookup_locked(&parent, name) + let inode = match self.lookup_locked(name) { + Ok(inode) => inode, + Err(err) if err.canonicalize() == VfsError::NotFound => return Ok(None), + Err(err) => return Err(err), + }; + dentry.instantiate_or_alias(inode) } fn create( @@ -228,7 +206,7 @@ impl InodeDirOperations for Inode { mode: kvfs::Umode, _exclusive: bool, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let (mode, uid, gid) = inode_init_owner(dir, mode, cred); let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; let name = dentry.name(); @@ -264,11 +242,7 @@ impl InodeDirOperations for Inode { }; let inode = Ext4Filesystem::iget_from_disk_inode(&self.fs, ino, &inode); - Ok(Dentry::new_file_from_inode( - inode, - Some(parent.clone()), - name.to_owned(), - )) + dentry.instantiate(inode) } fn mkdir( @@ -278,7 +252,7 @@ impl InodeDirOperations for Inode { dentry: &LockedDentry<'_>, mode: kvfs::Umode, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let (mode, uid, gid) = inode_init_owner(dir, mode, cred); let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; let name = dentry.name(); @@ -313,11 +287,7 @@ impl InodeDirOperations for Inode { }; let inode = Ext4Filesystem::iget_from_disk_inode(&self.fs, ino, &inode); - Ok(Dentry::new_dir_from_inode( - inode, - Some(parent.clone()), - name.to_owned(), - )) + dentry.instantiate(inode) } fn mknod( @@ -328,7 +298,7 @@ impl InodeDirOperations for Inode { mode: kvfs::Umode, device: DeviceId, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let (mode, uid, gid) = inode_init_owner(dir, mode, cred); let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; let name = dentry.name(); @@ -384,11 +354,7 @@ impl InodeDirOperations for Inode { }; let inode = Ext4Filesystem::iget_from_disk_inode(&self.fs, ino, &inode); - Ok(Dentry::new_file_from_inode( - inode, - Some(parent.clone()), - name.to_owned(), - )) + dentry.instantiate(inode) } fn symlink( @@ -398,7 +364,7 @@ impl InodeDirOperations for Inode { dentry: &LockedDentry<'_>, target: &str, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let (_, uid, gid) = inode_init_owner( dir, kvfs::Umode::new( @@ -420,7 +386,8 @@ impl InodeDirOperations for Inode { .ok_or(VfsError::InvalidInput)?; Self::set_initial_owner(fs, dev, ino, uid, gid)?; } - self.lookup_locked(&parent, name) + let inode = self.lookup_locked(name)?; + dentry.instantiate(inode) } fn link( @@ -428,7 +395,7 @@ impl InodeDirOperations for Inode { node: &Dentry, _dir: &kvfs::VfsInode, dentry: &LockedDentry<'_>, - ) -> VfsResult { + ) -> VfsResult<()> { let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; let name = dentry.name(); let dir_path = parent.absolute_path()?.to_string(); @@ -454,7 +421,8 @@ impl InodeDirOperations for Inode { rsext4::file::link(fs, dev, &link_path, &target_path).map_err(into_vfs_err)?; Self::update_ctime_with(fs, dev, node.inode() as u32)?; } - self.lookup_locked(&parent, name) + let inode = self.lookup_locked(name)?; + dentry.instantiate(inode) } fn unlink(&self, _dir: &kvfs::VfsInode, dentry: &LockedDentry<'_>) -> VfsResult<()> { diff --git a/fs/bridges/v9fs/src/inode.rs b/fs/bridges/v9fs/src/inode.rs index 4d641d439..cf44a0c3c 100644 --- a/fs/bridges/v9fs/src/inode.rs +++ b/fs/bridges/v9fs/src/inode.rs @@ -121,23 +121,14 @@ impl Inode { } /// Look up a child entry via the 9P session. - fn lookup_locked(&self, parent: &Dentry, name: &str) -> VfsResult { + fn lookup_locked(&self, name: &str) -> VfsResult> { let child_path = join_child_path(&self.dir_path()?, name); let mut session = self.fs.lock(); let attr = session.getattr(&child_path).map_err(into_vfs_err)?; - Ok(self.create_entry_from_attr(parent, name, &attr, &child_path)) + Ok(self.create_inode_from_attr(&attr, &child_path)) } - /// Build a `Dentry` from 9P file attributes. - fn create_entry_from_attr( - &self, - parent: &Dentry, - name: &str, - attr: &FileAttr, - path: &str, - ) -> Dentry { - let d_parent = Some(parent.clone()); - let d_name = String::from(name); + fn create_inode_from_attr(&self, attr: &FileAttr, path: &str) -> Arc { let node_type = kvfs::Umode::from_bits(attr.mode as u16).node_type(); let node = if node_type == NodeType::Directory { Inode::new_dir(self.fs.clone(), Some(path.into())) @@ -149,19 +140,13 @@ impl Inode { } else { NodeFlags::empty() }; - node.into_dentry(inode_init_from_attr(attr), flags, d_parent, d_name) + node.into_vfs_inode(flags, inode_init_from_attr(attr)) } /// Create a Dentry for a symlink, using the 9P `TSYMLINK` operation. /// /// This is called from KFS path handling for the special symlink path. - fn create_symlink_entry( - &self, - parent: &Dentry, - name: &str, - target: &str, - gid: u32, - ) -> VfsResult { + fn create_symlink_inode(&self, name: &str, target: &str, gid: u32) -> VfsResult> { let dir_path = self.dir_path()?; let link_path = join_child_path(&dir_path, name); let mut session = self.fs.lock(); @@ -172,12 +157,8 @@ impl Inode { drop(session); Ok( - Inode::new_file(self.fs.clone(), NodeType::Symlink, Some(link_path)).into_dentry( - inode_init_from_attr(&attr), - NodeFlags::empty(), - Some(parent.clone()), - String::from(name), - ), + Inode::new_file(self.fs.clone(), NodeType::Symlink, Some(link_path)) + .into_vfs_inode(NodeFlags::empty(), inode_init_from_attr(&attr)), ) } @@ -354,16 +335,14 @@ impl InodeDirOperations for Inode { _dir: &kvfs::VfsInode, dentry: &LockedDentry<'_>, _flags: kvfs::InodeLookupFlags, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult> { let name = dentry.name(); - if name == "." { - return Ok(parent.clone()); - } - if name == ".." { - return parent.parent().ok_or(VfsError::NotFound); - } - self.lookup_locked(&parent, name) + let inode = match self.lookup_locked(name) { + Ok(inode) => inode, + Err(err) if err.canonicalize() == VfsError::NotFound => return Ok(None), + Err(err) => return Err(err), + }; + dentry.instantiate_or_alias(inode) } fn create( @@ -374,9 +353,8 @@ impl InodeDirOperations for Inode { mode: kvfs::Umode, _exclusive: bool, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let (mode, _, gid) = inode_init_owner(dir, mode, cred); - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; let name = dentry.name(); if mode.node_type() != NodeType::RegularFile { return Err(VfsError::InvalidInput); @@ -393,14 +371,9 @@ impl InodeDirOperations for Inode { let attr = session.getattr(&child_path).map_err(into_vfs_err)?; drop(session); - Ok( - Inode::new_file(self.fs.clone(), NodeType::RegularFile, Some(child_path)).into_dentry( - inode_init_from_attr(&attr), - NodeFlags::NON_CACHEABLE, - Some(parent.clone()), - name.to_string(), - ), - ) + let inode = Inode::new_file(self.fs.clone(), NodeType::RegularFile, Some(child_path)) + .into_vfs_inode(NodeFlags::NON_CACHEABLE, inode_init_from_attr(&attr)); + dentry.instantiate(inode) } fn mkdir( @@ -410,9 +383,8 @@ impl InodeDirOperations for Inode { dentry: &LockedDentry<'_>, mode: kvfs::Umode, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let (mode, _, gid) = inode_init_owner(dir, mode, cred); - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; let name = dentry.name(); if mode.node_type() != NodeType::Directory { return Err(VfsError::InvalidInput); @@ -427,14 +399,9 @@ impl InodeDirOperations for Inode { let attr = session.getattr(&child_path).map_err(into_vfs_err)?; drop(session); - Ok( - Inode::new_dir(self.fs.clone(), Some(child_path)).into_dentry( - inode_init_from_attr(&attr), - NodeFlags::empty(), - Some(parent.clone()), - name.to_string(), - ), - ) + let inode = Inode::new_dir(self.fs.clone(), Some(child_path)) + .into_vfs_inode(NodeFlags::empty(), inode_init_from_attr(&attr)); + dentry.instantiate(inode) } fn mknod( @@ -445,9 +412,8 @@ impl InodeDirOperations for Inode { mode: kvfs::Umode, device: DeviceId, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let (mode, _, gid) = inode_init_owner(dir, mode, cred); - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; let name = dentry.name(); let node_type = mode.node_type(); if matches!( @@ -467,14 +433,9 @@ impl InodeDirOperations for Inode { let attr = session.getattr(&child_path).map_err(into_vfs_err)?; drop(session); - Ok( - Inode::new_file(self.fs.clone(), node_type, Some(child_path)).into_dentry( - inode_init_from_attr(&attr), - NodeFlags::empty(), - Some(parent.clone()), - name.to_string(), - ), - ) + let inode = Inode::new_file(self.fs.clone(), node_type, Some(child_path)) + .into_vfs_inode(NodeFlags::empty(), inode_init_from_attr(&attr)); + dentry.instantiate(inode) } fn symlink( @@ -484,15 +445,15 @@ impl InodeDirOperations for Inode { dentry: &LockedDentry<'_>, target: &str, cred: &Cred, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let (_, _, gid) = inode_init_owner( dir, Umode::new(NodeType::Symlink, NodePermission::from_bits_truncate(0o777)), cred, ); - self.create_symlink_entry(&parent, name, target, gid) + let inode = self.create_symlink_inode(name, target, gid)?; + dentry.instantiate(inode) } fn link( @@ -500,8 +461,7 @@ impl InodeDirOperations for Inode { node: &Dentry, _dir: &kvfs::VfsInode, dentry: &LockedDentry<'_>, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let dir_path = self.dir_path()?; let link_path = join_child_path(&dir_path, name); @@ -513,12 +473,13 @@ impl InodeDirOperations for Inode { .map_err(into_vfs_err)?; drop(session); - self.lookup_locked(&parent, name) + let inode = self.lookup_locked(name)?; + dentry.instantiate(inode) } fn unlink(&self, _dir: &kvfs::VfsInode, dentry: &LockedDentry<'_>) -> VfsResult<()> { let dir_path = self.dir_path()?; - let child_path = join_child_path(&dir_path, &dentry.name()); + let child_path = join_child_path(&dir_path, dentry.name()); let mut session = self.fs.lock(); session.remove_path(&child_path).map_err(into_vfs_err) } @@ -537,8 +498,8 @@ impl InodeDirOperations for Inode { } let dst_dir = new_dentry.parent().ok_or(VfsError::InvalidInput)?; let dst_dir: Arc = dst_dir.downcast().map_err(|_| VfsError::InvalidInput)?; - let src_path = join_child_path(&self.dir_path()?, &old_dentry.name()); - let dst_path = join_child_path(&dst_dir.dir_path()?, &new_dentry.name()); + let src_path = join_child_path(&self.dir_path()?, old_dentry.name()); + let dst_path = join_child_path(&dst_dir.dir_path()?, new_dentry.name()); let mut session = self.fs.lock(); session .rename_path(&src_path, &dst_path) diff --git a/fs/filesystems/bpffs/src/lib.rs b/fs/filesystems/bpffs/src/lib.rs index 20c9e51c1..0db8596a4 100644 --- a/fs/filesystems/bpffs/src/lib.rs +++ b/fs/filesystems/bpffs/src/lib.rs @@ -226,14 +226,14 @@ impl BpfMap { flags: u32, cred: Arc, ) -> KResult> { - Ok(Arc::new(Self::new( + Arc::new(Self::new( map_type, key_size, value_size, max_entries, flags, )?) - .into_file(cred)?) + .into_file(cred) } /// Wraps this map as an anonymous inode file. @@ -327,22 +327,18 @@ impl BpfNode { Arc::new(Self { fs, inode }) } - fn new_entry(&self, parent: &Dentry, name: &str, entry: BpfEntry) -> VfsResult { + fn vfs_inode_for_entry(&self, entry: BpfEntry) -> Arc { let inode = entry.inode(); - let d_parent = Some(parent.clone()); - let d_name = String::from(name); - - Ok(match entry { - BpfEntry::Dir(_) => BpfNode::new(self.fs.clone(), inode).into_dentry( - NodeFlags::empty(), - d_parent, - d_name, - ), + + match entry { + BpfEntry::Dir(_) => { + BpfNode::new(self.fs.clone(), inode).into_vfs_inode(NodeFlags::empty()) + } BpfEntry::Program(_) => { let node = BpfNode::new(self.fs.clone(), inode); - node.into_dentry(NodeFlags::NON_CACHEABLE, d_parent, d_name) + node.into_vfs_inode(NodeFlags::NON_CACHEABLE) } - }) + } } fn into_vfs_inode(self: Arc, flags: NodeFlags) -> Arc { @@ -372,21 +368,6 @@ impl BpfNode { } } - fn into_dentry( - self: Arc, - flags: NodeFlags, - parent: Option, - name: String, - ) -> Dentry { - let is_dir = self.inode.is_dir(); - let inode = self.into_vfs_inode(flags); - if is_dir { - Dentry::new_dir_from_inode(inode, parent, name) - } else { - Dentry::new_file_from_inode(inode, parent, name) - } - } - fn pin_program( &self, name: &str, @@ -462,18 +443,14 @@ impl InodeDirOperations for BpfInodeOperations { _dir: &kvfs::VfsInode, dentry: &LockedDentry<'_>, _flags: kvfs::InodeLookupFlags, - ) -> VfsResult { - let dir = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult> { let name = dentry.name(); - let entry = self - .node - .inode - .as_dir()? - .lock() - .get(name) - .cloned() - .ok_or(VfsError::NotFound)?; - self.node.new_entry(&dir, name, entry) + let entry = self.node.inode.as_dir()?.lock().get(name).cloned(); + let Some(entry) = entry else { + return Ok(None); + }; + let inode = self.node.vfs_inode_for_entry(entry); + dentry.instantiate_or_alias(inode) } fn mkdir( @@ -483,8 +460,7 @@ impl InodeDirOperations for BpfInodeOperations { dentry: &LockedDentry<'_>, mode: kvfs::Umode, cred: &kcred::Cred, - ) -> VfsResult { - let dir = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let (mode, uid, gid) = inode_init_owner(dir_inode, mode, cred); let inode = Inode::new_dir(self.node.fs.clone(), mode.permission(), uid, gid); @@ -494,7 +470,9 @@ impl InodeDirOperations for BpfInodeOperations { return Err(VfsError::AlreadyExists); } entries.insert(name.to_string(), entry.clone()); - self.node.new_entry(&dir, name, entry) + drop(entries); + let inode = self.node.vfs_inode_for_entry(entry); + dentry.instantiate(inode) } fn link( @@ -502,7 +480,7 @@ impl InodeDirOperations for BpfInodeOperations { _old_dentry: &Dentry, _dir: &kvfs::VfsInode, _new_dentry: &LockedDentry<'_>, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::OperationNotPermitted) } diff --git a/fs/filesystems/fat/src/dir.rs b/fs/filesystems/fat/src/dir.rs index ded6663d2..573a362c9 100644 --- a/fs/filesystems/fat/src/dir.rs +++ b/fs/filesystems/fat/src/dir.rs @@ -43,15 +43,12 @@ impl FatDirInode { }) } - fn create_entry( + fn create_inode( &self, - parent: &Dentry, entry: ff::DirEntry<'_>, - name: impl Into, inode_number: u64, block_size: u64, - ) -> Dentry { - let d_name = name.into(); + ) -> Arc { if entry.is_file() { let mut file = entry.to_file(); let init = VfsInodeInit::from_metadata(&file_metadata( @@ -60,13 +57,12 @@ impl FatDirInode { &mut file, NodeType::RegularFile, )); - let vfs_inode = VfsInode::new_file( + VfsInode::new_file( FatFileInode::new(self.fs.clone(), self.fs.as_ref(), file, inode_number), init, - ); - Dentry::new_file_from_inode(vfs_inode, Some(parent.clone()), d_name) + ) } else { - let vfs_inode = VfsInode::new_openable_dir( + VfsInode::new_openable_dir( FatDirInode::new( self.fs.clone(), self.fs.as_ref(), @@ -74,8 +70,23 @@ impl FatDirInode { inode_number, ), dir_init(inode_number, block_size), - ); - Dentry::new_dir_from_inode(vfs_inode, Some(parent.clone()), d_name) + ) + } + } + + fn create_entry( + &self, + parent: &Dentry, + entry: ff::DirEntry<'_>, + name: String, + inode_number: u64, + block_size: u64, + ) -> Dentry { + let inode = self.create_inode(entry, inode_number, block_size); + if inode.is_dir() { + Dentry::new_dir_from_inode(inode, Some(parent.clone()), name) + } else { + Dentry::new_file_from_inode(inode, Some(parent.clone()), name) } } @@ -160,13 +171,13 @@ impl InodeDirOperations for FatDirInode { _dir: &kvfs::VfsInode, dentry: &LockedDentry<'_>, _flags: kvfs::InodeLookupFlags, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult> { let name = dentry.name(); let mut fs = self.fs.lock(); let block_size = fs.inner.cluster_size() as u64; let dir = self.inner.borrow(&fs); - dir.iter() + let entry = dir + .iter() .find_map(|entry| { entry .ok() @@ -174,9 +185,13 @@ impl InodeDirOperations for FatDirInode { }) .map(|entry| { let inode = fs.alloc_inode(); - self.create_entry(&parent, entry, name.to_ascii_lowercase(), inode, block_size) - }) - .ok_or(VfsError::NotFound) + self.create_inode(entry, inode, block_size) + }); + let Some(entry) = entry else { + return Ok(None); + }; + drop(fs); + dentry.instantiate_or_alias(entry) } fn create( @@ -187,12 +202,10 @@ impl InodeDirOperations for FatDirInode { mode: kvfs::Umode, _exclusive: bool, _cred: &kcred::Cred, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let mut fs = self.fs.lock(); let dir = self.inner.borrow(&fs); - let d_name = name.to_ascii_lowercase(); match mode.node_type() { NodeType::RegularFile => { let mut file = dir.create_file(name).map_err(into_vfs_err)?; @@ -208,11 +221,8 @@ impl InodeDirOperations for FatDirInode { FatFileInode::new(self.fs.clone(), self.fs.as_ref(), file, inode_number), init, ); - Ok(Dentry::new_file_from_inode( - vfs_inode, - Some(parent.clone()), - d_name, - )) + drop(fs); + dentry.instantiate(vfs_inode) } _ => Err(VfsError::InvalidInput), } @@ -225,8 +235,7 @@ impl InodeDirOperations for FatDirInode { dentry: &LockedDentry<'_>, _mode: kvfs::Umode, _cred: &kcred::Cred, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let mut fs = self.fs.lock(); let dir = self.inner.borrow(&fs); @@ -237,11 +246,8 @@ impl InodeDirOperations for FatDirInode { FatDirInode::new(self.fs.clone(), self.fs.as_ref(), child, inode_number), dir_init(inode_number, block_size), ); - Ok(Dentry::new_dir_from_inode( - vfs_inode, - Some(parent), - name.to_ascii_lowercase(), - )) + drop(fs); + dentry.instantiate(vfs_inode) } fn link( @@ -249,7 +255,7 @@ impl InodeDirOperations for FatDirInode { _old_dentry: &Dentry, _dir: &kvfs::VfsInode, _new_dentry: &LockedDentry<'_>, - ) -> VfsResult { + ) -> VfsResult<()> { // EPERM The filesystem containing oldpath and newpath does not // support the creation of hard links. Err(VfsError::PermissionDenied) @@ -284,13 +290,13 @@ impl InodeDirOperations for FatDirInode { // The default implementation throws EEXIST if dst exists, so we need to // dispatch_irq it - match dst_dir.inner.borrow(&fs).remove(&dst_name) { + match dst_dir.inner.borrow(&fs).remove(dst_name) { Ok(_) => {} Err(fatfs::Error::NotFound) => {} Err(err) => return Err(into_vfs_err(err)), } - dir.rename(&src_name, dst_dir.inner.borrow(&fs), &dst_name) + dir.rename(src_name, dst_dir.inner.borrow(&fs), dst_name) .map_err(into_vfs_err) } } diff --git a/fs/filesystems/memfs/src/lib.rs b/fs/filesystems/memfs/src/lib.rs index 5a0da20fc..b9c5d4622 100644 --- a/fs/filesystems/memfs/src/lib.rs +++ b/fs/filesystems/memfs/src/lib.rs @@ -126,7 +126,8 @@ impl MemoryFs { 0, DeviceId::default(), ); - let root = MemoryNode::dentry_from_inode(&fs, None, String::new(), root_ino); + let root_inode = MemoryNode::vfs_inode_from_inode(&fs, root_ino); + let root = Dentry::new_dir_from_inode(root_inode, None, String::new()); SuperBlock::new(fs, root) } @@ -304,17 +305,12 @@ impl MemoryNode { Arc::new(Self { fs, inode }) } - fn dentry_from_inode( - fs: &Arc, - d_parent: Option, - d_name: String, - inode: Arc, - ) -> Dentry { + fn vfs_inode_from_inode(fs: &Arc, inode: Arc) -> Arc { let metadata = inode.metadata.lock(); let node_type = metadata.mode.node_type(); let init = VfsInodeInit::from_metadata(&metadata); drop(metadata); - let vfs_inode = match node_type { + match node_type { NodeType::Directory => fs.inode_cache.get_or_insert_openable_dir_with_init( NodeFlags::empty(), init, @@ -337,18 +333,9 @@ impl MemoryNode { .get_or_insert_special_with_init(NodeFlags::empty(), init, || { MemoryNode::new(fs.clone(), inode) }), - }; - if node_type == NodeType::Directory { - Dentry::new_dir_from_inode(vfs_inode, d_parent, d_name) - } else { - Dentry::new_file_from_inode(vfs_inode, d_parent, d_name) } } - fn new_entry(&self, parent: &Dentry, name: &str, inode: Arc) -> Dentry { - Self::dentry_from_inode(&self.fs, Some(parent.clone()), name.to_owned(), inode) - } - fn ramfs_get_inode( &self, parent: Option, @@ -376,8 +363,7 @@ impl MemoryNode { mode: kvfs::Umode, device: DeviceId, cred: &Cred, - ) -> VfsResult { - let dir = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let content = self.inode.as_dir()?; let mut entries = content.entries.lock(); @@ -388,7 +374,8 @@ impl MemoryNode { let inode = self.ramfs_get_inode(Some(self.inode.ino), dir_inode, mode, device, cred); entries.insert(name.into(), InodeRef::new(self.fs.clone(), inode.ino)); drop(entries); - Ok(self.new_entry(&dir, name, inode)) + let inode = Self::vfs_inode_from_inode(&self.fs, inode); + dentry.instantiate(inode) } fn remove_dir_links(inode: &Arc) { @@ -490,15 +477,18 @@ impl InodeDirOperations for MemoryNode { _dir: &kvfs::VfsInode, dentry: &LockedDentry<'_>, _flags: kvfs::InodeLookupFlags, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult> { let name = dentry.name(); let content = self.inode.as_dir()?; let entries = content.entries.lock(); - let entry = entries.get(name).ok_or(VfsError::NotFound)?; + let Some(entry) = entries.get(name) else { + return Ok(None); + }; let inode = entry.get(); - Ok(self.new_entry(&parent, name, inode)) + drop(entries); + let inode = Self::vfs_inode_from_inode(&self.fs, inode); + dentry.instantiate_or_alias(inode) } fn create( @@ -509,7 +499,7 @@ impl InodeDirOperations for MemoryNode { mode: kvfs::Umode, _exclusive: bool, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let mode = mode.with_node_type(NodeType::RegularFile); self.ramfs_mknod(_dir, dentry, mode, DeviceId::default(), cred) } @@ -521,11 +511,11 @@ impl InodeDirOperations for MemoryNode { dentry: &LockedDentry<'_>, mode: kvfs::Umode, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let mode = mode.with_node_type(NodeType::Directory); - let entry = self.ramfs_mknod(dir_inode, dentry, mode, DeviceId::default(), cred)?; + self.ramfs_mknod(dir_inode, dentry, mode, DeviceId::default(), cred)?; dir_inode.increment_link_count(); - Ok(entry) + Ok(()) } fn mknod( @@ -536,7 +526,7 @@ impl InodeDirOperations for MemoryNode { mode: kvfs::Umode, device: DeviceId, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { self.ramfs_mknod(dir, dentry, mode, device, cred) } @@ -547,8 +537,7 @@ impl InodeDirOperations for MemoryNode { dentry: &LockedDentry<'_>, target: &str, cred: &Cred, - ) -> VfsResult { - let dir = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let content = self.inode.as_dir()?; let mut entries = content.entries.lock(); @@ -574,7 +563,9 @@ impl InodeDirOperations for MemoryNode { *file.symlink.lock() = Some(target.to_owned()); inode.metadata.lock().size = target.len() as u64; entries.insert(name.into(), InodeRef::new(self.fs.clone(), inode.ino)); - Ok(self.new_entry(&dir, name, inode)) + drop(entries); + let inode = Self::vfs_inode_from_inode(&self.fs, inode); + dentry.instantiate(inode) } fn link( @@ -582,8 +573,7 @@ impl InodeDirOperations for MemoryNode { target_dentry: &Dentry, _dir: &kvfs::VfsInode, dentry: &LockedDentry<'_>, - ) -> VfsResult { - let dir = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let name = dentry.name(); let content = self.inode.as_dir()?; let mut entries = content.entries.lock(); @@ -596,7 +586,9 @@ impl InodeDirOperations for MemoryNode { let inode = target.inode.clone(); entries.insert(name.into(), InodeRef::new(self.fs.clone(), inode.ino)); target_dentry.increment_link_count(); - Ok(self.new_entry(&dir, name, inode)) + drop(entries); + let inode = Self::vfs_inode_from_inode(&self.fs, inode); + dentry.instantiate(inode) } fn unlink(&self, dir_inode: &kvfs::VfsInode, dentry: &LockedDentry<'_>) -> VfsResult<()> { diff --git a/fs/kvfs/docs/design.md b/fs/kvfs/docs/design.md index bf1445a01..ad5640768 100644 --- a/fs/kvfs/docs/design.md +++ b/fs/kvfs/docs/design.md @@ -71,7 +71,9 @@ filesystem operation traits rename 不会 flush 文件数据，也不会替换 PageCache identity。 每个 live backing inode number 通过 filesystem `InodeCache` 复用同一个 `VfsInode`；hard -link、rename 和重复 lookup 因此共享 AddressSpace/PageCache。具体文件系统完成 mutation +link、rename 和重复 lookup 因此共享 AddressSpace/PageCache。非目录 inode 可以有多个 +dentry alias；目录 inode 至多有一个 live alias，重复 lookup 复用该 dentry，对应 Linux +`d_splice_alias()` 所依赖的目录单 alias 不变量。具体文件系统完成 mutation 后，operation callback 已持有 `VfsInode` 时可用 `update_metadata_after_backing_change()` 或 不改变 size 的 `update_attributes_after_backing_change()` 刷新 VFS 缓存；只持有目标 `Dentry` 时使用同名的 dentry metadata refresh。`Dentry` 不向外部 crate 暴露内部 @@ -114,20 +116,31 @@ VFS 采用 Linux directory-locking ownership model： - same-directory rename 按 parent dentry identity 判定并只锁一次父目录； - cross-directory rename 先获取 superblock topology mutex，再按拓扑顺序锁父目录。 +Slow lookup 对应 Linux `d_alloc_parallel()`：cache miss 的任务建立唯一 hashed negative +dentry，设置 parallel-lookup 状态并持有该 dentry 的 lookup mutex 后调用 filesystem +`lookup`；同名并发 lookup 复用该对象并等待 owner 完成。filesystem 以 `Ok(None)` 表达 +negative miss，找到 inode 时通常原位实例化 candidate；只有 `d_splice_alias()` 语义需要 +复用目录 alias 时才返回另一个 dentry。普通 lookup 和 namespace mutation 使用同一个 +locked lookup 对象方法，差别只在父目录分别持有 shared 或 exclusive namespace lock， +不存在 mutation 专属的 negative-cache 路径。 + rename 在两个父目录稳定后解析 source 和 target，然后先锁参与的子目录，再锁非目录 -inode。两个 parent dentry 即使引用同一个目录 inode，也仍属于 cross-directory topology -mutation；对应 inode lock 会去重。participant 最多是 source 和 target 两个 inode，直接按 +inode。目录单 alias 不变量保证不同 parent dentry 不会引用同一个目录 inode。participant +最多是 source 和 target 两个 inode，直接按 目录/非目录组合获取，不构造临时 `Vec`；两个非目录 inode 按指针值排序。父目录拓扑遍历 同时给出锁顺序和 Linux `lock_two_directories()` 语义中的 `trap`，最终 source/target 直接与 `trap` 比较，不再次遍历父链。类型、flag、祖先关系以及针对最终 source/target 的 Path -policy 都在同一个 namespace transaction 内完成；文件系统 callback 成功后，VFS 再提交 -dentry cache move 或 exchange。目录是否为空由 filesystem rename/rmdir callback 判定， +policy 都在同一个 namespace transaction 内完成；filesystem callback 前已保存旧/新 cache +key，成功后 VFS 只交换 dentry location 并原位替换已有 cache slot，不再分配 name 或插入 +新的 hash slot。目录是否为空由 filesystem rename/rmdir callback 判定， 通用层不把 dentry child cache 当作后端目录内容。 open-create 在同一个父目录 exclusive lock 下完成最终 lookup 和可能的 create，避免 `O_EXCL` 与 lookup/create 竞争。`O_CREAT | O_EXCL` 跳过 speculative lookup，直接执行锁内 最终 lookup；lookup 得到的同一个 negative dentry 会传给 filesystem create callback，不再 -按名称构造第二个对象。read-only mount 和创建权限属于 create-only 错误：只有锁内最终 +按名称构造第二个对象。create、mkdir、mknod、symlink 和 link callback 都必须实例化该 +negative dentry；lookup 只有在复用既有目录 alias 时才返回另一个 dentry。read-only mount +和创建权限属于 create-only 错误：只有锁内最终 lookup 仍为 negative 时才检查；若名称已经变为 positive，普通 `O_CREAT` 打开现有对象， `O_CREAT | O_EXCL` 返回 `AlreadyExists`。 @@ -219,8 +232,8 @@ dentry 的 inode、children 和可变 operation 状态由各自 mutex 保护。 在 `Lazy` 初始化闭包中构造复杂 VFS 对象。 Namespace callback 在 inode namespace lock 下运行。文件系统应在回调返回前把 core -mutation result 同步到受影响的 live inode；dentry cache 的删除/rebind 仍由 KVFS 在成功 -返回后完成。 +mutation result 同步到受影响的 live inode，并通过 `LockedDentry::instantiate()` 完成创建 +对象的 inode attachment；dentry cache 的删除/rebind 仍由 KVFS 在成功返回后完成。 全局 namespace lock 顺序为： @@ -229,9 +242,14 @@ superblock rename mutex -> parent-directory namespace locks -> child-directory namespace locks -> non-directory namespace locks (pointer order) + -> per-dentry parallel-lookup mutex + -> mount topology lock -> dentry cache and location locks ``` +挂载操作与 namespace validator 都按 inode namespace lock 在外、mount topology lock 在内 +的顺序执行，挂载点检查不会引入反向嵌套。parallel-lookup mutex 只序列化同一个 hashed +candidate 的 filesystem lookup callback，不会把不同名称的 slow lookup 串行化。 `SuperBlock::rename_mutex` 对应 Linux `s_vfs_rename_mutex`，不是 dcache rename seqlock；它只在 cross-directory rename 中获取。`VfsInode::namespace_lock` 表达 Linux `inode->i_rwsem` 中和 namespace 相关的子集。`DentryLocation` 用一个 `RwLock` @@ -278,6 +296,7 @@ parent children 弱索引与 superblock dentry cache 中移除 dentry；仍有 P - FAT 等不能原生表达 Unix UID/GID 的后端不能完整持久化创建者身份。 - 当前 POSIX rename 路径不支持 `RENAME_WHITEOUT`。 - superblock dentry cache 尚无 Linux 风格 LRU/shrinker。 -- fast lookup 仍是 mutex-based，没有 RCU 或 rename sequence validation。 +- fast lookup 仍是 mutex-based，没有 RCU 或 rename sequence validation；slow lookup 已按 + hashed candidate owner/waiter 模型合并同名并发 lookup。 - layered filesystem 的跨文件系统 lock rank 尚未建模。 - mount topology 同步与 superblock rename mutex 仍是不同机制。 diff --git a/fs/kvfs/docs/security.md b/fs/kvfs/docs/security.md index f81c378d6..3057c9fe2 100644 --- a/fs/kvfs/docs/security.md +++ b/fs/kvfs/docs/security.md @@ -38,6 +38,12 @@ namespace 状态前校验 name、mount relationship、类型、topology 和 oper - `LockedDentry` 的 location guard 限定借用的 dentry name 生命周期。 - `parent` 和 `name` 在同一个 location write lock 下同时替换。 - rename 期间 source dentry 和对应 `VfsInode` 保持存活。 +- 目录 inode 至多关联一个 live dentry；重复 lookup 必须复用该 alias，不能建立第二套 + child cache 和 topology location。 +- create-like callback 成功前必须实例化 VFS 提供的 negative dentry，不能返回另一个 + 未经事务校验的对象。 +- lookup miss 在 filesystem callback 前发布带 parallel-lookup 状态的 hashed negative + dentry；同名 lookup 必须等待 owner 完成，不能并行建立第二个 candidate。 - filesystem callback 不能直接修改 VFS cache internals。 - 每个 raw flags 家族必须在边界转换为对应 bitflags 类型。 - 未知 open/rename 位不得进入内部 namespace 或 open 算法。 @@ -76,8 +82,9 @@ children map 与 superblock dcache 不嵌套持锁；namespace 操作先更新 p 所有 namespace lock 都是 sleepable lock。全局顺序为 superblock topology、父目录、 子目录、非目录 inode，最后才是 dentry cache/location。cross-directory 由 parent dentry -identity 决定，父目录锁按 ancestor-first 顺序；互不为祖先时先锁 source parent。多个 -dentry alias 引用同一 inode 时对应 inode lock 去重，非目录 inode 按指针值排序。 +identity 决定，父目录锁按 ancestor-first 顺序；互不为祖先时先锁 source parent。目录 +inode 依赖单 alias 不变量，非目录 alias 对应的 inode lock 按 identity 去重并按指针值排序。 +mount topology lock 始终在相关 inode namespace lock 之后获取。 匿名 inode pseudo fs 的 singleton 由 `Once` 发布，但不允许普通运行时路径触发初始化； 并发创建匿名文件只共享已经发布的 mount/inode，不竞争初始化闭包。 @@ -114,7 +121,9 @@ lower filesystem lock；在推广此类嵌套前还需要明确的跨文件系 | T-19 | create 与 lookup、删除或 replacement 竞争 | 高 | final lookup、对象校验和 mutation 分离持锁 | final lookup、validator、participant lock 和 callback 位于同一父目录 exclusive transaction；create callback 复用最终 negative dentry，create-only 错误仅在该对象仍为 negative 时返回 | | T-20 | 反向 cross-directory rename 死锁 | 高 | 两个线程按相反顺序锁父目录 | 一个 topology mutex 串行化 topology mutation，父目录按拓扑顺序加锁 | | T-21 | dentry name 和 parent 不一致 | 中 | parent/name 分开更新或读者观察中间状态 | 两个字段在同一个 location write lock 下替换 | -| T-22 | 目录 alias 绕过 topology 序列化或重复锁 inode | 高 | 用 parent inode identity 判定 same-directory | topology 按 parent dentry identity 判定；parent 和 participant inode lock 独立去重 | +| T-22 | 目录 alias 形成两套 child cache 或绕过 topology 序列化 | 高 | 同一目录 inode 建立多个 live dentry | inode alias 表强制目录单 alias；lookup 复用已有 alias，rename topology 按 parent dentry identity 判定 | +| T-23 | filesystem callback 替换 VFS 已检查的创建对象 | 高 | callback 返回另一个 parent/name 下的 dentry | create-like callback 只能实例化事务 negative dentry；lookup alternate result 校验 positive state、parent 和 name | +| T-24 | 并发 slow lookup 建立重复 dentry | 高 | cache miss 检查与 candidate 发布分离，或等待者提前观察未完成对象 | candidate 在 callback 前原子加入 dcache；owner 保持 parallel-lookup 状态和 lookup mutex，等待者只在 lookup done 后读取结果 | ## 故障模式与影响分析（FMEA） @@ -130,8 +139,9 @@ lower filesystem lock；在推广此类嵌套前还需要明确的跨文件系 | F-08 | split truncate 在 backing prepare 后 cache invalidation 失败 | 分配或 mapping invalidation 错误 | 当前 truncate 返回失败 | backing inode 可能保持 orphan/recovery state | 2 | 由具体文件系统的持久化 recovery protocol 收敛，禁止静默执行 finish | 校验失败会在 filesystem callback 前返回 typed VFS error。后端失败时 dentry cache -location 保持不变，因为 cache commit 只在 callback 成功后执行；cache commit 是内存内 -不可失败步骤，持久化操作的 rollback 与 logging 仍由后端负责。 +location 保持不变，因为 cache commit 只在 callback 成功后执行；rename 所需 key 和 cache +slot 在 callback 前已经存在，commit 只交换 location 和原位替换 slot，不执行可能失败的 +分配。持久化操作的 rollback 与 logging 仍由后端负责。 ## 已知限制 @@ -143,9 +153,11 @@ location 保持不变，因为 cache commit 只在 callback 成功后执行；ca 删除和卸载路径主动驱逐。 - KVFS 提供 Mapping view invalidation 通知，但各文件系统仍需用 live mmap/truncate case 验证自身接线。 -- fast lookup 仍是 mutex-based，没有 RCU 或 rename sequence validation。 +- fast lookup 仍是 mutex-based，没有 RCU 或 rename sequence validation；同名 slow lookup + 通过 hashed candidate 的 owner/waiter 协议合并。 - layered filesystem 的 lock ordering 尚未建模。 -- mount topology synchronization 与 superblock rename mutex 是不同机制。 +- mount topology synchronization 与 superblock rename mutex 是不同机制，但均遵循已记录的 + inode-then-topology 嵌套顺序。 ## 审计清单 diff --git a/fs/kvfs/src/mount.rs b/fs/kvfs/src/mount.rs index 2c15a85b9..ec685ffce 100644 --- a/fs/kvfs/src/mount.rs +++ b/fs/kvfs/src/mount.rs @@ -526,9 +526,11 @@ impl Path { let name = name.to_string(); let entry = create(&self.dentry, name.clone()); - let inode = entry.inode(); - self.dentry.insert_cache(name, entry); - inode + self.dentry + .insert_cache(name, entry.clone()) + .filter(Dentry::is_really_positive) + .unwrap_or(entry) + .inode() } /// Returns metadata for the inode referenced by this location. @@ -883,6 +885,8 @@ impl Path { devname: Option<&str>, ) -> VfsResult> { self.dentry.as_dir()?; + let inode = self.inode(); + let _namespace_guard = inode.lock_namespace_exclusive(); let result = Mount::new_with_flags_and_devname(fs, Some(self.clone()), flags, devname); if let Some(old) = self.mnt.install_child_mount(&self.dentry, &result) { *result.covers.lock() = Some(old); @@ -1142,22 +1146,17 @@ mod tests { _dir: &VfsInode, dentry: &LockedDentry<'_>, _flags: crate::InodeLookupFlags, - ) -> VfsResult { + ) -> VfsResult> { let name = dentry.name(); if name != "mnt" { - return Err(VfsError::NotFound); + return Ok(None); } - let dir = dentry.parent().ok_or(VfsError::InvalidInput)?; let inode = VfsInode::new_openable_dir( Arc::new(MockDirOps::new(self.mount_flags, self.inode + 1)), inode_init(self.inode + 1), ); - Ok(Dentry::new_dir_from_inode( - inode, - Some(dir.clone()), - String::from(name), - )) + dentry.instantiate_or_alias(inode) } fn create( @@ -1168,7 +1167,7 @@ mod tests { _mode: crate::Umode, _exclusive: bool, _cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::OperationNotSupported) } @@ -1179,8 +1178,7 @@ mod tests { dentry: &LockedDentry<'_>, mode: crate::Umode, cred: &Cred, - ) -> VfsResult { - let parent = dentry.parent().ok_or(VfsError::InvalidInput)?; + ) -> VfsResult<()> { let (mode, uid, gid) = crate::inode_init_owner(dir, mode, cred); let inode = self.inode + 1; let init = VfsInodeInit::new(inode, 0, mode) @@ -1190,11 +1188,7 @@ mod tests { Arc::new(MockDirOps::new(self.mount_flags, inode)), init, ); - Ok(Dentry::new_dir_from_inode( - inode, - Some(parent.clone()), - String::from(dentry.name()), - )) + dentry.instantiate(inode) } fn link( @@ -1202,7 +1196,7 @@ mod tests { _old_dentry: &Dentry, _dir: &VfsInode, _new_dentry: &LockedDentry<'_>, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::OperationNotSupported) } diff --git a/fs/kvfs/src/namei.rs b/fs/kvfs/src/namei.rs index e3b32cc12..82d051bd6 100644 --- a/fs/kvfs/src/namei.rs +++ b/fs/kvfs/src/namei.rs @@ -956,7 +956,6 @@ mod tests { struct TestDir { inode: u64, children: crate::Mutex>, - miss_once: crate::Mutex>, } impl TestDir { @@ -964,17 +963,12 @@ mod tests { Self { inode, children: crate::Mutex::default(), - miss_once: crate::Mutex::new(None), } } fn insert(&self, name: &str, entry: Dentry) { self.children.lock().insert(String::from(name), entry); } - - fn miss_next_lookup(&self, name: &str) { - *self.miss_once.lock() = Some(String::from(name)); - } } impl InodeOperations for TestDir { @@ -1008,18 +1002,12 @@ mod tests { _dir: &VfsInode, dentry: &LockedDentry<'_>, _flags: crate::InodeLookupFlags, - ) -> VfsResult { - let mut miss_once = self.miss_once.lock(); - if miss_once.as_deref() == Some(dentry.name()) { - *miss_once = None; - return Err(VfsError::NotFound); - } - drop(miss_once); - self.children - .lock() - .get(dentry.name()) - .cloned() - .ok_or(VfsError::NotFound) + ) -> VfsResult> { + let entry = self.children.lock().get(dentry.name()).cloned(); + let Some(entry) = entry else { + return Ok(None); + }; + dentry.instantiate_or_alias(entry.vfs_inode()) } fn create( @@ -1030,7 +1018,7 @@ mod tests { _mode: crate::Umode, _exclusive: bool, _cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::OperationNotSupported) } @@ -1039,7 +1027,7 @@ mod tests { _old_dentry: &Dentry, _dir: &VfsInode, _new_dentry: &LockedDentry<'_>, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::OperationNotSupported) } @@ -1243,7 +1231,6 @@ mod tests { root: Path, base: Path, target: Path, - root_ops: Arc, magic_file: Arc, magic_dir: Arc, } @@ -1357,7 +1344,6 @@ mod tests { root: root_location.clone(), base: root_location, target: target_location, - root_ops, magic_file, magic_dir, } @@ -1448,9 +1434,8 @@ mod tests { } #[def_test] - fn open_create_relookup_precedes_create_only_errors() { + fn open_create_existing_precedes_create_only_errors() { let tree = test_tree(); - tree.root_ops.miss_next_lookup("target"); let mount = Mount::new_root_with_flags(&tree.fs, crate::MountFlags::RDONLY); let root = mount.root_path(); @@ -1461,7 +1446,7 @@ mod tests { NodePermission::empty(), kcred::initial_cred(), ); - assert!(existing.is_ok()); + assert_eq!(existing.map(|_| ()), Ok(())); let tree = test_tree(); let mount = Mount::new_root_with_flags(&tree.fs, crate::MountFlags::RDONLY); @@ -1491,7 +1476,7 @@ mod tests { &kcred::initial_cred(), ) .unwrap(); - assert!(followed.ptr_eq(&tree.target)); + assert!(followed.dentry().is_same_inode(tree.target.dentry())); let link = Filename::new("/link") .lookup_at( diff --git a/fs/kvfs/src/node/dentry.rs b/fs/kvfs/src/node/dentry.rs index d3ccead7a..e21ebeacd 100644 --- a/fs/kvfs/src/node/dentry.rs +++ b/fs/kvfs/src/node/dentry.rs @@ -33,6 +33,7 @@ bitflags! { /// Dentry state flags. #[derive(Debug, Clone, Copy)] pub struct DentryFlags: u32 { + const PAR_LOOKUP = 1 << 4; const DONTCACHE = 1 << 7; const ENTRY_TYPE = 7 << 19; const MISS_TYPE = 0 << 19; @@ -145,6 +146,7 @@ struct DentryLocation { struct DentryInner { flags: Mutex, + lookup_mutex: Mutex<()>, inode: Mutex>>, location: RwLock, operations: Mutex>>, @@ -167,6 +169,7 @@ impl DentryInner { .map(|super_block| Arc::downgrade(&super_block)); Self { flags: Mutex::new(flags), + lookup_mutex: Mutex::default(), inode: Mutex::new(inode), location: RwLock::new(DentryLocation { parent, name }), operations: Mutex::default(), @@ -204,6 +207,10 @@ impl DentryAlias { .upgrade() .is_some_and(|inner| Arc::ptr_eq(&inner, &dentry.0)) } + + pub(super) fn upgrade(&self) -> Option { + self.0.upgrade().map(Dentry) + } } /// Strong reference to a VFS directory entry. @@ -249,6 +256,22 @@ impl LockedDentry<'_> { pub fn as_dentry(&self) -> &Dentry { self.dentry } + + /// Attaches an inode to this negative transaction dentry. + pub fn instantiate(&self, inode: Arc) -> VfsResult<()> { + self.dentry.instantiate(inode) + } + + /// Instantiates this lookup dentry or returns an existing directory alias. + pub fn instantiate_or_alias(&self, inode: Arc) -> VfsResult> { + if inode.is_dir() + && let Some(alias) = inode.directory_alias() + { + return Ok(Some(alias)); + } + self.dentry.instantiate(inode)?; + Ok(None) + } } impl Deref for LockedDentry<'_> { @@ -376,14 +399,58 @@ impl Dentry { fn new_inner(inode: Arc, parent: Option, name: String) -> Self { let flags = DentryFlags::for_inode(&inode); - let dentry = Self(Arc::new(DentryInner::new(flags, Some(inode), parent, name))); - dentry.vfs_inode().add_dentry_alias(&dentry); + let dentry = Self(Arc::new(DentryInner::new( + flags, + Some(inode.clone()), + parent, + name, + ))); + if !inode.add_dentry_alias(&dentry) { + let alias = inode + .directory_alias() + .expect("directory alias disappeared while constructing dentry"); + let requested_location = dentry.0.location.read(); + let location = alias.0.location.read(); + let has_same_parent = match (&location.parent, &requested_location.parent) { + (Some(existing), Some(requested)) => existing.ptr_eq(requested), + (None, None) => true, + _ => false, + }; + assert!( + has_same_parent && location.name == requested_location.name, + "directory inode already has a live dentry alias" + ); + drop(location); + drop(requested_location); + return alias; + } if let Some(super_block) = dentry.super_block() { - dentry.vfs_inode().bind_super_block(&super_block); + inode.bind_super_block(&super_block); } dentry } + fn instantiate(&self, inode: Arc) -> VfsResult<()> { + let mut attached = self.0.inode.lock(); + if attached.is_some() { + return Err(VfsError::AlreadyExists); + } + if !inode.add_dentry_alias(self) { + return Err(VfsError::InvalidInput); + } + let mut flags = self.0.flags.lock(); + let retained = + *flags & (DentryFlags::PAR_LOOKUP | DentryFlags::DONTCACHE | DentryFlags::PERSISTENT); + *flags = DentryFlags::for_inode(&inode) | retained; + drop(flags); + *attached = Some(inode.clone()); + drop(attached); + if let Some(super_block) = self.super_block() { + inode.bind_super_block(&super_block); + } + Ok(()) + } + /// Construct a negative dentry for pathname lookup. pub fn new_negative(parent: Option, name: String) -> Self { Self(Arc::new(DentryInner::new( @@ -401,6 +468,9 @@ impl Dentry { } /// Construct a directory entry that points at an existing inode identity. + /// + /// Reuses an existing alias at the same location. A directory inode at a + /// different live location violates the VFS single-alias invariant. pub fn new_dir_from_inode(inode: Arc, parent: Option, name: String) -> Self { debug_assert!(inode.is_dir()); Self::new_inner(inode, parent, name) @@ -651,6 +721,73 @@ impl Dentry { !flags.contains(DentryFlags::DONTCACHE) || flags.contains(DentryFlags::PERSISTENT) } + fn is_parallel_lookup(&self) -> bool { + self.dentry_flags().contains(DentryFlags::PAR_LOOKUP) + } + + fn begin_parallel_lookup(&self) { + self.0.flags.lock().insert(DentryFlags::PAR_LOOKUP); + } + + fn end_parallel_lookup(&self) { + self.0.flags.lock().remove(DentryFlags::PAR_LOOKUP); + } + + fn lookup_cache_entry(&self, name: &str) -> Option { + let mut children = self.0.children.lock(); + let entry = children + .get(name) + .and_then(|entry| entry.upgrade().map(Dentry)); + if entry.is_none() { + children.remove(name); + } + entry + } + + fn cache_lookup_candidate(&self, name: &str, candidate: &Dentry) -> Option { + let mut children = self.0.children.lock(); + if let Some(entry) = children + .get(name) + .and_then(|entry| entry.upgrade().map(Dentry)) + { + return Some(entry); + } + children.insert(name.to_owned(), Arc::downgrade(&candidate.0)); + drop(children); + if let Some(super_block) = candidate.super_block() { + super_block.cache_dentry(candidate.clone()); + } + None + } + + fn replace_lookup_candidate(&self, name: &str, candidate: &Dentry, entry: &Dentry) { + let mut children = self.0.children.lock(); + if children + .get(name) + .is_some_and(|cached| Weak::ptr_eq(cached, &Arc::downgrade(&candidate.0))) + { + *children.get_mut(name).expect("checked cache entry") = Arc::downgrade(&entry.0); + } + drop(children); + if let Some(super_block) = candidate.super_block() { + super_block.cache_dentry(entry.clone()); + } + } + + fn uncache_lookup_candidate(&self, name: &str, candidate: &Dentry) { + let mut children = self.0.children.lock(); + let is_candidate = children + .get(name) + .is_some_and(|entry| Weak::ptr_eq(entry, &Arc::downgrade(&candidate.0))); + if is_candidate { + children.remove(name); + } + drop(children); + if is_candidate && let Some(super_block) = candidate.super_block() { + super_block.uncache_dentry(candidate); + } + } + fn remove_cache_entry(&self, name: &str) -> Option { let entry = self .0 @@ -678,34 +815,43 @@ impl Dentry { /// Looks up a directory entry by name in this dentry's cache. pub fn lookup_cache(&self, name: &str) -> Option { if self.can_cache_children() { - let mut children = self.0.children.lock(); - let entry = children - .get(name) - .and_then(|entry| entry.upgrade().map(Dentry)); - if entry.is_none() { - children.remove(name); - } - entry + self.lookup_cache_entry(name) + .filter(|entry| !entry.is_parallel_lookup()) } else { None } } - /// Inserts a child dentry into this dentry's cache. + /// Inserts a child dentry if the cache has no live entry with the same name. + /// + /// Returns the existing entry when the name is already cached. pub fn insert_cache(&self, name: String, entry: Dentry) -> Option { if self.can_cache_children() && entry.can_cache_as_child() { - let previous = self - .0 - .children - .lock() - .insert(name, Arc::downgrade(&entry.0)) - .and_then(|entry| entry.upgrade().map(Dentry)); + let mut children = self.0.children.lock(); + if let Some(existing) = children + .get(&name) + .and_then(|entry| entry.upgrade().map(Dentry)) + { + return Some(existing); + } + children.insert(name, Arc::downgrade(&entry.0)); + drop(children); if let Some(super_block) = entry.super_block() { super_block.cache_dentry(entry); } - previous + } + None + } + + fn swap_locations(left: &Dentry, right: &Dentry) { + if left.as_ptr() < right.as_ptr() { + let mut left_location = left.0.location.write(); + let mut right_location = right.0.location.write(); + mem::swap(&mut *left_location, &mut *right_location); } else { - None + let mut right_location = right.0.location.write(); + let mut left_location = left.0.location.write(); + mem::swap(&mut *left_location, &mut *right_location); } } @@ -713,16 +859,46 @@ impl Dentry { *self.0.location.write() = DentryLocation { parent, name }; } - fn d_move(&self, src_name: &str, src: &Dentry, dst_dir: &Self, dst_name: &str, dst: &Dentry) { - self.remove_cache_entry(src_name); - if dst.is_really_positive() { - dst_dir.forget_cache_entry(dst_name); + fn d_move( + &self, + src: &Dentry, + dst_dir: &Self, + dst: &Dentry, + destination_name: String, + keys: (&DentryKey, &DentryKey), + ) { + if dst.is_really_positive() + && let Ok(dir) = dst.as_dir() + { + dir.forget(); + } + + let source_location = src.0.location.read(); + let target_location = dst.0.location.read(); + if self.ptr_eq(dst_dir) { + let mut children = self.0.children.lock(); + children.remove(source_location.name.as_str()); + if let Some(target_slot) = children.get_mut(target_location.name.as_str()) { + *target_slot = Arc::downgrade(&src.0); + } } else { - dst_dir.remove_cache_entry(dst_name); + self.0.children.lock().remove(source_location.name.as_str()); + if let Some(target_slot) = dst_dir + .0 + .children + .lock() + .get_mut(target_location.name.as_str()) + { + *target_slot = Arc::downgrade(&src.0); + } } + drop(target_location); + drop(source_location); - src.rebind(Some(dst_dir.clone()), dst_name.to_owned()); - dst_dir.insert_cache(dst_name.to_owned(), src.clone()); + if let Some(super_block) = src.super_block() { + super_block.move_cached_dentry(keys.0, keys.1, src); + } + src.rebind(Some(dst_dir.clone()), destination_name); } fn d_exchange( @@ -732,15 +908,45 @@ impl Dentry { dst_dir: &Self, dst_name: &str, dst: &Dentry, + keys: (&DentryKey, &DentryKey), ) { - self.remove_cache_entry(src_name); - dst_dir.remove_cache_entry(dst_name); - - src.rebind(Some(dst_dir.clone()), dst_name.to_owned()); - dst.rebind(Some(self.clone()), src_name.to_owned()); + if self.ptr_eq(dst_dir) { + let mut children = self.0.children.lock(); + let source_cached = children.contains_key(src_name); + let target_cached = children.contains_key(dst_name); + if source_cached && target_cached { + *children.get_mut(src_name).expect("checked cache entry") = Arc::downgrade(&dst.0); + *children.get_mut(dst_name).expect("checked cache entry") = Arc::downgrade(&src.0); + } else { + children.remove(src_name); + children.remove(dst_name); + } + } else { + let source_cached = self.0.children.lock().contains_key(src_name); + let target_cached = dst_dir.0.children.lock().contains_key(dst_name); + if source_cached && target_cached { + *self + .0 + .children + .lock() + .get_mut(src_name) + .expect("checked cache entry") = Arc::downgrade(&dst.0); + *dst_dir + .0 + .children + .lock() + .get_mut(dst_name) + .expect("checked cache entry") = Arc::downgrade(&src.0); + } else { + self.0.children.lock().remove(src_name); + dst_dir.0.children.lock().remove(dst_name); + } + } - dst_dir.insert_cache(dst_name.to_owned(), src.clone()); - self.insert_cache(src_name.to_owned(), dst.clone()); + if let Some(super_block) = src.super_block() { + super_block.exchange_cached_dentries(keys.0, src, keys.1, dst); + } + Self::swap_locations(src, dst); } /// Looks up a child dentry below this directory. @@ -751,30 +957,68 @@ impl Dentry { } let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_shared(); - self.lookup_no_namespace_lock(&dir_inode, name) + self.lookup_positive_locked(&dir_inode, name) } - fn lookup_dentry_no_namespace_lock( - &self, - dir_inode: &VfsInode, - name: &str, - ) -> VfsResult { - if let Some(entry) = self.lookup_cache(name) { - return Ok(entry); - } + fn lookup_locked(&self, dir_inode: &VfsInode, name: &str) -> VfsResult { + loop { + if let Some(entry) = self.lookup_cache_entry(name) { + if entry.is_parallel_lookup() { + let _lookup_guard = entry.0.lookup_mutex.lock(); + continue; + } + return Ok(entry); + } - let candidate = Dentry::new_negative(Some(self.clone()), name.to_owned()); - let Some(entry) = dir_inode.lookup_child(&candidate)? else { - return Ok(candidate); - }; - if self.can_cache_children() && entry.can_cache_as_child() { - self.insert_cache(name.to_owned(), entry.clone()); + let candidate = Dentry::new_negative(Some(self.clone()), name.to_owned()); + let lookup_guard = candidate.0.lookup_mutex.lock(); + candidate.begin_parallel_lookup(); + if let Some(entry) = self.cache_lookup_candidate(name, &candidate) { + candidate.end_parallel_lookup(); + drop(lookup_guard); + if entry.is_parallel_lookup() { + let _lookup_guard = entry.0.lookup_mutex.lock(); + } + continue; + } + + let result = match dir_inode.lookup_child(&candidate) { + Ok(Some(entry)) => { + let location = entry.0.location.read(); + let has_expected_parent = location + .parent + .as_ref() + .is_some_and(|parent| parent.ptr_eq(self)); + let is_valid = + entry.is_really_positive() && has_expected_parent && location.name == name; + drop(location); + if is_valid { + self.replace_lookup_candidate(name, &candidate, &entry); + Ok(entry) + } else { + self.uncache_lookup_candidate(name, &candidate); + Err(VfsError::InvalidInput) + } + } + Ok(None) => Ok(candidate.clone()), + Err(err) => { + self.uncache_lookup_candidate(name, &candidate); + Err(err) + } + }; + if let Ok(entry) = &result + && (!self.can_cache_children() || !entry.can_cache_as_child()) + { + self.uncache_lookup_candidate(name, &candidate); + } + candidate.end_parallel_lookup(); + drop(lookup_guard); + return result; } - Ok(entry) } - fn lookup_no_namespace_lock(&self, dir_inode: &VfsInode, name: &str) -> VfsResult { - let entry = self.lookup_dentry_no_namespace_lock(dir_inode, name)?; + fn lookup_positive_locked(&self, dir_inode: &VfsInode, name: &str) -> VfsResult { + let entry = self.lookup_locked(dir_inode, name)?; if entry.is_negative() { Err(VfsError::NotFound) } else { @@ -793,9 +1037,15 @@ impl Dentry { Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); - let entry = dir_inode.create(self, name, permission, cred)?; - self.insert_cache(name.to_owned(), entry.clone()); - Ok(entry) + let candidate = self.lookup_locked(&dir_inode, name)?; + if candidate.is_really_positive() { + return Err(VfsError::AlreadyExists); + } + dir_inode.create(&candidate, permission, cred)?; + if candidate.is_negative() { + return Err(VfsError::InvalidInput); + } + Ok(candidate) } pub(crate) fn lookup_or_create_with_mode( @@ -813,7 +1063,7 @@ impl Dentry { Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); - let candidate = self.lookup_dentry_no_namespace_lock(&dir_inode, name)?; + let candidate = self.lookup_locked(&dir_inode, name)?; if candidate.is_really_positive() { return if exclusive { Err(VfsError::AlreadyExists) @@ -823,9 +1073,11 @@ impl Dentry { } may_create_fn()?; - let entry = dir_inode.create_with_mode(&candidate, mode, exclusive, cred)?; - self.insert_cache(name.to_owned(), entry.clone()); - Ok(LookupCreateResult::Created(entry)) + dir_inode.create_with_mode(&candidate, mode, exclusive, cred)?; + if candidate.is_negative() { + return Err(VfsError::InvalidInput); + } + Ok(LookupCreateResult::Created(candidate)) } /// Creates a directory child dentry below this directory. @@ -839,9 +1091,15 @@ impl Dentry { Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); - let entry = dir_inode.mkdir(self, name, permission, cred)?; - self.insert_cache(name.to_owned(), entry.clone()); - Ok(entry) + let candidate = self.lookup_locked(&dir_inode, name)?; + if candidate.is_really_positive() { + return Err(VfsError::AlreadyExists); + } + dir_inode.mkdir(&candidate, permission, cred)?; + if candidate.is_negative() { + return Err(VfsError::InvalidInput); + } + Ok(candidate) } /// Creates a special child dentry below this directory. @@ -857,9 +1115,15 @@ impl Dentry { Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); - let entry = dir_inode.mknod(self, name, node_type, permission, device, cred)?; - self.insert_cache(name.to_owned(), entry.clone()); - Ok(entry) + let candidate = self.lookup_locked(&dir_inode, name)?; + if candidate.is_really_positive() { + return Err(VfsError::AlreadyExists); + } + dir_inode.mknod(&candidate, node_type, permission, device, cred)?; + if candidate.is_negative() { + return Err(VfsError::InvalidInput); + } + Ok(candidate) } /// Creates a symbolic-link child dentry below this directory. @@ -868,9 +1132,15 @@ impl Dentry { Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); - let entry = dir_inode.symlink(self, name, target, cred)?; - self.insert_cache(name.to_owned(), entry.clone()); - Ok(entry) + let candidate = self.lookup_locked(&dir_inode, name)?; + if candidate.is_really_positive() { + return Err(VfsError::AlreadyExists); + } + dir_inode.symlink(&candidate, target, cred)?; + if candidate.is_negative() { + return Err(VfsError::InvalidInput); + } + Ok(candidate) } /// Creates a hard link below this directory. @@ -882,11 +1152,17 @@ impl Dentry { } let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); + let candidate = self.lookup_locked(&dir_inode, name)?; + if candidate.is_really_positive() { + return Err(VfsError::AlreadyExists); + } let source_inode = node.vfs_inode(); let _source_guard = source_inode.lock_namespace_exclusive(); - let entry = dir_inode.link(self, name, node)?; - self.insert_cache(name.to_owned(), entry.clone()); - Ok(entry) + dir_inode.link(&candidate, node)?; + if candidate.is_negative() { + return Err(VfsError::InvalidInput); + } + Ok(candidate) } /// Unlinks a non-directory child by name. @@ -902,8 +1178,11 @@ impl Dentry { Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); - let entry = self.lookup_no_namespace_lock(&dir_inode, name)?; + let entry = self.lookup_positive_locked(&dir_inode, name)?; let victim_inode = entry.vfs_inode(); + if Arc::ptr_eq(&victim_inode, &dir_inode) { + return Err(VfsError::InvalidInput); + } let _victim_guard = victim_inode.lock_namespace_exclusive(); if entry.is_dir() { return Err(VfsError::IsADirectory); @@ -930,8 +1209,11 @@ impl Dentry { Self::verify_child_name(name)?; let dir_inode = self.vfs_inode(); let _namespace_guard = dir_inode.lock_namespace_exclusive(); - let entry = self.lookup_no_namespace_lock(&dir_inode, name)?; + let entry = self.lookup_positive_locked(&dir_inode, name)?; let victim_inode = entry.vfs_inode(); + if Arc::ptr_eq(&victim_inode, &dir_inode) { + return Err(VfsError::InvalidInput); + } let _victim_guard = victim_inode.lock_namespace_exclusive(); if !entry.is_dir() { return Err(VfsError::NotADirectory); @@ -1054,11 +1336,11 @@ impl Dentry { /// Clears cached child dentries and per-dentry data recursively. pub(crate) fn forget(&self) { - let children: Vec<_> = mem::take(&mut *self.0.children.lock()) + let children = mem::take(&mut *self.0.children.lock()); + for child in children .into_values() .filter_map(|entry| entry.upgrade().map(Dentry)) - .collect(); - for child in children { + { if child.is_really_positive() && let Ok(dir) = child.as_dir() { @@ -1173,10 +1455,10 @@ impl RenameData<'_> { { let source = self .old_parent - .lookup_no_namespace_lock(old_dir_inode, self.old_name)?; + .lookup_positive_locked(old_dir_inode, self.old_name)?; let target = self .new_parent - .lookup_dentry_no_namespace_lock(new_dir_inode, self.new_name)?; + .lookup_locked(new_dir_inode, self.new_name)?; if target.is_negative() && self.flags.contains(RenameFlags::EXCHANGE) { return Err(VfsError::NotFound); } @@ -1254,6 +1536,10 @@ impl RenameData<'_> { F: FnOnce(&Dentry, &Dentry) -> VfsResult<()>, { self.validate_locked(source, target)?; + let source_key = source.key(); + let target_key = target.key(); + let destination_name = + (!self.flags.contains(RenameFlags::EXCHANGE)).then(|| target.name_snapshot()); may_rename_fn(source, target)?; old_dir_inode.rename(source, new_dir_inode, target, self.flags)?; if self.flags.contains(RenameFlags::EXCHANGE) { @@ -1263,14 +1549,15 @@ impl RenameData<'_> { self.new_parent, self.new_name, target, + (&source_key, &target_key), ); } else { self.old_parent.d_move( - self.old_name, source, self.new_parent, - self.new_name, target, + destination_name.expect("non-exchange rename has a destination name"), + (&source_key, &target_key), ); } Ok(()) @@ -1575,20 +1862,26 @@ mod tests_dentry { _dir: &VfsInode, _dentry: &LockedDentry<'_>, _flags: crate::InodeLookupFlags, - ) -> VfsResult { - Err(VfsError::NotFound) + ) -> VfsResult> { + Ok(None) } fn create( &self, _idmap: &crate::MountIdmap, _dir: &VfsInode, - _dentry: &LockedDentry<'_>, - _mode: crate::Umode, + dentry: &LockedDentry<'_>, + mode: crate::Umode, _exclusive: bool, _cred: &kcred::Cred, - ) -> VfsResult { - Err(VfsError::OperationNotSupported) + ) -> VfsResult<()> { + let operations = Arc::new(MockFileOps::new( + Arc::new(MockFilesystem), + self.inode + 1, + &[], + )); + let inode = VfsInode::new_file(operations, VfsInodeInit::new(self.inode + 1, 0, mode)); + dentry.instantiate(inode) } fn link( @@ -1596,7 +1889,7 @@ mod tests_dentry { _old_dentry: &Dentry, _dir: &VfsInode, _new_dentry: &LockedDentry<'_>, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::OperationNotSupported) } @@ -1817,6 +2110,23 @@ mod tests_dentry { assert!(root.lookup_cache("missing").is_none()); } + #[def_test] + fn test_create_instantiates_cached_lookup_miss() { + let fs = Arc::new(MockFilesystem); + let root = make_dir_entry(fs.clone(), 24, ""); + let _super_block = SuperBlock::new(fs, root.clone()); + + assert!(matches!(root.lookup("created"), Err(VfsError::NotFound))); + let negative = root.lookup_cache("created").unwrap(); + let created = root + .create("created", NodePermission::default(), &kcred::initial_cred()) + .unwrap(); + + assert!(created.ptr_eq(&negative)); + assert!(created.is_really_positive()); + assert!(root.lookup_cache("created").unwrap().ptr_eq(&created)); + } + #[def_test] fn test_rename_moves_cache_entry_without_changing_inode_identity() { let fs = Arc::new(MockFilesystem); @@ -2061,38 +2371,48 @@ mod tests_dentry { } #[def_test] - fn test_rename_distinct_parent_aliases_deduplicates_inode_lock() { + fn test_directory_inode_rejects_second_live_alias() { let root = make_renamable_dir_entry(66, None, ""); let parent_inode = VfsInode::new_openable_dir( Arc::new(MockDirOps::new_renamable(67)), inode_init(67, NodeType::Directory, 0), ); - let old_parent = Dentry::new_dir_from_inode( + let parent = Dentry::new_dir_from_inode( parent_inode.clone(), Some(root.clone()), - String::from("old"), + String::from("parent"), ); - let new_parent = - Dentry::new_dir_from_inode(parent_inode, Some(root.clone()), String::from("new")); - let (source, _) = make_file_entry( - Arc::new(MockFilesystem), - 68, - Some(old_parent.clone()), - "source", + root.insert_cache(String::from("parent"), parent.clone()); + let candidate = Dentry::new_negative(Some(root), String::from("alias")); + + assert_eq!( + candidate.instantiate(parent_inode), + Err(VfsError::InvalidInput) ); - root.insert_cache(String::from("old"), old_parent.clone()); - root.insert_cache(String::from("new"), new_parent.clone()); - old_parent.insert_cache(String::from("source"), source.clone()); - let _super_block = SuperBlock::new(Arc::new(MockFilesystem), root); + assert!(candidate.is_negative()); + } - assert!(!old_parent.ptr_eq(&new_parent)); - assert!(old_parent.is_same_inode(&new_parent)); - old_parent - .rename("source", &new_parent, "target", RenameFlags::empty()) - .unwrap(); + #[def_test] + fn test_directory_constructor_reuses_alias_at_same_location() { + let root = make_renamable_dir_entry(75, None, ""); + let inode = VfsInode::new_openable_dir( + Arc::new(MockDirOps::new_renamable(76)), + inode_init(76, NodeType::Directory, 0), + ); + let first = + Dentry::new_dir_from_inode(inode.clone(), Some(root.clone()), String::from("child")); + let second = Dentry::new_dir_from_inode(inode, Some(root), String::from("child")); - assert!(old_parent.lookup_cache("source").is_none()); - assert!(new_parent.lookup_cache("target").unwrap().ptr_eq(&source)); + assert!(first.ptr_eq(&second)); + } + + #[def_test] + fn test_remove_rejects_parent_inode_as_victim() { + let root = make_renamable_dir_entry(77, None, ""); + root.insert_cache(String::from("self"), root.clone()); + + assert_eq!(root.unlink("self"), Err(VfsError::InvalidInput)); + assert_eq!(root.rmdir("self"), Err(VfsError::InvalidInput)); } #[def_test] diff --git a/fs/kvfs/src/node/inode.rs b/fs/kvfs/src/node/inode.rs index 118481bcc..6ccf34d7e 100644 --- a/fs/kvfs/src/node/inode.rs +++ b/fs/kvfs/src/node/inode.rs @@ -5,7 +5,6 @@ //! VFS inode identity and inode cache helpers. use alloc::{ - borrow::ToOwned, string::String, sync::{Arc, Weak}, vec::Vec, @@ -119,6 +118,12 @@ bitflags! { /// names are passed as [`LockedDentry`], allowing callbacks to borrow /// `dentry.name()` without cloning. /// +/// Lookup follows Linux `->lookup`: a miss leaves the supplied dentry negative +/// and returns `Ok(None)`, while a found inode normally instantiates that same +/// dentry and also returns `Ok(None)`. `Ok(Some(_))` is reserved for an existing +/// directory alias, matching `d_splice_alias()`. Create-like callbacks must +/// instantiate the supplied dentry before returning success. +/// /// Implementations may sleep, but must not re-enter namespace operations on the /// same VFS objects while these locks are held. pub trait InodeDirOperations: Send + Sync { @@ -127,7 +132,7 @@ pub trait InodeDirOperations: Send + Sync { _dir: &VfsInode, _dentry: &LockedDentry<'_>, _flags: InodeLookupFlags, - ) -> VfsResult { + ) -> VfsResult> { Err(VfsError::NotADirectory) } @@ -139,7 +144,7 @@ pub trait InodeDirOperations: Send + Sync { _mode: Umode, _exclusive: bool, _cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::PermissionDenied) } @@ -148,7 +153,7 @@ pub trait InodeDirOperations: Send + Sync { _old_dentry: &Dentry, _dir: &VfsInode, _new_dentry: &LockedDentry<'_>, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::NotADirectory) } @@ -163,7 +168,7 @@ pub trait InodeDirOperations: Send + Sync { _dentry: &LockedDentry<'_>, _symname: &str, _cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::NotADirectory) } @@ -174,7 +179,7 @@ pub trait InodeDirOperations: Send + Sync { _dentry: &LockedDentry<'_>, _mode: Umode, _cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::OperationNotPermitted) } @@ -190,7 +195,7 @@ pub trait InodeDirOperations: Send + Sync { _mode: Umode, _device: DeviceId, _cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::OperationNotPermitted) } @@ -426,13 +431,23 @@ impl InodeIdentity { self.super_block.get().and_then(Weak::upgrade) } - fn add_alias(&self, dentry: &Dentry) { + fn add_alias(&self, dentry: &Dentry, is_directory: bool) -> bool { let mut aliases = self.aliases.lock(); aliases.retain(DentryAlias::is_live); if aliases.iter().any(|alias| alias.points_to(dentry)) { - return; + return true; + } + if is_directory && !aliases.is_empty() { + return false; } aliases.push(DentryAlias::new(dentry)); + true + } + + fn directory_alias(&self) -> Option { + let mut aliases = self.aliases.lock(); + aliases.retain(DentryAlias::is_live); + aliases.first().and_then(DentryAlias::upgrade) } } @@ -1425,7 +1440,7 @@ impl VfsInode { mode: Umode, exclusive: bool, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let dentry = dentry.lock_location(); self.require_directory_operations()?.create( &MountIdmap, @@ -1438,40 +1453,29 @@ impl VfsInode { } pub(crate) fn lookup_child(&self, dentry: &Dentry) -> VfsResult> { - let result = { - let dentry = dentry.lock_location(); - self.require_directory_operations()? - .lookup(self, &dentry, InodeLookupFlags::empty()) - }; - match result { - Ok(entry) => Ok(Some(entry)), - Err(err) if err.canonicalize() == VfsError::NotFound => Ok(None), - Err(err) => Err(err), - } + let dentry = dentry.lock_location(); + self.require_directory_operations()? + .lookup(self, &dentry, InodeLookupFlags::empty()) } /// Create a regular-file child below this directory inode. - pub fn create( + pub(crate) fn create( &self, - dir: &Dentry, - name: &str, + dentry: &Dentry, permission: NodePermission, cred: &Cred, - ) -> VfsResult { + ) -> VfsResult<()> { let mode = Umode::new(NodeType::RegularFile, permission); - let dentry = Dentry::new_negative(Some(dir.clone()), name.to_owned()); - self.create_with_mode(&dentry, mode, false, cred) + self.create_with_mode(dentry, mode, false, cred) } /// Create a directory child below this directory inode. - pub fn mkdir( + pub(crate) fn mkdir( &self, - dir: &Dentry, - name: &str, + dentry: &Dentry, permission: NodePermission, cred: &Cred, - ) -> VfsResult { - let dentry = Dentry::new_negative(Some(dir.clone()), name.to_owned()); + ) -> VfsResult<()> { let dentry = dentry.lock_location(); let mode = Umode::new(NodeType::Directory, permission); self.require_directory_operations()? @@ -1479,16 +1483,14 @@ impl VfsInode { } /// Create a special child below this directory inode. - pub fn mknod( + pub(crate) fn mknod( &self, - dir: &Dentry, - name: &str, + dentry: &Dentry, node_type: NodeType, permission: NodePermission, device: DeviceId, cred: &Cred, - ) -> VfsResult { - let dentry = Dentry::new_negative(Some(dir.clone()), name.to_owned()); + ) -> VfsResult<()> { let dentry = dentry.lock_location(); let mode = Umode::new(node_type, permission); self.require_directory_operations()? @@ -1496,22 +1498,14 @@ impl VfsInode { } /// Create a symbolic link below this directory inode. - pub fn symlink( - &self, - dir: &Dentry, - name: &str, - target: &str, - cred: &Cred, - ) -> VfsResult { - let dentry = Dentry::new_negative(Some(dir.clone()), name.to_owned()); + pub(crate) fn symlink(&self, dentry: &Dentry, target: &str, cred: &Cred) -> VfsResult<()> { let dentry = dentry.lock_location(); self.require_directory_operations()? .symlink(&MountIdmap, self, &dentry, target, cred) } /// Link `source` below this directory inode. - pub fn link(&self, dir: &Dentry, name: &str, source: &Dentry) -> VfsResult { - let dentry = Dentry::new_negative(Some(dir.clone()), name.to_owned()); + pub(crate) fn link(&self, dentry: &Dentry, source: &Dentry) -> VfsResult<()> { let dentry = dentry.lock_location(); self.require_directory_operations()? .link(source, self, &dentry) @@ -1586,8 +1580,16 @@ impl VfsInode { super_block.register_inode(self); } - pub(crate) fn add_dentry_alias(&self, dentry: &Dentry) { - self.identity.add_alias(dentry); + pub(crate) fn add_dentry_alias(&self, dentry: &Dentry) -> bool { + self.identity.add_alias(dentry, self.is_dir()) + } + + pub(crate) fn directory_alias(&self) -> Option { + if self.is_dir() { + self.identity.directory_alias() + } else { + None + } } /// Sets the access timestamp. diff --git a/fs/kvfs/src/nullfs.rs b/fs/kvfs/src/nullfs.rs index 3106b6f1b..7f26e2083 100644 --- a/fs/kvfs/src/nullfs.rs +++ b/fs/kvfs/src/nullfs.rs @@ -114,8 +114,8 @@ impl InodeDirOperations for NullFsRoot { _dir: &VfsInode, _dentry: &LockedDentry<'_>, _flags: InodeLookupFlags, - ) -> VfsResult { - Err(VfsError::NotFound) + ) -> VfsResult> { + Ok(None) } } diff --git a/fs/kvfs/src/simple_dir.rs b/fs/kvfs/src/simple_dir.rs index f350a0b73..ee9ad588c 100644 --- a/fs/kvfs/src/simple_dir.rs +++ b/fs/kvfs/src/simple_dir.rs @@ -430,10 +430,15 @@ impl InodeDirOperations for SimpleDirInodeOperations { _dir: &crate::VfsInode, dentry: &LockedDentry<'_>, _flags: crate::InodeLookupFlags, - ) -> VfsResult { + ) -> VfsResult> { let dir = dentry.parent().ok_or(VfsError::InvalidInput)?; let name = dentry.name(); - self.dir.ops.lookup_child(SimpleDirLookup::new(&dir), name) + let entry = match self.dir.ops.lookup_child(SimpleDirLookup::new(&dir), name) { + Ok(entry) => entry, + Err(err) if err.canonicalize() == VfsError::NotFound => return Ok(None), + Err(err) => return Err(err), + }; + Ok(Some(entry)) } fn mknod( @@ -444,7 +449,7 @@ impl InodeDirOperations for SimpleDirInodeOperations { mode: Umode, device: DeviceId, cred: &kcred::Cred, - ) -> VfsResult { + ) -> VfsResult<()> { if !matches!( mode.node_type(), NodeType::CharacterDevice | NodeType::BlockDevice | NodeType::Fifo | NodeType::Socket @@ -464,9 +469,13 @@ impl InodeDirOperations for SimpleDirInodeOperations { node.set_rdev(device); let init = node.inode_init(); let inode = VfsInode::new_special(node, NodeFlags::empty(), init); - self.dir - .ops - .create_inode_child(SimpleDirLookup::new(&parent), dentry.name(), inode) + let entry = + self.dir + .ops + .create_inode_child(SimpleDirLookup::new(&parent), dentry.name(), inode)?; + let inode = entry.vfs_inode(); + drop(entry); + dentry.instantiate(inode) } fn link( @@ -474,7 +483,7 @@ impl InodeDirOperations for SimpleDirInodeOperations { _old_dentry: &Dentry, _dir: &crate::VfsInode, _new_dentry: &LockedDentry<'_>, - ) -> VfsResult { + ) -> VfsResult<()> { Err(VfsError::OperationNotPermitted) } diff --git a/fs/kvfs/src/super_block.rs b/fs/kvfs/src/super_block.rs index c74aecf16..89c1d7a06 100644 --- a/fs/kvfs/src/super_block.rs +++ b/fs/kvfs/src/super_block.rs @@ -249,6 +249,41 @@ impl SuperBlock { drop(removed); } + pub(crate) fn move_cached_dentry( + &self, + old_key: &DentryKey, + new_key: &DentryKey, + source: &Dentry, + ) { + let mut cache = self.dentry_cache.lock(); + let removed = cache.remove(old_key); + if let Some(target_slot) = cache.get_mut(new_key) { + *target_slot = source.clone(); + } + drop(cache); + drop(removed); + } + + pub(crate) fn exchange_cached_dentries( + &self, + old_key: &DentryKey, + source: &Dentry, + new_key: &DentryKey, + target: &Dentry, + ) { + let mut cache = self.dentry_cache.lock(); + if cache.contains_key(old_key) && cache.contains_key(new_key) { + *cache.get_mut(old_key).expect("checked cache entry") = target.clone(); + *cache.get_mut(new_key).expect("checked cache entry") = source.clone(); + } else { + let old = cache.remove(old_key); + let new = cache.remove(new_key); + drop(cache); + drop(old); + drop(new); + } + } + /// Returns this superblock's maximum regular-file size. pub fn max_file_size(&self) -> u64 { self.max_file_size -- Gitee