rm:fix safe traversal/access #9577

mattsu2020 · 2025-12-06T03:43:45Z

Modify path control to enable secure access

related
#9542

codspeed-hq · 2025-12-06T03:56:59Z

CodSpeed Performance Report

Merging #9577 will improve performances by 14.61%

_{Comparing mattsu2020:rm_fix_full_path (a647f9c) with main (5b261bc)}

Summary

⚡ 1 improvement
✅ 126 untouched
⏩ 6 skipped¹

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`rm_recursive_tree`	13.6 ms	11.9 ms	+14.61%

6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

github-actions · 2025-12-06T03:59:05Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

collinfunk · 2025-12-06T04:07:09Z

Similar to #9554, it hits file descriptor limits:

$ mkdir -p `python3 -c 'print("a/" * 1024 * 16)'`
$ ./target/debug/rm -rf a
rm: cannot remove '[long file name snipped]': Directory not empty
$ strace ./target/debug/rm -rf a
[...]
newfstatat(1022, "a", {st_mode=S_IFDIR|0755, st_size=2, ...}, AT_SYMLINK_NOFOLLOW) = 0
openat(1022, "a", O_RDONLY|O_CLOEXEC|O_DIRECTORY) = 1023
dup(1023)                               = -1 EMFILE (Too many open files)
unlinkat(1022, "a", AT_REMOVEDIR)       = -1 ENOTEMPTY (Directory not empty)
[...]

mattsu2020 · 2025-12-06T04:36:44Z

Similar to #9554, it hits file descriptor limits:

$ mkdir -p `python3 -c 'print("a/" * 1024 * 16)'`
$ ./target/debug/rm -rf a
rm: cannot remove '[long file name snipped]': Directory not empty
$ strace ./target/debug/rm -rf a
[...]
newfstatat(1022, "a", {st_mode=S_IFDIR|0755, st_size=2, ...}, AT_SYMLINK_NOFOLLOW) = 0
openat(1022, "a", O_RDONLY|O_CLOEXEC|O_DIRECTORY) = 1023
dup(1023)                               = -1 EMFILE (Too many open files)
unlinkat(1022, "a", AT_REMOVEDIR)       = -1 ENOTEMPTY (Directory not empty)
[...]

We need to make a fix to prevent the file from opening continuously.

github-actions · 2025-12-06T05:02:12Z

GNU testsuite comparison:

GNU test failed: tests/du/long-from-unreadable. tests/du/long-from-unreadable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/many-dir-entries-vs-OOM. tests/rm/many-dir-entries-vs-OOM is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)

github-actions · 2025-12-06T05:26:39Z

GNU testsuite comparison:

GNU test failed: tests/du/long-from-unreadable. tests/du/long-from-unreadable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/many-dir-entries-vs-OOM. tests/rm/many-dir-entries-vs-OOM is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

github-actions · 2025-12-06T05:46:07Z

GNU testsuite comparison:

GNU test failed: tests/du/long-from-unreadable. tests/du/long-from-unreadable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/many-dir-entries-vs-OOM. tests/rm/many-dir-entries-vs-OOM is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)

github-actions · 2025-12-06T06:58:03Z

GNU testsuite comparison:

GNU test failed: tests/du/long-from-unreadable. tests/du/long-from-unreadable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/many-dir-entries-vs-OOM. tests/rm/many-dir-entries-vs-OOM is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

github-actions · 2025-12-06T07:26:49Z

GNU testsuite comparison:

GNU test failed: tests/du/long-from-unreadable. tests/du/long-from-unreadable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/many-dir-entries-vs-OOM. tests/rm/many-dir-entries-vs-OOM is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

sylvestre · 2025-12-06T08:55:15Z

i am confused, why do you create a PR with 24 commits ?
why not waiting for this to be ok before creating the PR ?
thanks

mattsu2020 · 2025-12-06T09:35:28Z

i am confused, why do you create a PR with 24 commits ? why not waiting for this to be ok before creating the PR ? thanks

I attempted to address the file descriptor limitations, but it seems significant modifications are required.
After making the necessary changes as a draft, I will request another review.

github-actions · 2025-12-06T10:01:54Z

GNU testsuite comparison:

GNU test failed: tests/du/long-from-unreadable. tests/du/long-from-unreadable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/many-dir-entries-vs-OOM. tests/rm/many-dir-entries-vs-OOM is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/rm/rm1 (passes in this run but fails in the 'main' branch)

