du, chmod, perms: avoid stack overflow on deep directory traversal by vpashaiev · Pull Request #14887 · uutils/coreutils

vpashaiev · 2026-09-27T10:40:08Z

Replace recursive directory traversal in safe_du (du), safe_traverse_dir (chmod), and safe_traverse_dir (perms, used by chown and chgrp) with an iterative heap-allocated stack.

Problem

When traversing deep directory trees (depth >= 2000), recursive function calls cause a stack overflow and crash with SIGSEGV (signal 11, code 139) because Linux has an 8MB default stack limit (ulimit -s 8192). Each stack frame holds open directory file descriptors (DirFd), Stat/Metadata, and path buffers.

Fix

Replaced recursion in safe_du with an iterative loop using a heap-allocated Vec<DuFrame> stack.
Replaced recursion in safe_traverse_dir (chmod) with an iterative loop using Vec<ChmodFrame>, maintaining cycle detection via ancestors.
Replaced recursion in safe_traverse_dir (perms, powering chown and chgrp) with an iterative loop using Vec<PermsFrame>, maintaining cycle detection via ancestors.
Call stack remains O(1) regardless of directory depth.
Directory descriptors (DirFd) are closed as soon as a directory frame completes and pops from the stack.

Verified on Linux with test directory trees created via openat/mkdirat up to 4000 levels deep:

du -s passes with code 0 (previously crashed with SIGSEGV at depth 2000 and 4000).
chmod -R passes with code 0.
chown -R and chgrp -R pass with code 0.
All existing unit tests pass.

Fixes #10173
Fixes: https://bugs.launchpad.net/ubuntu/+source/rust-coreutils/+bug/2167204

anastygnome · 2026-09-27T11:05:17Z

Please add a test

codspeed · 2026-09-27T11:34:20Z

Merging this PR will degrade performance by 4.57%

2 warnings, 1 note

⚠️ 122 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

ℹ️ 1 benchmark has no flamegraph

The flamegraph wasn't generated: most of the time was spent in runtime internals, which would leave nothing useful to analyze.

See affected benchmarks

⚡ 2 improved benchmarks
❌ 2 regressed benchmarks
✅ 397 untouched benchmarks
⏩ 70 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	Memory	`du_deep_tree[(100, 3)]`	76.5 KB	100.6 KB	-23.97%
❌	CPU Simulation	`mv_single_file`	146.3 ms	151.1 ms	-3.15%
⚡	CPU Simulation	`du_deep_tree[(100, 3)]`	1.9 ms	1.8 ms	+7.87%
⚡	CPU Simulation	`du_summarize_balanced_tree[(5, 4, 10)]`	11.5 ms	11 ms	+4.41%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing vpashaiev:fix-deep-traversal-stack-overflow (2728039) with main (1d6ca65)²}

70 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. ↩
No successful run was found on main (24f90f5) during the generation of this report, so 1d6ca65 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

vpashaiev · 2026-09-27T12:20:10Z

Added regression test test_deep_directory_traversal_no_stack_overflow in tests/by-util/test_du.rs verifying nested directory traversal up to 1500 levels without stack overflow.

Also moved S_IFMT, S_IFDIR, S_IFLNK constants to module level in du.rs to resolve the clippy items-after-statements warning, and formatted changed files via cargo fmt.

vpashaiev · 2026-09-27T15:00:55Z

Adjusted the regression test depth to 400 so it stays safely within the default file descriptor limit (RLIMIT_NOFILE = 1024) on WSL2 runners (depth 1500 hit EMFILE). All tests should pass cleanly now.

sylvestre · 2026-09-27T15:15:13Z

-                } else {
-                    // Report the successful ownership change using the shared helper
-                    self.report_ownership_change_success(&entry_path, meta.uid(), meta.gid());
+                    return;


before, this return only left the current directory. Now it stops the whole traversal. Is this intended?

Not intended. Fixed, now popping the current frame and continuing.

sylvestre · 2026-09-27T15:15:19Z

+                                    *ret = 1;
+                                    if self.verbosity.level != VerbosityLevel::Silent {
+                                        show_error!(
+                                            "cannot read directory {}: {}",


please use translate!() here

Done, switched to translate!().

sylvestre · 2026-09-27T15:15:28Z

+                            top.my_stat.inodes += this_stat.inodes;
+                            top.my_stat.latest_time =
+                                match (top.my_stat.latest_time, this_stat.latest_time) {
+                                    (Some(a), Some(b)) => Some(a.max(b)),


the same size/blocks/inodes/latest_time merge is now there 4 times, could you please move it into a helper on Stat?

Done, added merge() helper on Stat.

sylvestre · 2026-09-27T15:15:35Z

+
+    while let Some(top) = stack.last_mut() {
+        if top.entry_idx < top.entries.len() {
+            let entry_name = top.entries[top.entry_idx].clone();


codspeed shows an 18% regression. Could you please look at it? Cloning every entry name probably doesn't help, std::mem::take would avoid that

Fixed. Converted entries to IntoIter to move them directly without cloning.

sylvestre · 2026-09-27T15:15:44Z

+            } else {
+                // Backtrack so sibling subtrees that legitimately reach the same directory
+                // (e.g. two symlinks to one dir) are not mistaken for cycles.
+                let finished = stack.pop().unwrap();


please avoid the unwrap here (same in perms.rs and du.rs). while let Some(top) = stack.last_mut() already guarantees it

Fixed across chmod, perms, and du.

sylvestre · 2026-09-27T15:15:50Z

 }

+#[cfg(target_os = "linux")]
+struct PermsFrame {


ChmodFrame and PermsFrame are the same struct, and the loop is almost the same too. Could this be a shared walker in uucore safe_traversal?

The frames are similar, but per-entry logic diverges quite a bit between chmod (symlink handling) and perms (preserve-root, filters, bitmask). I can extract a shared generic walker in a follow-up PR to keep this bugfix focused.

sylvestre · 2026-09-27T15:15:58Z

    }

-    Ok(my_stat)
+    unreachable!()


please avoid unreachable!(). Returning at the end of the loop would work

Done. Returning at the end of the loop now.

sylvestre · 2026-09-27T15:16:15Z

+    let mut depth = 0;
+    // Limit depth to 400 to stay safely below the default RLIMIT_NOFILE (1024)
+    // on WSL2 and macOS while testing deep traversal without stack overflow.
+    for _ in 0..400 {


400 levels never overflowed the stack before this fix either, so this test would pass on main.
could you please make it deep enough to crash without the patch (maybe raising RLIMIT_NOFILE in the test)? and please add the same kind of test for chmod -R and chown -R

Alternatively, making the test unix only could be acceptable with 1500, right?

Done. Marked as unix-only, raised RLIMIT_NOFILE to 4096, set depth to 1500, and added tests for both chmod -R and chown -R.

vpashaiev · 2026-09-27T16:36:12Z

Pushed updates addressing the review comments and added tests for chmod/chown.

vpashaiev · 2026-09-30T08:22:30Z

Regarding the CodSpeed report:

Most of the reported regressions are in dd, which this PR does not touch (caused by different runner base environments).

The memory increase in du_deep_tree (+35 KB at depth 100) is expected. This PR moves traversal state from the call stack to a heap vector to avoid stack overflow on deep trees. Runtime speed itself did not regress.

github-actions · 2026-10-02T13:49:17Z

GNU testsuite comparison:

Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)

vpashaiev · 2026-10-19T14:48:15Z

Thanks for the review!

Removed the unused parameters and dead branches in du.
Removed the duplicate localization key in en-US.ftl.
Replaced unwrap_err() with idiomatic match and cleaned up explicit type annotations in perms and chmod.
Deduplicated the test setup across du, chmod, and chown into a common prepare_deep_traversal helper in uutests.
Rebased onto latest main and squashed the branch into a single commit.

Replace recursive directory traversal in safe_du (du), safe_traverse_dir (chmod), and safe_traverse_dir (perms, used by chown and chgrp) with an iterative heap-allocated stack. Previously, traversing deep directory structures (depth >= 2000) exhausted the default 8MB thread stack on Linux due to recursive function calls, causing a segmentation fault (core dumped). Using heap-allocated stacks (DuFrame, ChmodFrame, PermsFrame) keeps the call stack strictly O(1) while preserving exact post-order / pre-order semantics and cycle detection via ancestors. Fixes: uutils#10173 Fixes: https://bugs.launchpad.net/ubuntu/+source/rust-coreutils/+bug/2167204

vpashaiev force-pushed the fix-deep-traversal-stack-overflow branch from d664578 to 0b0b191 Compare September 27, 2026 14:46

sylvestre reviewed Sep 27, 2026

View reviewed changes