Following is the latest version. I should mention that I think the only possibly contentious part of this is the FIXME comment I addressed where a warning is now printed if you skip past EOF, as demonstrated by this command:
$ printf %4s | dd bs=1 skip=5 && \ echo only warning, but mixed with status ./dd: `standard input': cannot skip: Invalid argument 0+0 records in 0+0 records out 0 bytes (0 B) copied, 0.00143536 s, 0.0 kB/s only warning, but mixed with status cheers, Pádraig. >From 3f1d6e479095a0d5f8c52f82af020181a9380305 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Thu, 20 Nov 2008 10:28:31 +0000 Subject: [PATCH] dd: Better handle user specified offsets that are too big Following are the before and after operations for seekable files, for the various erroneous offsets handled by this patch: skip beyond end of file before: immediately exit(0); after : immediately printf("cannot skip: Invalid argument); exit(0); skip > max file size before: read whole file and exit(0); after : immediately printf("cannot skip: Invalid argument); exit(1); seek > max file size before: immediately printf("truncate error: EFBIG"); exit(1); after : immediately printf("truncate error: EFBIG"); exit(1); skip > OFF_T_MAX before: read whole device/file and exit(0); after : immediately printf("cannot skip:"); exit(1); seek > OFF_T_MAX before: immediately printf("truncate error: offset too large"); exit(1); after : immediately printf("truncate error: offset too large"); exit(1); skip > device size before: read whole device and exit(0); after : immediately printf("cannot skip: Invalid argument); exit(1); seek > device size before: read whole device and printf("write error: ENOSPC"); exit(1); after : immediately printf("cannot seek: Invalid argument); exit(1); * NEWS: Summarise this change in behaviour. * src/dd.c (skip): Add error checking for large seek/skip offsets on seekable files, rather than deferring to using read() to advance offset. (dd_copy): Print a warning if skip past EOF, as per FIXME comment. * test/Makefile.am: Add 2 new tests. * tests/dd/seek-skip-past-file: Add tests for first 3 cases above. * tests/dd/seek-skip-past-dev: Add root only test for last case above. --- NEWS | 4 ++ src/dd.c | 55 +++++++++++++++++++++++++++++++++-- tests/Makefile.am | 2 + tests/dd/skip-seek-past-dev | 63 ++++++++++++++++++++++++++++++++++++++++ tests/dd/skip-seek-past-file | 65 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+), 4 deletions(-) create mode 100755 tests/dd/skip-seek-past-dev create mode 100755 tests/dd/skip-seek-past-file diff --git a/NEWS b/NEWS index 83b8ed9..99fc182 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,10 @@ GNU coreutils NEWS -*- outline -*- cp and mv: the --reply={yes,no,query} option has been removed. Using it has elicited a warning for the last three years. + dd: user specified offsets that are too big are handled better. + Previously, erroneous parameters to skip and seek could result + in redundant reading of the file with no warnings or errors. + du: -H (initially equivalent to --si) is now equivalent to --dereference-args, and thus works as POSIX requires diff --git a/src/dd.c b/src/dd.c index d683c5d..30d06df 100644 --- a/src/dd.c +++ b/src/dd.c @@ -1259,12 +1259,56 @@ skip (int fdesc, char const *file, uintmax_t records, size_t blocksize, && 0 <= skip_via_lseek (file, fdesc, offset, SEEK_CUR)) { if (fdesc == STDIN_FILENO) - advance_input_offset (offset); - return 0; + { + struct stat st; + if (fstat (STDIN_FILENO, &st) != 0) + error (EXIT_FAILURE, errno, _("cannot fstat %s"), quote (file)); + if (S_ISREG (st.st_mode) && st.st_size < offset) + records = ( offset - st.st_size + blocksize - 1 ) / blocksize; + else + records = 0; + advance_input_offset (offset); + } + else + records = 0; + return records; } else { int lseek_errno = errno; + off_t soffset; + + /* The seek request may have failed above if it was too big + (> device size, > max file size, etc.) + Or it may not have been done at all (> OFF_T_MAX). + Therefore try to seek to the end of the file, + to avoid redundant reading. */ + if ((soffset = skip_via_lseek (file, fdesc, 0, SEEK_END)) >= 0) + { + /* File is seekable, and we're at the end of it, and + size <= OFF_T_MAX. So there's no point using read to advance. */ + + if (!lseek_errno) + { + /* Orig seek not attempted as offset > OFF_T_MAX + We should error for write as can't get to desired location, + even if OFF_T_MAX < max file size. + For read we're not going to read any data anyway, + so we should error for consistency. + It would be nice to not error for /dev/{zero,null} + for any offset, but that's not significant issue I think. */ + lseek_errno = EOVERFLOW; + } + + if (fdesc == STDIN_FILENO) + error (0, lseek_errno, _("%s: cannot skip"), quote (file)); + else + error (0, lseek_errno, _("%s: cannot seek"), quote (file)); + /* If the file has a specific size and we've asked + to skip/seek beyond the max allowable, then should quit. */ + quit (EXIT_FAILURE); + } + /* else file_size && offset > OFF_T_MAX or file ! seekable */ do { @@ -1537,10 +1581,13 @@ dd_copy (void) if (skip_records != 0) { - skip (STDIN_FILENO, input_file, skip_records, input_blocksize, ibuf); + uintmax_t unskipped = skip (STDIN_FILENO, input_file, + skip_records, input_blocksize, ibuf); /* 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 if the - call to 'skip' returns nonzero. FIXME: maybe give a warning. */ + call to 'skip' returns nonzero. */ + if (unskipped) + error (0, EINVAL, _("%s: cannot skip"), quote (input_file)); } if (seek_records != 0) diff --git a/tests/Makefile.am b/tests/Makefile.am index 6dce9cd..69475ad 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -24,6 +24,7 @@ root_tests = \ cp/cp-a-selinux \ cp/preserve-gid \ cp/special-bits \ + dd/skip-seek-past-dev \ ls/capability \ ls/nameless-uid \ misc/chcon \ @@ -287,6 +288,7 @@ TESTS = \ dd/reblock \ dd/skip-seek \ dd/skip-seek2 \ + dd/skip-seek-past-file \ dd/unblock-sync \ df/total-verify \ du/2g \ diff --git a/tests/dd/skip-seek-past-dev b/tests/dd/skip-seek-past-dev new file mode 100755 index 0000000..906344c --- /dev/null +++ b/tests/dd/skip-seek-past-dev @@ -0,0 +1,63 @@ +#!/bin/sh +# test diagnostics are printed immediately when seeking beyond device. + +# Copyright (C) 2008 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 <http://www.gnu.org/licenses/>. + +if test "$VERBOSE" = yes; then + set -x + dd --version +fi + +. $srcdir/test-lib.sh + +# need write access to device +# (even though we don't actually write anything) +require_root_ + +get_device_size() { + BLOCKDEV=blockdev + $BLOCKDEV -V >/dev/null 2>&1 || BLOCKDEV=/sbin/blockdev + $BLOCKDEV --getsize64 "$1" +} + +fail=0 + +# Get path to device the current dir is on. +# Note df can only get fs size, not device size. +device=$(df -P --local . | tail -n1 | cut -d' ' -f1) || +skip_test_ 'this test runs only on local file systems' + +dev_size=$(get_device_size "$device") || +skip_test_ "Could not determine size of $device" + +# Don't use shell arithimetic as older version of dash use longs +DEV_OFLOW=$(expr $dev_size + 1) + +timeout 1 dd bs=1 skip=$DEV_OFLOW count=0 status=noxfer < "$device" 2> err +test "$?" = "1" || fail=1 +echo "dd: \`standard input': cannot skip: Invalid argument +0+0 records in +0+0 records out" > err_ok || framework_failure +compare err_ok err || fail=1 + +timeout 1 dd bs=1 seek=$DEV_OFLOW count=0 status=noxfer > "$device" 2> err +test "$?" = "1" || fail=1 +echo "dd: \`standard output': cannot seek: Invalid argument +0+0 records in +0+0 records out" > err_ok || framework_failure +compare err_ok err || fail=1 + +Exit $fail diff --git a/tests/dd/skip-seek-past-file b/tests/dd/skip-seek-past-file new file mode 100755 index 0000000..dc02642 --- /dev/null +++ b/tests/dd/skip-seek-past-file @@ -0,0 +1,65 @@ +#!/bin/sh +# test diagnostics are printed when seeking too far in seekable files. + +# Copyright (C) 2008 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 <http://www.gnu.org/licenses/>. + +if test "$VERBOSE" = yes; then + set -x + dd --version +fi + +. $srcdir/test-lib.sh +eval $(getlimits) #for OFF_T limits + +fail=0 + +printf "1234" > file || framework_failure + +# skipping beyond end of file should issue a warning +dd bs=1 skip=5 count=0 status=noxfer < file 2> err || fail=1 +echo "dd: \`standard input': cannot skip: Invalid argument +0+0 records in +0+0 records out" > err_ok || framework_failure +compare err_ok err || fail=1 + +# seeking beyond end of file is OK +dd bs=1 seek=5 count=0 status=noxfer > file 2> err || fail=1 +echo "0+0 records in +0+0 records out" > err_ok || framework_failure +compare err_ok err || fail=1 + +# skipping > OFF_T_MAX should fail immediately +dd bs=1 skip=$OFF_T_OFLOW count=0 status=noxfer < file 2> err && fail=1 +echo "dd: \`standard input': cannot skip: Value too large for defined data type +0+0 records in +0+0 records out" > err_ok || framework_failure +compare err_ok err || fail=1 + +# skipping > max file size should fail immediately +# Note I'm guessing there is a small chance that an lseek() could actually work +# and only a write() would fail (with EFBIG) when offset > max file size. +# So this test will both test for that, and ensure that dd +# exits immediately with an appropriate error when lseek() does error. +if ! truncate --size=$OFF_T_MAX in 2>/dev/null; then + # truncate is to ensure file system doesn't actually support OFF_T_MAX files + dd bs=1 skip=$OFF_T_MAX count=0 status=noxfer < file 2> err && fail=1 + echo "dd: \`standard input': cannot skip: Invalid argument +0+0 records in +0+0 records out" > err_ok || framework_failure + compare err_ok err || fail=1 +fi + +Exit $fail -- 1.5.3.6 _______________________________________________ Bug-coreutils mailing list [email protected] http://lists.gnu.org/mailman/listinfo/bug-coreutils