* feat(chmod): use dirfd for recursive subdirectory traversal - Update chmod recursive logic to use directory file descriptors instead of full paths for subdirectories - Improves performance, avoids path length issues, and ensures dirfd-relative openat calls - Add test to verify strace output shows no AT_FDCWD with multi-component paths * test(chmod): add spell-check ignore for dirfd, subdirs, openat, FDCWD Added a spell-checker ignore directive in the chmod test file to suppress false positives for legitimate technical terms used in Unix API calls. * test(chmod): enforce strace requirement in recursive test, fail fast instead of skip Previously, the test_chmod_recursive_uses_dirfd_for_subdirs test skipped gracefully if strace was unavailable, without failing. This change enforces the strace dependency by failing the test immediately if strace is not installed or runnable, ensuring the test runs reliably in environments where it is expected to pass, and preventing silent skips. * ci: install strace in Ubuntu CI jobs for debugging system calls Add installation of strace tool on Ubuntu runners in both individual build/test and feature build/test jobs. This enables tracing system calls during execution, aiding in debugging and performance analysis within the CI/CD pipeline. Updated existing apt-get commands and added conditional steps for Linux-only installations. * ci: Add strace installation to Ubuntu-based CI workflows Install strace on ubuntu-latest runners across multiple jobs to enable system call tracing for testing purposes, ensuring compatibility with tests that require this debugging tool. This includes updating package lists in existing installation steps. * chore(build): install strace and prevent apt prompts in Cross.toml pre-build Modified the pre-build command to install strace utility for debugging and added -y flag to apt-get install to skip prompts, ensuring non-interactive builds. * feat(build): support Alpine-based cross images in pre-build Detect package manager (apt vs apk) to install tzdata and strace in both Debian/Ubuntu and Alpine *-musl targets. Added fallback warning for unsupported managers. This ensures strace is available for targets using Alpine, which doesn't have apt-get. * refactor(build): improve pre-build script readability by using multi-line strings Replace escaped multi-line string with triple-quoted string for better readability in Cross.toml. * feat(ci): install strace in WSL2 GitHub Actions workflow Install strace utility in the WSL2 environment to support tracing system calls during testing. Minor update to Cross.toml spell-checker ignore list for consistency with change. * ci(wsl2): install strace as root with non-interactive apt-get Updated the WSL2 workflow step to use root shell (wsl-bash-root) for installing strace, removing sudo calls and adding DEBIAN_FRONTEND=noninteractive to prevent prompts. This improves CI reliability by ensuring direct root access and automated, interrupt-free package installation. * ci: Move strace installation to user shell and update spell ignore Fix WSL2 GitHub Actions workflow by installing strace as the user instead of root for better permission handling, and add "noninteractive" to the spell-checker ignore comment for consistency with the new apt-get command. This ensures the tool is available in the testing environment without unnecessary privilege escalation. * chore: ci: remove unused strace installation from CI workflows Remove strace package installation from multiple GitHub Actions workflow files (CICD.yml, l10n.yml, wsl2.yml). Strace was historically installed in Ubuntu jobs for debugging system calls, but it's no longer required for the tests and builds, reducing CI setup time and dependencies. * ci: add strace installation and fix spell-checker comments in CI files - Install strace package in CICD workflow to support safe traversal verification for utilities like rm, chmod, chown, chgrp, mv, and du, enabling syscall tracing for testing. - Clean up spell-checker ignore comments in wsl2.yml and Cross.toml by removing misplaced flags.第二个测试产品**ci: add strace installation and fix spell-checker comments in CI files** - Install strace package in CICD workflow to support safe traversal verification for utilities like rm, chmod, chown, chgrp, mv, and du, enabling syscall tracing for testing. - Clean up spell-checker ignore comments in wsl2.yml and Cross.toml by removing misplaced flags. * test: add regression guard for recursive chmod dirfd-relative traversal Add a check in check-safe-traversal.sh to ensure recursive chmod operations use dirfd-relative openat calls instead of AT_FDCWD with multi-component paths, preventing potential race conditions. Ignore the corresponding Rust test as it is now covered by this shell script guard.

seq: adding large integers benchmarks

- `pseudo` is a tool which simulates being root by intercepting calls to e.g. `geteuid` and `chown` (by using the `LD_PRELOAD` mechanism). This is used e.g. to build filesystems for embedded devices without running as root on the build machine. - the `chown` call getting removed in this commit does not work when running with `pseudo` and using `PSEUDO_IGNORE_PATHS`: in this case, the call to `geteuid()` gets intercepted by `libpseudo.so` and returns 0, however the call to `chown()` isn't intercepted by `libpseudo.so` in case it is in a path from `PSEUDO_IGNORE_PATHS`, and will thus fail since the process is not really root - the call to `chown()` was added in uutils#5735 with the intent of making the test `install-C-root.sh` pass, however it isn't required (GNU coreutils also does not call `chown` just because `install` was called as root) Fixes uutils#9116 Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com>

* du: handle --files0-from=- with piped in '-' * build-gnu.sh: remove incorrect string replacement in tests/du/files0-from.pl --------- Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>

This change adds inline functions for checking file modes and refactors prompt functions to accept pre-fetched stat data. It modifies safe_remove_* functions to handle paths without parents and updates safe_remove_dir_recursive to fetch and reuse initial mode. This reduces redundant statx system calls, improving performance during recursive removals.

