From 460ebb078ed7bf249af745d8725d6c37e7f15b57 Mon Sep 17 00:00:00 2001 From: =?utf8?q?P=C3=A1draig=20Brady?= 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 to specified offset"); 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: Summarize this change in behavior. * 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 | 77 +++++++++++++++++++++++++++++++++---- tests/Makefile.am | 2 + tests/dd/skip-seek-past-dev | 63 ++++++++++++++++++++++++++++++ tests/dd/skip-seek-past-file | 91 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 229 insertions(+), 8 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 83b8ed9a1..99fc182f8 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 d683c5d67..afc51481a 100644 --- a/src/dd.c +++ b/src/dd.c @@ -200,8 +200,7 @@ static bool input_seekable; static int input_seek_errno; /* File offset of the input, in bytes, along with a flag recording - whether it overflowed. The offset is valid only if the input is - seekable and if the offset has not overflowed. */ + whether it overflowed. */ static uintmax_t input_offset; static bool input_offset_overflow; @@ -1259,12 +1258,62 @@ 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 < (input_offset + offset)) + { + /* When skipping past EOF, return the number of _full_ blocks + * that are not skipped, and set offset to EOF, so the caller + * can determine the requested skip was not satisfied. */ + records = ( offset - st.st_size ) / blocksize; + offset = st.st_size - input_offset; + } + 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) + { + /* The original seek was not attempted as offset > OFF_T_MAX. + We should error for write as can't get to the 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 a significant issue. */ + 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 quit. */ + quit (EXIT_FAILURE); + } + /* else file_size && offset > OFF_T_MAX or file ! seekable */ do { @@ -1537,10 +1586,22 @@ dd_copy (void) if (skip_records != 0) { - skip (STDIN_FILENO, input_file, skip_records, input_blocksize, ibuf); + uintmax_t us_bytes = input_offset + (skip_records * input_blocksize); + uintmax_t us_blocks = skip (STDIN_FILENO, input_file, + skip_records, input_blocksize, ibuf); + us_bytes -= input_offset; + /* 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. */ + asked to skip past EOF, so I assume it's non-fatal. + There are 3 reasons why there might be unskipped blocks/bytes: + 1. file is too small + 2. pipe has not enough data + 3. short reads */ + if (us_blocks || (!input_offset_overflow && us_bytes)) + { + error (0, 0, + _("%s: cannot skip to specified offset"), quote (input_file)); + } } if (seek_records != 0) @@ -1778,7 +1839,7 @@ main (int argc, char **argv) offset = lseek (STDIN_FILENO, 0, SEEK_CUR); input_seekable = (0 <= offset); - input_offset = offset; + input_offset = MAX(0, offset); input_seek_errno = errno; if (output_file == NULL) diff --git a/tests/Makefile.am b/tests/Makefile.am index 6dce9cdbb..69475add0 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 000000000..04101d222 --- /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 . + +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_ "failed to 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 000000000..cfc143963 --- /dev/null +++ b/tests/dd/skip-seek-past-file @@ -0,0 +1,91 @@ +#!/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 . + +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 + +echo "\ +dd: \`standard input': cannot skip to specified offset +0+0 records in +0+0 records out" > skip_err || framework_failure + +# skipping beyond number of blocks in file should issue a warning +dd bs=1 skip=5 count=0 status=noxfer < file 2> err || fail=1 +compare skip_err err || fail=1 + +# skipping beyond number of bytes in file should issue a warning +dd bs=3 skip=2 count=0 status=noxfer < file 2> err || fail=1 +compare skip_err err || fail=1 + +# skipping beyond number of blocks in pipe should issue a warning +cat file | dd bs=1 skip=5 count=0 status=noxfer 2> err || fail=1 +compare skip_err err || fail=1 + +# skipping beyond number of bytes in pipe should issue a warning +cat file | dd bs=3 skip=2 count=0 status=noxfer 2> err || fail=1 +compare skip_err err || fail=1 + +# Check seeking beyond file already offset into +# skipping beyond number of blocks in file should issue a warning +(dd bs=1 skip=1 count=0 2>/dev/null && + dd bs=1 skip=4 status=noxfer 2> err) < file || fail=1 +compare skip_err err || fail=1 + +# Check seeking beyond file already offset into +# skipping beyond number of bytes in file should issue a warning +(dd bs=1 skip=1 count=0 2>/dev/null && + dd bs=2 skip=2 status=noxfer 2> err) < file || fail=1 +compare skip_err 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 -- 2.11.4.GIT