Skip to content

Conversation

@kaleidoscope416
Copy link
Contributor

@kaleidoscope416 kaleidoscope416 commented Nov 10, 2025

-修复 VFS 中相对路径与 dirfd 的解析逻辑,确保 parent inode 选取正确
-增强路径名检查,处理空路径/特殊名等异常情况
-添加针对 unlink 的测试用例


Note

Fixes parent inode resolution for dirfd-relative paths in openat/unlinkat, adds stricter path/dir checks and unlink behavior, and enables unlink tests.

  • VFS:
    • Parent resolution: Add vcore::resolve_parent_inode and use it in open.rs::do_sys_openat2 (on O_CREAT) and vcore.rs::{do_remove_dir, do_unlink_at} to correctly resolve parents for dirfd-relative paths.
    • Unlink / Rmdir validation:
      • do_remove_dir: reject / (EBUSY), empty path (ENOENT); ensure parent and target are dirs.
      • do_unlink_at: reject empty path (ENOENT), trailing / (ENOTDIR); check parent is dir; do not follow final symlink; return EISDIR for directories; truncate page cache before unlink.
  • Tests:
    • Enable unlink_test in gvisor/whitelist.txt and add blocklisted cases in blocklists/unlink_test (pending fchmod).

Written by Cursor Bugbot for commit 6592e11. This will update automatically on new commits. Configure here.

Copilot AI review requested due to automatic review settings November 10, 2025 12:26
@github-actions github-actions bot added the Bug fix A bug is fixed in this pull request label Nov 10, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for the unlink_test from the gvisor test suite and fixes several path handling issues in filesystem operations. The changes improve edge case handling for empty paths, trailing slashes, and parent directory lookups.

Key changes:

  • Added unlink_test to the gvisor test whitelist with a corresponding blocklist for unsupported test cases
  • Fixed parent directory lookup logic in do_remove_dir, do_unlink_at, and do_sys_openat2 to handle cases where parent_path is None
  • Added validation for empty paths and trailing slashes in do_unlink_at
  • Added validation for empty paths and root directory in do_remove_dir
  • Removed unused variable and unnecessary blank lines

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
user/apps/tests/syscall/gvisor/whitelist.txt Adds unlink_test to the test whitelist
user/apps/tests/syscall/gvisor/blocklists/unlink_test Creates blocklist for unsupported unlink_test cases requiring fchmod
kernel/src/syscall/user_access.rs Removes unnecessary blank line
kernel/src/filesystem/vfs/vcore.rs Fixes parent directory lookup logic and adds path validation for do_remove_dir and do_unlink_at
kernel/src/filesystem/vfs/open.rs Fixes parent directory lookup logic in do_sys_openat2 and removes unused variable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

inode_begin.clone()
} else {
inode_begin
.lookup_follow_symlink(parent_path.unwrap_or("/"), VFS_MAX_FOLLOW_SYMLINK_TIMES)?
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unwrap_or(\"/\") fallback on line 249 is unreachable since the else branch is only taken when parent_path.is_some(). Consider using unwrap() instead to make the code's intent clearer, or restructure the logic to avoid the unnecessary fallback.

Suggested change
.lookup_follow_symlink(parent_path.unwrap_or("/"), VFS_MAX_FOLLOW_SYMLINK_TIMES)?
.lookup_follow_symlink(parent_path.unwrap(), VFS_MAX_FOLLOW_SYMLINK_TIMES)?

Copilot uses AI. Check for mistakes.
inode_begin.clone()
} else {
inode_begin
.lookup_follow_symlink(parent_path.unwrap_or("/"), VFS_MAX_FOLLOW_SYMLINK_TIMES)?
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unwrap_or(\"/\") fallback on line 286 is unreachable since the else branch is only taken when parent_path.is_some(). Consider using unwrap() instead to make the code's intent clearer, or restructure the logic to avoid the unnecessary fallback.

Suggested change
.lookup_follow_symlink(parent_path.unwrap_or("/"), VFS_MAX_FOLLOW_SYMLINK_TIMES)?
.lookup_follow_symlink(parent_path.unwrap(), VFS_MAX_FOLLOW_SYMLINK_TIMES)?

Copilot uses AI. Check for mistakes.
inode_begin.clone()
} else {
inode_begin.lookup_follow_symlink(
parent_path.unwrap_or("/"),
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unwrap_or(\"/\") fallback on line 190 is unreachable since the else branch is only taken when parent_path.is_some(). Consider using unwrap() instead to make the code's intent clearer, or restructure the logic to avoid the unnecessary fallback.

Suggested change
parent_path.unwrap_or("/"),
parent_path.unwrap(),

Copilot uses AI. Check for mistakes.
Signed-off-by: longjin <longjin@DragonOS.org>
@fslongjin fslongjin merged commit 678cad5 into DragonOS-Community:master Nov 10, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug fix A bug is fixed in this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants