Skip to content

dd: derive the count_bytes read budget from bytes read#13435

Open
relative23 wants to merge 3 commits into
uutils:mainfrom
relative23:dd-count-bytes-loop-bsize
Open

dd: derive the count_bytes read budget from bytes read#13435
relative23 wants to merge 3 commits into
uutils:mainfrom
relative23:dd-count-bytes-loop-bsize

Conversation

@relative23

@relative23 relative23 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Fixes #13434.

calc_loop_bsize computed the remaining byte budget for count=N iflag=count_bytes from the bytes written so far, while below_count_limit gates the copy loop on bytes read. A conversion that writes more than it reads (conv=block record expansion, conv=sync padding of partial reads) pushes the written total past N while the read total is still below it, and the u128 subtraction underflowed: a panic in debug builds (attempt to subtract with overflow), and a wrapped huge value in release builds that unlocked a full-sized read on every following iteration — silently overshooting the count limit (the reproducer in #13434 reads 1512 input bytes instead of 1000; GNU reads exactly 1000).

Fix

Derive the remaining budget from rstat.bytes_total (bytes read), matching below_count_limit and the documented semantics of count_bytes, with a saturating subtraction so any future inconsistency degrades to a zero-sized read instead of wrapping. The now-unused WriteStat parameter is dropped.

With the fix, the #13434 reproducer reads exactly 1000 input bytes in both build profiles (like GNU). The remaining output difference vs GNU on that reproducer (92 vs 91 records) is the pre-existing chunk-local conv=block behavior, which occurs without count= as well and is out of scope here (details in the issue).

Testing

  • New unit test for calc_loop_bsize (budget derived from bytes read; exhausted budget yields a zero-sized read; an overshot budget saturates to zero instead of wrapping).
  • New integration test with the deterministic reproducer: panics in debug builds before the fix, passes after, and pins the input side (1+1 records in, i.e. exactly 1000 bytes read) as well as the expected output size.
  • cargo test -p uu_dd (89 tests) and the dd integration suite (123 tests) pass.
  • Outputs are byte-identical to main across non-expanding count configurations (blocks, plain count_bytes, count_bytes with conv=sync on full reads, split ibs/obs).

@relative23
relative23 force-pushed the dd-count-bytes-loop-bsize branch from 4d451be to 136f5a9 Compare July 17, 2026 19:17
calc_loop_bsize computed the remaining byte budget for
count=N iflag=count_bytes from the bytes written so far, while
below_count_limit gates the copy loop on bytes read. A conversion that
writes more than it reads (conv=block record expansion, conv=sync
padding of partial reads) pushes the written total past the limit while
the read total is still below it, and the u128 subtraction underflowed:
a panic in debug builds ('attempt to subtract with overflow' in
calc_loop_bsize), and a wrapped huge value in release builds that
unlocked a full-sized read on every following iteration, silently
overshooting the count limit.

Derive the remaining budget from the bytes read instead, matching
below_count_limit and the documented semantics of count_bytes
(saturating, so an inconsistency degrades to a zero-sized read instead
of wrapping), and drop the now-unused WriteStat parameter.
@relative23
relative23 force-pushed the dd-count-bytes-loop-bsize branch from 136f5a9 to c2c13db Compare July 18, 2026 13:00
@codspeed-hq

codspeed-hq Bot commented Jul 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 339 untouched benchmarks
⏩ 46 skipped benchmarks1


Comparing relative23:dd-count-bytes-loop-bsize (0a07694) with main (7807c8d)

Open in CodSpeed

Footnotes

  1. 46 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

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/pr/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/pid-pipe (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.
Congrats! The gnu test tests/expand/bounded-memory is now passing!
Congrats! The gnu test tests/rm/many-dir-entries-vs-OOM is now passing!

Comment thread src/uu/dd/src/dd.rs Outdated
cmp::min(ideal_bsize as u64, rremain * ibs as u64) as usize
}
Some(Num::Bytes(bmax)) => {
// `count=N iflag=count_bytes` limits the *input*, and

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is a long comment, please make it shorter

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — shortened it (and the matching test comment) to two lines; the full rationale stays in the commit message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dd: count=N iflag=count_bytes underflows when a conversion writes more than N bytes (debug panic, count limit bypassed in release)

2 participants