fix(dd): ensure full block writes to handle partial writes to slow pipes#8840
Closed
romanstingler wants to merge 2 commits intouutils:mainfrom
Closed
fix(dd): ensure full block writes to handle partial writes to slow pipes#8840romanstingler wants to merge 2 commits intouutils:mainfrom
romanstingler wants to merge 2 commits intouutils:mainfrom
Conversation
|
GNU testsuite comparison: |
RenjiSann
reviewed
Oct 11, 2025
|
GNU testsuite comparison: |
Collaborator
|
The tests you added pass without your patch to |
|
GNU testsuite comparison: |
Contributor
|
both test_buffer_flush_on_input_exhaustion test_single_byte_input_buffer_flush pass without your changes in dd.rs, is that expected ? |
Contributor
|
I stumbled upon this PR while investigating issue #8983. The issue is, as @romanstingler has correctly pointed out, that The A simple way to reproduce this bug on my machine is dd if=/dev/urandom bs=16384 count=1000 | md5sumGNU I have created a test in akretz@3deeb91 which reproduces this failure case and which gets fixed by this PR. It always fails on my machine. Feel free to cherry-pick it! |
Author
|
@akretz hey, first of all thank you very much for picking up that one. @sylvestre maybe you can have a look thanks guys |
|
GNU testsuite comparison: |
This was referenced Nov 3, 2025
Contributor
|
There are some dead code and linting issues. |
Author
maybe your rebase or whatever? if you mean |
|
GNU testsuite comparison: |
Collaborator
error[E0599]: no function or associated item named `from_utf8` found for type `str` in the current scope
--> tests/by-util/test_dd.rs:1813:14
|
1813 | str::from_utf8(&output.stderr)
| ^^^^^^^^^ function or associated item not found in `str`
|
help: you are looking for the module in `std`, not the primitive type
|
1813 | std::str::from_utf8(&output.stderr)
| +++++ |
romanstingler
force-pushed
the
main
branch
2 times, most recently
from
0fee31b to
40c9c41
Compare
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
Author
|
@RenjiSann does it work for you, Ubuntu solved it by cherry picking this MR |
Collaborator
|
Yes, thank you ! |
Collaborator
|
I'll happily merge this as soon as the conflicts are fixed (sorry for not merging this sooner, I wanted to retry the failed jobs and then forgot about it) |
Collaborator
|
Looks good to me, could you just squash the merge commits that were just added ? |
|
GNU testsuite comparison: |
romanstingler
force-pushed
the
main
branch
2 times, most recently
from
a6c925e to
b2c00de
Compare
|
GNU testsuite comparison: |
Contributor
|
still fails |
(cherry picked from commit 4aaf025)
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
(cherry picked from commit 0f7c531)
Contributor
|
The root cause of these partial writes and reads is line-buffered stdout, and this PR does not fix it. The fix also did not work for me when installing makeself packages, as these packages still cannot be installed if the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressed a bug in
ddwhere partial writes to a slow pipe reader resulted in truncated output. Modifiedwrite_blockinOutputstruct to retry writing until the full block is written, ignoring thefullblockflag for output operations. This ensures data integrity even with slow readers, matching GNUddbehavior. Fixes issue observed in tests with mismatched MD5 sums and partial record counts (e.g.,0+1 records out).https://download.virtualbox.org/virtualbox/7.2.2/ -> VBoxGuestAdditions_7.2.2.iso
Partial Write Bug in
uutils ddThe partial write bug in
uutils ddoccurs when writing large blocks to a pipe with a slow reader, resulting in truncated output (as seen with0+1 records outand mismatched MD5 sums). The key area of interest is howddhandles writing data to the output destination.In dd.rs, the write_block method of the Output struct (lines 864-883) is responsible for writing a block of data to the destination.
Issue Identified
The current implementation retries writing if the write operation is interrupted (
io::ErrorKind::Interrupted), which is correct. However, it does not handle the case where a partial write occurs (i.e., wlen < chunk[base_idx..].len()) without being interrupted. When writing to a pipe with a slow reader, the kernel may return a partial write (less than the requested amount) without an error, and the code exits the loop if!self.settings.iflags.fullblockis true. Sinceiflags.fullblockis typically not set for output operations (it's meant for input), the loop exits after the first partial write, leading to truncated output.Root Cause
The condition
if (base_idx >= full_len) || !self.settings.iflags.fullblockmeans that unlessfullblockis set (which it often isn't for output), the function returns after the first write attempt, even if only part of the data was written. This mimics the behavior we observed in tests whereuutils dddoes not retry to write the remaining data, causing the0+1 records outand mismatched byte counts/MD5 sums.Proposed Fix
To fix the partial write issue, we need to ensure that write_block continues to retry writing until the entire block is written or an error occurs, regardless of the
fullblockflag. Thefullblockflag should only apply to input operations, not output. Here's how we can modify the code:!self.settings.iflags.fullblockcondition from the loop exit criteria in write_block.base_idx >= full_lenor a non-interrupted error occurs.This change ensures that
uutils ddmatches the behavior of GNUddin handling partial writes to slow pipes, preventing data truncation.related
https://bugs.launchpad.net/ubuntu/+source/makeself/+bug/2125535
VirtualBox/virtualbox#226 (comment)
megastep/makeself@51e7299