- Add specific prompts for symlinks and empty files in 'Always' mode - Refactor matching logic for better clarity and to match GNU rm behavior - Improve handling of write-protected and non-terminal stdin scenarios This enhances the user experience by providing more accurate and targeted confirmations during file removal on Linux.

Refactored multiple call sites of the prompt_yes! macro in linux.rs to use consistent multi-line formatting, enhancing code readability and adhering to style guidelines without altering functionality. Adjusted import ordering slightly for better organization.

…t from std::io Removed the unused 'self' import from the std::io module to clean up the code and avoid potential confusion, as it was not referenced anywhere in the file. This is a minor refactoring for better maintainability.

Add "statx" (a Linux system call name) and "behaviour" (potential spelling variant) to the spell-checker ignore comment in the rm utility's Linux platform code, preventing false positives in linting.

…nteractive::Always mode Refactor the prompt_file_with_stat function in src/uu/rm/src/platform/linux.rs to fix inconsistent prompting for Interactive::Always. Previously, it always used non-protected wording regardless of file writability. Now, it checks if the file is writable and uses appropriate messaging (simple for writable, protected for non-writable). The match logic for Interactive::Once and PromptProtected is also simplified using a triple condition for better readability and to ensure empty vs non-empty protected files are distinguished correctly, matching expected rm behavior.

Reformatted the prompt_yes! macro call across multiple lines to improve code readability and adhere to line length conventions. No functional changes.

github-actions · 2025-12-06T10:36:59Z

GNU testsuite comparison:

Congrats! The gnu test tests/du/files0-from is no longer failing!

mattsu2020 · 2025-12-06T10:55:27Z

The file descriptor issue will be addressed in a separate pull request, so I've reverted it.

ChrisDryden · 2025-12-24T17:05:51Z

src/uu/rm/src/platform/linux.rs

+        (false, _, _, InteractiveMode::PromptProtected) => true,
+        (false, false, false, InteractiveMode::Never) => true,
+        (_, false, false, _) => prompt_yes!(
+            "attempt removal of inaccessible directory {}?",


Do we need translations for these?

but it can be done in a different pr :)

mattsu2020 changed the title rm rm:fix safe traversal/access Dec 6, 2025

mattsu2020 marked this pull request as draft December 6, 2025 09:30

mattsu2020 and others added 11 commits December 6, 2025 19:10

Merge pull request uutils#9561 from ChrisDryden/seq_benches

5abbbe6

seq: adding large integers benchmarks

du: handle --files0-from=- with piped in - (uutils#8985)

61e760b

* du: handle --files0-from=- with piped in '-' * build-gnu.sh: remove incorrect string replacement in tests/du/files0-from.pl --------- Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>

chore(spell-checker): update ignore list to include statx and behaviour

7bd092f

Add "statx" (a Linux system call name) and "behaviour" (potential spelling variant) to the spell-checker ignore comment in the rm utility's Linux platform code, preventing false positives in linting.

style(rm): wrap long line in prompt_file_with_stat macro for readability

c08c802

Reformatted the prompt_yes! macro call across multiple lines to improve code readability and adhere to line length conventions. No functional changes.

mattsu2020 force-pushed the rm_fix_full_path branch from 95a11ab to c08c802 Compare December 6, 2025 10:12

Merge branch 'main' into rm_fix_full_path

a647f9c

mattsu2020 marked this pull request as ready for review December 6, 2025 10:55

ChrisDryden reviewed Dec 24, 2025

View reviewed changes

sylvestre merged commit 3edf14e into uutils:main Dec 24, 2025
129 checks passed

sylvestre mentioned this pull request Dec 24, 2025

rm/linux.rs: move some hardcoded strings as translatable #9824

Open

Uh oh!

rm:fix safe traversal/access #9577

rm:fix safe traversal/access #9577

Conversation

mattsu2020 commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq bot commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #9577 will improve performances by 14.61%

Summary

Benchmarks breakdown

Footnotes

Uh oh!

github-actions bot commented Dec 6, 2025

Uh oh!

collinfunk commented Dec 6, 2025

Uh oh!

mattsu2020 commented Dec 6, 2025

Uh oh!

github-actions bot commented Dec 6, 2025

Uh oh!

github-actions bot commented Dec 6, 2025

Uh oh!

github-actions bot commented Dec 6, 2025

Uh oh!

github-actions bot commented Dec 6, 2025

Uh oh!

github-actions bot commented Dec 6, 2025

Uh oh!

sylvestre commented Dec 6, 2025

Uh oh!

mattsu2020 commented Dec 6, 2025

Uh oh!

github-actions bot commented Dec 6, 2025

Uh oh!

github-actions bot commented Dec 6, 2025

Uh oh!

mattsu2020 commented Dec 6, 2025

Uh oh!

ChrisDryden Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

sylvestre Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

sylvestre Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mattsu2020 commented Dec 6, 2025 •

edited

Loading

codspeed-hq bot commented Dec 6, 2025 •

edited

Loading