Subject: bug#13391: closed (Re: bug#13391: dd silently ignores lseek error)

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 09 Jan 2013 01:15:02 +0000

Your bug report

#13391: dd silently ignores lseek error

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
13391: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13391
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#13391: dd silently ignores lseek error
From: Pádraig Brady <[email-masked]>
To: Paul Eggert <[email-masked]>
Cc: Neil Klopfenstein <[email-masked]>, [email-masked]
Date: Wed, 09 Jan 2013 01:14:22 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

On 01/08/2013 08:55 PM, Paul Eggert wrote:
> On 01/08/13 10:11, Neil Klopfenstein wrote:
>> Note that it begins reading at the _beginning of the ar file_ -- the 'skip'
>> argument has failed silently.
>
> But the 'skip' hasn't failed.  It's merely being implemented via 'read'
> rather than via 'lseek'.  The records are being skipped correctly.
>
> It might be useful to give dd a new option, which causes it
> to insist on lseeking rather than reading in cases like these,
> and to report an error if the lseek fails.

I had a look around for a tool to verify
that a file/device supports the seek operation
and couldn't find one.
So this seems like useful functionality.
Worth applying the attached?

thanks,
Pádraig.
>From ea524ab7388bb35e591dcdb0fc7f7989d61143ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Wed, 9 Jan 2013 00:42:38 +0000
Subject: [PATCH] dd: add [io]flag=seekable to verify file support for lseek

* src/dd.c: Add the new O_SEEKABLE flag.
(main): Verify leek() works if O_SEEKABLE is set.
(usage): Describe the new flag.
* tests/dd/misc.sh: Augment the test for the new options.
* doc/coreutils.texi (dd invocation): Describe the new option.
* cfg.mk (sc_dd_O_FLAGS): Add O_SEEKABLE to the list of private
flags with a a single underscore.
* NEWS: Mention the new feature.
Suggested by Paul Eggert in http://bugs.gnu.org/13391
---
 NEWS               |    3 +++
 cfg.mk             |    2 +-
 doc/coreutils.texi |    6 ++++++
 src/dd.c           |   28 +++++++++++++++++++++++++++-
 tests/dd/misc.sh   |    8 ++++++++
 5 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 754b2cf..9c415ec 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** New features
 
+  dd now accepts 'iflag=seekable' and 'oflag=seekable' to verify that the
+  corresponding files support the seek operation.
+
   df now accepts the --output[=FIELD_LIST] option to define the list of columns
   to include in the output, or all available columns if the FIELD_LIST is
   omitted.  Note this enables df to output both block and inode fields together.
diff --git a/cfg.mk b/cfg.mk
index fbc64b4..bf43861 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -57,7 +57,7 @@ _hv_file ?= $(srcdir)/tests/misc/help-version
 dd = $(srcdir)/src/dd.c
 sc_dd_O_FLAGS:
 	@rm -f $@.1 $@.2
-	@{ echo O_FULLBLOCK; echo O_NOCACHE;				\
+	@{ echo O_FULLBLOCK; echo O_NOCACHE; echo O_SEEKABLE;		\
 	  perl -nle '/^ +\| (O_\w*)$$/ and print $$1' $(dd); } | sort > $@.1
 	@{ echo O_NOFOLLOW; perl -nle '/{"[a-z]+",\s*(O_\w+)},/ and print $$1' \
 	  $(dd); } | sort > $@.2
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 45a4b3d..9ee9d86 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8399,6 +8399,12 @@ rather than a block count, which allows specifying
 an offset that is not a multiple of the I/O block size.
 This flag can be used only with @code{oflag}.
 
+@item seekable
+@opindex seekable
+Fail unless the file is seekable.
+Note @samp{skip=} or @samp{seek=} don't suffice for this check,
+as they will resort to reading, to skip over data.
+
 @end table
 
 These flags are not supported on all systems, and @samp{dd} rejects
diff --git a/src/dd.c b/src/dd.c
index ef5664b..21b4b21 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -216,6 +216,9 @@ static xtime_t start_time;
 /* True if input is seekable.  */
 static bool input_seekable;
 
+/* True if output is seekable.  */
+static bool output_seekable;
+
 /* Error number corresponding to initial attempt to lseek input.
    If ESPIPE, do not issue any more diagnostics about it.  */
 static int input_seek_errno;
@@ -326,7 +329,10 @@ enum
     O_SKIP_BYTES = FFS_MASK (v4),
     v5 = v4 ^ O_SKIP_BYTES,
 
-    O_SEEK_BYTES = FFS_MASK (v5)
+    O_SEEK_BYTES = FFS_MASK (v5),
+    v6 = v5 ^ O_SEEK_BYTES,
+
+    O_SEEKABLE = FFS_MASK (v6)
   };
 
 /* Ensure that we got something.  */
@@ -335,6 +341,7 @@ verify (O_NOCACHE != 0);
 verify (O_COUNT_BYTES != 0);
 verify (O_SKIP_BYTES != 0);
 verify (O_SEEK_BYTES != 0);
+verify (O_SEEKABLE != 0);
 
 #define MULTIPLE_BITS_SET(i) (((i) & ((i) - 1)) != 0)
 
