[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: dd skip bug?
From: |
Pádraig Brady |
Subject: |
Re: dd skip bug? |
Date: |
Mon, 8 Dec 2008 23:46:55 +0000 |
User-agent: |
Thunderbird 2.0.0.6 (X11/20071008) |
Jim Meyering wrote:
> Pádraig Brady <address@hidden> wrote:
>
>> I spent a little more time on this issue.
>> The attached patch makes the following changes for seekable inputs and
>> outputs.
>> Note skip is for input, and seek is for output in the following:
>>
>> skip beyond end of file
>> before: immediately exit(0);
>> after : immediately printf("cannot skip: Invalid argument); exit(0);
>>
>> 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);
>>
>> 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);
>>
>>
>> If the above is desired then I'll add tests before checking in.
>
> Those look like good changes.
> Thanks!
The proposed patch is attached.
thanks,
Pádraig.
>From de2926ab04f9e7ebf5bb706bbdad337c3e14e474 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
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);
* src/dd.c (skip): Add error checking for large seek/skip offsets on
seekable files, rather than deferring to using read() to advance offset.
* 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.
---
src/dd.c | 55 ++++++++++++++++++++++++++++++++--
tests/Makefile.am | 2 +
tests/dd/skip-seek-past-dev | 63 ++++++++++++++++++++++++++++++++++++++++
tests/dd/skip-seek-past-file | 66 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 182 insertions(+), 4 deletions(-)
create mode 100755 tests/dd/skip-seek-past-dev
create mode 100755 tests/dd/skip-seek-past-file
diff --git a/src/dd.c b/src/dd.c
index e54cc14..84d0124 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1248,12 +1248,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
{
@@ -1526,10 +1570,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 8dbbe9f..87b0e1e 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..75e291e
--- /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/lang-default
+. $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 . | tail -n1 | cut -d' ' -f1)
+
+dev_size=$(get_device_size "$device") ||
+skip_test_ "Could not determine size of $device"
+
+# Don't use shell arithimetic as dash at least only uses 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..a06afae
--- /dev/null
+++ b/tests/dd/skip-seek-past-file
@@ -0,0 +1,66 @@
+#!/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/lang-default
+. $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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: dd skip bug?,
Pádraig Brady <=