With iflag=count_bytes or iflag=skip_bytes, a short read from a non-seekable input could stop the copy or skip early, so fewer bytes than requested were processed. Meet the byte budget even when individual reads are short, and diagnose partial reads in these modes when bs= is used without iflag=fullblock (including when only a byte remnant remains).
Keep a single skip() read/error path; only progress accounting differs between record-oriented and byte-oriented modes. The same scanargs split feeds both skip_bytes and seek_bytes call sites. * src/dd.c (scanargs): Enable warn_partial_read for skip_bytes/max_bytes. (skip): Byte-oriented skip/seek accumulates discarded input. (dd_copy): Track remaining bytes for count_bytes; debit conv=noerror,sync substitutions with need. * tests/dd/count-bytes-pipe.sh: LD_PRELOAD short-read repro, fifo cases, skip EOF remnant, seek_bytes, and probed strace EIO. * tests/local.mk: Reference the new test. * NEWS: Mention the fix. Signed-off-by: Iván Ezequiel Rodriguez <[email protected]> --- NEWS | 8 ++ src/dd.c | 105 ++++++++++++--- tests/dd/count-bytes-pipe.sh | 254 +++++++++++++++++++++++++++++++++++ tests/local.mk | 1 + 4 files changed, 352 insertions(+), 16 deletions(-) create mode 100755 tests/dd/count-bytes-pipe.sh diff --git a/NEWS b/NEWS index cacd907b0..72dd8b794 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,14 @@ GNU coreutils NEWS -*- outline -*- ** Bug fixes + dd iflag=count_bytes and iflag=skip_bytes now honor the requested byte + count when reading from non-seekable inputs that return short reads + (for example pipes). Previously a short read could stop the copy or + skip early, so fewer bytes than requested were processed. dd also + diagnoses partial reads in these byte-oriented modes when bs= is used + without iflag=fullblock, matching the existing advice for block counts. + [bug introduced with count_bytes and skip_bytes in coreutils-8.16] + 'basenc --base32hex' no longer decodes BASE32 only characters "WXYZ". [bug introduced with the basenc program in coreutils-8.31] diff --git a/src/dd.c b/src/dd.c index 26382a233..f477e1bf1 100644 --- a/src/dd.c +++ b/src/dd.c @@ -1664,13 +1664,15 @@ scanargs (int argc, char *const *argv) seek_records = seek; /* Warn about partial reads if bs=SIZE is given and iflag=fullblock - is not, and if counting or skipping bytes or using direct I/O. + is not, and if counting or skipping (including byte-oriented + count_bytes/skip_bytes) or using direct I/O. This helps to avoid confusion with miscounts, and to avoid issues with direct I/O on GNU/Linux. */ warn_partial_read = (! (conversions_mask & C_TWOBUFS) && ! (input_flags & O_FULLBLOCK) - && (skip_records + && (skip_records || skip_bytes || (0 < max_records && max_records < INTMAX_MAX) + || max_bytes || (input_flags | output_flags) & O_DIRECT)); iread_fnc = ((input_flags & O_FULLBLOCK) @@ -1808,7 +1810,7 @@ advance_input_offset (intmax_t offset) static intmax_t skip (int fdesc, char const *file, intmax_t records, idx_t blocksize, - idx_t *bytes) + idx_t *bytes, bool byte_count) { /* Try lseek and if an error indicates it was an inappropriate operation -- or if the file offset is not representable as an off_t -- @@ -1894,9 +1896,36 @@ skip (int fdesc, char const *file, intmax_t records, idx_t blocksize, buf = obuf; } + /* Byte-oriented skip/seek: meet a byte budget despite short reads. + Record-oriented skip: a short read still consumes one record. + Both share the same read/error path; only progress accounting + differs. + + Precondition for byte_count: scanargs split the operand with the + same formula for iflag=skip_bytes and oflag=seek_bytes + (records = N / blocksize, *bytes = N % blocksize). Call sites + pass input_flags&O_SKIP_BYTES or output_flags&O_SEEK_BYTES + accordingly. Reconstituting N therefore cannot overflow + intmax_t when the original operand was accepted. */ + intmax_t bytes_left = 0; + if (byte_count + && (ckd_mul (&bytes_left, records, blocksize) + || ckd_add (&bytes_left, bytes_left, *bytes))) + { + diagnose (EOVERFLOW, + gettext (fdesc == STDIN_FILENO + ? N_("%s: cannot skip") + : N_("%s: cannot seek")), + quotef (file)); + quit (EXIT_FAILURE); + } + do { - ssize_t nread = iread_fnc (fdesc, buf, records ? blocksize : *bytes); + idx_t need = byte_count + ? (bytes_left < blocksize ? bytes_left : blocksize) + : (records ? blocksize : *bytes); + ssize_t nread = iread_fnc (fdesc, buf, need); if (nread < 0) { if (fdesc == STDIN_FILENO) @@ -1906,20 +1935,37 @@ skip (int fdesc, char const *file, intmax_t records, idx_t blocksize, print_stats (); } else + /* Output is not readable; report the earlier lseek + failure (same as before this change). */ diagnose (lseek_errno, _("%s: cannot seek"), quotef (file)); quit (EXIT_FAILURE); } else if (nread == 0) - break; + { + if (byte_count) + { + records = bytes_left / blocksize; + *bytes = bytes_left % blocksize; + } + break; + } else if (fdesc == STDIN_FILENO) advance_input_offset (nread); - if (records != 0) + if (byte_count) + bytes_left -= nread; + else if (records != 0) records--; else *bytes = 0; } - while (records || *bytes); + while (byte_count ? 0 < bytes_left : (records || *bytes)); + + if (byte_count && bytes_left == 0) + { + records = 0; + *bytes = 0; + } return records; } @@ -2127,7 +2173,8 @@ dd_copy (void) || ckd_add (&us_bytes, skip_bytes, us_bytes)); off_t input_offset0 = input_offset; intmax_t us_blocks = skip (STDIN_FILENO, input_file, - skip_records, input_blocksize, &skip_bytes); + skip_records, input_blocksize, &skip_bytes, + input_flags & O_SKIP_BYTES); /* POSIX doesn't say what to do when dd detects it has been asked to skip past EOF, so I assume it's non-fatal. @@ -2150,7 +2197,8 @@ dd_copy (void) { idx_t bytes = seek_bytes; intmax_t write_records = skip (STDOUT_FILENO, output_file, - seek_records, output_blocksize, &bytes); + seek_records, output_blocksize, &bytes, + output_flags & O_SEEK_BYTES); if (write_records != 0 || bytes != 0) { @@ -2181,6 +2229,16 @@ dd_copy (void) alloc_obuf (); int saved_byte = -1; + /* With iflag=count_bytes (or count=NB), meet the requested byte count + even when individual reads are short. Traditional record-oriented + counting is unchanged when counting blocks. */ + bool count_bytes = input_flags & O_COUNT_BYTES; + intmax_t bytes_remaining = 0; + if (count_bytes + && (ckd_mul (&bytes_remaining, max_records, input_blocksize) + || ckd_add (&bytes_remaining, bytes_remaining, max_bytes))) + error (EXIT_FAILURE, 0, "%s: %s", _("invalid number"), quote ("count")); + while (true) { if (status_level == STATUS_PROGRESS) @@ -2193,7 +2251,9 @@ dd_copy (void) } } - if (r_partial + r_full >= max_records + !!max_bytes) + if (count_bytes + ? bytes_remaining <= 0 + : r_partial + r_full >= max_records + !!max_bytes) break; /* Zero the buffer before reading, so that if we get a read error, @@ -2204,14 +2264,22 @@ dd_copy (void) (conversions_mask & (C_BLOCK | C_UNBLOCK)) ? ' ' : '\0', input_blocksize); - if (r_partial + r_full >= max_records) - nread = iread_fnc (STDIN_FILENO, ibuf, max_bytes); + idx_t need; + if (count_bytes) + need = bytes_remaining < input_blocksize + ? bytes_remaining : input_blocksize; + else if (r_partial + r_full >= max_records) + need = max_bytes; else - nread = iread_fnc (STDIN_FILENO, ibuf, input_blocksize); + need = input_blocksize; + + nread = iread_fnc (STDIN_FILENO, ibuf, need); if (nread > 0) { advance_input_offset (nread); + if (count_bytes) + bytes_remaining -= nread; if (i_nocache) invalidate_cache (STDIN_FILENO, nread); } @@ -2245,9 +2313,14 @@ dd_copy (void) input_seek_errno = ESPIPE; } if ((conversions_mask & C_SYNC) && !partread) - /* Replace the missing input with null bytes and - proceed normally. */ - nread = 0; + { + /* Replace the missing input with null bytes and + proceed normally. 'need' is already capped by + bytes_remaining when count_bytes is set. */ + nread = 0; + if (count_bytes) + bytes_remaining -= need; + } else continue; } diff --git a/tests/dd/count-bytes-pipe.sh b/tests/dd/count-bytes-pipe.sh new file mode 100755 index 000000000..851e20265 --- /dev/null +++ b/tests/dd/count-bytes-pipe.sh @@ -0,0 +1,254 @@ +#!/bin/sh +# count_bytes/skip_bytes must honor the byte count on short reads. + +# Copyright (C) 2026 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <https://www.gnu.org/licenses/>. + +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ dd + +python3 -c 'import sys' || skip_ 'python3 required' +command -v timeout >/dev/null || skip_ 'timeout required' + +# retry_delay_ uses awk for the delay; keep '.' as the decimal point. +LC_ALL=C; export LC_ALL + +DD_TIMEOUT=5 + +mkfifo_or_skip_ dd.fifo + +cleanup_fifo_writer() { + test -n "$1" || return 0 + kill "$1" 2>/dev/null + wait "$1" 2>/dev/null +} + +# Write exactly LEN zero bytes in CHUNK-sized writes; DELAY may be 0. +# 'local' matches tests/dd/reblock.sh; the suite runs under bash. +feed_chunks() { + python3 - "$@" <<'PY' +import os, sys, time +length, chunk = int(sys.argv[1]), int(sys.argv[2]) +delay = float(sys.argv[3].replace(',', '.') or '0') +fd = sys.stdout.fileno() +done = 0 +block = b'\0' * chunk +try: + while done < length: + n = min(chunk, length - done) + os.write(fd, block if n == chunk else b'\0' * n) + done += n + if delay: + time.sleep(delay) +except BrokenPipeError: + pass +PY +} + +fifo_dd() { + local len="$1" chunk="$2" delay="$3" + shift 3 + feed_chunks "$len" "$chunk" "$delay" > dd.fifo & + local wpid=$! + timeout "$DD_TIMEOUT" dd "$@" if=dd.fifo 2>err + local st=$? + cleanup_fifo_writer "$wpid" + test "$st" -eq 0 || return 1 +} + +# Deterministic short reads: LD_PRELOAD caps each read() at 50 bytes. +# This reproduces the historical under-copy (one short read → stop) without +# relying on pipe coalescing. require_gcc_shared_ skips the whole file if +# shared libs cannot be built; keep that case first so the bug is covered. +require_gcc_shared_ + +cat > shortread.c <<'EOF' || framework_failure_ +#define _GNU_SOURCE +#include <dlfcn.h> +#include <stdio.h> +#include <unistd.h> + +enum { CAP = 50 }; + +ssize_t +read (int fd, void *buf, size_t count) +{ + static ssize_t (*real_read) (int, void *, size_t); + if (!real_read) + real_read = dlsym (RTLD_NEXT, "read"); + if (count > CAP) + { + FILE *evidence; + count = CAP; + /* Evidence that the interceptor ran. */ + evidence = fopen ("shortread.ok", "w"); + if (evidence) + fclose (evidence); + } + return real_read (fd, buf, count); +} +EOF + +gcc_shared_ shortread.c shortread.so \ + || framework_failure_ 'failed to build shortread.so' + +rm -f shortread.ok out.sr +LD_PRELOAD=$LD_PRELOAD:./shortread.so \ + timeout "$DD_TIMEOUT" dd bs=1024 iflag=count_bytes count=200 \ + if=/dev/zero of=out.sr status=none 2>err.sr || fail=1 +test -f shortread.ok || skip_ 'LD_PRELOAD short-read interceptor ineffective' +test "$(wc -c < out.sr)" -eq 200 || fail=1 + +# Optional smoking gun: an unpatched dd under-copies with the same cap. +# Use a path that is not our just-built binary. +sysdd= +for cand in /usr/bin/dd /bin/dd; do + if test -x "$cand" && "$cand" --version >/dev/null 2>&1; then + sysdd=$cand + break + fi +done +if test -n "$sysdd"; then + rm -f out.sys + LD_PRELOAD=$LD_PRELOAD:./shortread.so \ + timeout "$DD_TIMEOUT" "$sysdd" bs=1024 iflag=count_bytes count=200 \ + if=/dev/zero of=out.sys status=none 2>/dev/null || true + # If the system dd still has the bug, it copies only CAP bytes. + # Do not fail the suite if the distro already shipped a fix. + if test -f out.sys && test "$(wc -c < out.sys)" -eq 50; then + : + fi +fi + +# Fifo result checks (byte count; reads may coalesce). +count_lt_bs() { + local delay="$1" + fifo_dd 200 50 "$delay" bs=1024 iflag=count_bytes count=200 \ + of=out status=none || return 1 + test "$(wc -c < out)" -eq 200 || return 1 +} +retry_delay_ count_lt_bs .05 8 || fail=1 + +count_span() { + local delay="$1" + fifo_dd 250 40 "$delay" bs=100 iflag=count_bytes count=250 \ + of=out status=none || return 1 + test "$(wc -c < out)" -eq 250 || return 1 +} +retry_delay_ count_span .05 8 || fail=1 + +count_eof() { + local delay="$1" + fifo_dd 80 20 "$delay" bs=1024 iflag=count_bytes count=200 \ + of=out status=none || return 1 + test "$(wc -c < out)" -eq 80 || return 1 +} +retry_delay_ count_eof .05 8 || fail=1 + +# Record-oriented count on a seekable short file (deterministic). +dd if=/dev/zero of=short.in bs=150 count=1 status=none || framework_failure_ +timeout "$DD_TIMEOUT" dd bs=100 count=2 if=short.in of=out status=none || fail=1 +test "$(wc -c < out)" -eq 150 || fail=1 + +skip_lt_bs() { + local delay="$1" + { + feed_chunks 10 3 "$delay" || exit 1 + printf 'abcdefghijklm' + } > dd.fifo & + local wpid=$! + timeout "$DD_TIMEOUT" dd bs=1024 skip=10 iflag=skip_bytes \ + if=dd.fifo of=out status=none 2>err + local st=$? + cleanup_fifo_writer "$wpid" + test "$st" -eq 0 || return 1 + test "$(cat out)" = 'abcdefghijklm' || return 1 +} +retry_delay_ skip_lt_bs .05 8 || fail=1 + +# skip_bytes past EOF; remnant not a multiple of bs (250 = 2*100+50). +skip_eof_remnant() { + local delay="$1" + feed_chunks 80 20 "$delay" > dd.fifo & + local wpid=$! + timeout "$DD_TIMEOUT" dd bs=100 skip=250 iflag=skip_bytes \ + if=dd.fifo of=out 2>err + local st=$? + cleanup_fifo_writer "$wpid" + test "$st" -eq 0 || return 1 + test "$(wc -c < out)" -eq 0 || return 1 + grep -F 'cannot skip to specified offset' err || return 1 +} +retry_delay_ skip_eof_remnant .05 8 || fail=1 + +fullblock_count() { + local delay="$1" + fifo_dd 200 50 "$delay" bs=1024 iflag=count_bytes,fullblock count=200 \ + of=out status=none || return 1 + test "$(wc -c < out)" -eq 200 || return 1 +} +retry_delay_ fullblock_count .05 8 || fail=1 + +warn_count() { + local delay="$1" + fifo_dd 250 40 "$delay" bs=100 iflag=count_bytes count=250 of=out || return 1 + test "$(wc -c < out)" -eq 250 || return 1 + grep -F 'suggest iflag=fullblock' err || return 1 +} +retry_delay_ warn_count .05 8 || fail=1 + +# seek_bytes: same scanargs split as skip_bytes (seekable of=). +printf 'MARK' > payload || framework_failure_ +timeout "$DD_TIMEOUT" dd bs=1024 seek=1500 oflag=seek_bytes \ + if=payload of=seek.out status=none 2>err || fail=1 +test "$(wc -c < seek.out)" -eq 1504 || fail=1 +test "$(tail -c 4 seek.out)" = 'MARK' || fail=1 + +# conv=noerror,sync + count_bytes: debit need so EIO cannot loop forever. +# Policy: +# - strace missing or fault unsupported → skip this case (not fail). +# - fault confirmed via trace/stderr EIO, but result wrong → fail. +# - unexpected infrastructure error → framework_failure_ with reason. +if command -v strace >/dev/null; then + dd if=/dev/zero of=ifile bs=1000 count=1 status=none || framework_failure_ + rm -f eio.probe eio.trace eio.probe.err + timeout "$DD_TIMEOUT" strace -o eio.trace --quiet=all -P ifile \ + -e fault=read:error=EIO \ + dd if=ifile of=eio.probe bs=100 count=1 status=none 2>eio.probe.err + probe_st=$? + if test "$probe_st" -eq 124; then + framework_failure_ 'strace EIO probe timed out' + elif test "$probe_st" -eq 0; then + # Fault did not affect the read; skip the noerror case. + : + elif grep -E 'EIO|Input/output error' eio.trace eio.probe.err >/dev/null + then + rm -f eio.out + timeout "$DD_TIMEOUT" strace -o /dev/null --quiet=all -P ifile \ + -e fault=read:error=EIO \ + dd if=ifile of=eio.out bs=100 iflag=count_bytes count=250 \ + conv=noerror,sync status=none 2>eio.err || fail=1 + test "$(wc -c < eio.out)" -eq 300 || fail=1 + else + framework_failure_ \ + 'strace probe failed without EIO evidence (check strace -e fault=read)' + fi +fi + +timeout "$DD_TIMEOUT" dd bs=1024 iflag=count_bytes count=200 \ + if=/dev/zero of=out2 status=none || fail=1 +test "$(wc -c < out2)" -eq 200 || fail=1 + +Exit $fail diff --git a/tests/local.mk b/tests/local.mk index 7d75b4b5a..0b67a22ff 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -656,6 +656,7 @@ all_tests = \ tests/dd/skip-seek.pl \ tests/dd/skip-seek2.sh \ tests/dd/bytes.sh \ + tests/dd/count-bytes-pipe.sh \ tests/dd/skip-seek-past-file.sh \ tests/dd/sparse.sh \ tests/dd/stderr.sh \ -- 2.43.0