@@ -344,6 +351,7 @@ verify ( ! MULTIPLE_BITS_SET (O_NOCACHE));
 verify ( ! MULTIPLE_BITS_SET (O_COUNT_BYTES));
 verify ( ! MULTIPLE_BITS_SET (O_SKIP_BYTES));
 verify ( ! MULTIPLE_BITS_SET (O_SEEK_BYTES));
+verify ( ! MULTIPLE_BITS_SET (O_SEEKABLE));
 
 /* Flags, for iflag="..." and oflag="...".  */
 static struct symbol_value const flags[] =
@@ -366,6 +374,7 @@ static struct symbol_value const flags[] =
   {"count_bytes", O_COUNT_BYTES},
   {"skip_bytes",  O_SKIP_BYTES},
   {"seek_bytes",  O_SEEK_BYTES},
+  {"seekable",    O_SEEKABLE},
   {"",		0}
 };
 
@@ -619,6 +628,9 @@ Each FLAG symbol may be:\n\
       if (O_SEEK_BYTES)
         fputs (_("  seek_bytes  treat 'seek=N' as a byte count (oflag only)\n\
 "), stdout);
+      if (O_SEEKABLE)
+        fputs (_("  seekable  fail unless seekable\n\
+"), stdout);
 
       {
         char const *siginfo_name = (SIGINFO == SIGUSR1 ? "USR1" : "INFO");
@@ -1607,6 +1619,7 @@ skip (int fdesc, char const *file, uintmax_t records, size_t blocksize,
         }
       /* else file_size && offset > OFF_T_MAX or file ! seekable */
 
+
       do
         {
           ssize_t nread = iread_fnc (fdesc, buf, records ? blocksize : *bytes);
@@ -2270,6 +2283,19 @@ main (int argc, char **argv)
         }
     }
 
+  offset = lseek (STDOUT_FILENO, 0, SEEK_CUR);
+  output_seekable = (0 <= offset);
+  if (! input_seekable && (input_flags & O_SEEKABLE))
+    {
+      error (EXIT_FAILURE, input_seek_errno, _("input is not seekable %s"),
+             quote (input_file));
+    }
+  if (! output_seekable && (output_flags & O_SEEKABLE))
+    {
+      error (EXIT_FAILURE, errno, _("output is not seekable %s"),
+             quote (output_file));
+    }
+
   start_time = gethrxtime ();
 
   exit_status = dd_copy ();
diff --git a/tests/dd/misc.sh b/tests/dd/misc.sh
index b9ad31a..cc84457 100755
--- a/tests/dd/misc.sh
+++ b/tests/dd/misc.sh
@@ -102,4 +102,12 @@ compare err_ok err || fail=1
 
 test $fail -eq 0 && fail=$warn
 
+# check iflag=seekable and oflag=seekable
+dd if=/dev/null of=/dev/null iflag=seekable oflag=seekable count=0 || fail=1
+echo 'pipe' | dd of=/dev/null iflag=seekable count=0 && fail=1
+dd if=/dev/null oflag=seekable count=0 status=none 2>err | cat
+echo "dd: output is not seekable 'standard output': Illegal seek" > err_ok ||
+  framework_failure_
+compare err_ok err || fail=1
+
 Exit $fail
-- 
1.7.6.4

Subject: dd silently ignores lseek error
From: Neil Klopfenstein <[email-masked]>
To: [email-masked]
Date: Tue, 8 Jan 2013 15:11:22 -0300
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi all,

While trying to diagnose a weird filesystem bug, I found an error in GNU dd
v8.12.

The weird bug is causing lseek() to fail improperly. That's not the problem
I'm reporting, though. I was trying to use dd to demonstrate the lseek
error to my sysadmin. Instead, I found that dd is ignoring the lseek
failure.

Here is the relevant strace output:
$ strace dd if=libdvapp-O.a of=/dev/null bs=33k skip=1
...
open("libdvapp-O.a", O_RDONLY) = 3
dup2(3, 0)                              = 0
close(3)                                = 0
lseek(0, 0, SEEK_CUR)                   = 0
open("/dev/null", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
dup2(3, 1)                              = 1
close(3)                                = 0
clock_gettime(CLOCK_MONOTONIC, {12951065, 389531862}) = 0
ioctl(0, MGSL_IOCSTXIDLE or MTIOCGET or SNDCTL_MIDI_MPUCMD, 0x7fbfffe3f0) =
-1 ENOTTY (Inappropriate ioctl for device)
lseek(0, 33792, SEEK_CUR)               = -1 EINVAL (Invalid argument)
 ^ this is the syscall which should not be failing
ioctl(0, MGSL_IOCSTXIDLE or MTIOCGET or SNDCTL_MIDI_MPUCMD, 0x7fbfffe3f0) =
-1 ENOTTY (Inappropriate ioctl for device)
lseek(0, 0, SEEK_END)                   = -1 EINVAL (Invalid argument)
read(0, "!<arch>\n/               13576110"..., 33792) = 33792
...

Note that it begins reading at the _beginning of the ar file_ -- the 'skip'
argument has failed silently. The output of dd does not indicate any error:

3+1 records in
3+1 records out
103310 bytes (103 kB) copied, 0.000375 s, 275 MB/s

Expected behavior: dd should pass on any unexpected errors reported by
system calls.

I realize this is an old version of coreutils but I don't have a newer
version available to test against the anomaly. Sorry if it's already been
fixed.

Regards,
--Neil