Re: dd bug

[Paul Eggert]

Buciuman Adrian <address@hidden> writes:

> read(0, "1                               "..., 737280) = 36864
> write(1, "1                               "..., 737280) = 737280
> read(0, 0x4015a000, 737280)             = -1 EIO (Input/output error)
> write(2, "dd: ", 4)                     = 4
> write(2, "reading `/dev/fd0\'", 18)     = 18
> write(2, ": Input/output error", 20)    = 20
> write(2, "\n", 1)                       = 1
> write(2, "0+1 records in\n", 15)        = 15
> write(2, "1+0 records out\n", 16)       = 16
> _llseek(0, 737280, [774144], SEEK_CUR)  = 0

I think I see one problem, though I'm not sure it's your problem.

Here is a patch to CVS coreutils.  I can't easily test this patch,
though, since I don't have your hardware.  Can you try this patch out?

2003-09-22  Paul Eggert  <address@hidden>

        Fix bug that may be related to Buciuman Adrian's bug report in
        <http://mail.gnu.org/archive/html/bug-coreutils/2003-08/msg00105.html>
        where 'dd' created a file that was too large.  The bug was that dd
        assumed that the input file offset does not advance after a failed
        read; but POSIX says that the input file offset is undefined after
        a failed read.

        This also causes 'noerror skip=...' to not fail on read error
        if the skip causes a read.  POSIX seems to require this.

        It also fixes a FIXME that I noticed while in the neighborhood.
        
        * src/dd.c (MAX_BLOCKSIZE): New macro.
        (input_seekable, input_seek_errno, input_offset,
        input_offset_overflow): New vars.
        (write_output, dd_copy): Pass EXIT_FAILURE to quit, not 1.
        (scanargs): Reject block sizes greater than MAX_BLOCKSIZE>
        (advance_input_offset): New function.
        (skip_via_lseek): Set errno to zero when reporting our failure,
        so that we don't report based on garbage errno.
        (skip): If fdesc is standard input, advance the input offset.
        Do not quit if reading, and if noerror was specified;
        POSIX seems to require this.
        If read fails on output file, report the earlier lseek failure
        instead; this fixes a FIXME in dd_copy.
        (advance_input_after_read_error): New function.
        (dd_copy): Use it, instead of assuming that failed reads
        do not advance the file pointer.  Advance input offset
        after nonfailed reads.
        (main): Determine initial input offset, seekability of input,
        and error if it wasn't seekable.

Index: dd.c
===================================================================
RCS file: /cvsroot/coreutils/coreutils/src/dd.c,v
retrieving revision 1.150
diff -p -u -r1.150 dd.c
--- dd.c        18 Sep 2003 22:19:03 -0000      1.150
+++ dd.c        22 Sep 2003 07:30:56 -0000
@@ -66,6 +66,12 @@
 /* Default input and output blocksize. */
 #define DEFAULT_BLOCKSIZE 512
 
+/* Maximum blocksize.  Keep it smaller than SIZE_MAX, so that we can
+   allocate buffers that size.  Keep it smaller than SSIZE_MAX, for
+   the benefit of system calls like "read".  And keep it smaller than
+   OFF_T_MAX, for the benefit of the large-offset seek code.  */
+#define MAX_BLOCKSIZE MIN (SIZE_MAX, MIN (SSIZE_MAX, OFF_T_MAX))
+
 /* Conversions bit masks. */
 #define C_ASCII 01
 #define C_EBCDIC 02
@@ -126,6 +132,19 @@ static uintmax_t r_partial = 0;
 /* Number of full blocks read. */
 static uintmax_t r_full = 0;
 
+/* True if input is seekable.  */
+static bool input_seekable;
+
+/* Error number corresponding to initial attempt to lseek input.
+   If ESPIPE, do not issue any more diagnostics about it.  */
+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.  */
+static uintmax_t input_offset;
+bool input_offset_overflow;
+
 /* Records truncated by conv=block. */
 static uintmax_t r_truncate = 0;
 
@@ -479,7 +498,7 @@ write_output (void)
       error (0, errno, _("writing to %s"), quote (output_file));
       if (nwritten != 0)
        w_partial++;
-      quit (1);
+      quit (EXIT_FAILURE);
     }
   else
     w_full++;
@@ -582,26 +601,20 @@ scanargs (int argc, char **argv)
 
          if (STREQ (name, "ibs"))
            {
-             /* Ensure that each blocksize is <= SSIZE_MAX.  */
-             invalid |= SSIZE_MAX < n;
+             invalid |= ! (0 < n && n <= MAX_BLOCKSIZE);
              input_blocksize = n;
-             invalid |= input_blocksize != n || input_blocksize == 0;
              conversions_mask |= C_TWOBUFS;
            }
          else if (STREQ (name, "obs"))
            {
-             /* Ensure that each blocksize is <= SSIZE_MAX.  */
-             invalid |= SSIZE_MAX < n;
+             invalid |= ! (0 < n && n <= MAX_BLOCKSIZE);
              output_blocksize = n;
-             invalid |= output_blocksize != n || output_blocksize == 0;
              conversions_mask |= C_TWOBUFS;
            }
          else if (STREQ (name, "bs"))
            {
-             /* Ensure that each blocksize is <= SSIZE_MAX.  */
-             invalid |= SSIZE_MAX < n;
+             invalid |= ! (0 < n && n <= MAX_BLOCKSIZE);
              output_blocksize = input_blocksize = n;
-             invalid |= output_blocksize != n || output_blocksize == 0;
            }
          else if (STREQ (name, "cbs"))
            {
@@ -747,6 +760,17 @@ swab_buffer (char *buf, size_t *nread)
   return ++bufstart;
 }
 
+/* Add OFFSET to the input offset, setting the overflow flag if
+   necessary.  */
+
+static uintmax_t
+advance_input_offset (uintmax_t offset)
+{
+  input_offset += offset;
+  if (input_offset < offset)
+    input_offset_overflow = true;
+}
+
 /* This is a wrapper for lseek.  It detects and warns about a kernel
    bug that makes lseek a no-op for tape devices, even though the kernel
    lseek return value suggests that the function succeeded.
@@ -791,6 +815,7 @@ skip_via_lseek (char const *filename, in
       error (0, 0, _("warning: working around lseek kernel bug for file (%s)\n\
   of mt_type=0x%0lx -- see <sys/mtio.h> for the list of types"),
             filename, s2.mt_type);
+      errno = 0;
       new_position = -1;
     }
 
@@ -803,7 +828,7 @@ skip_via_lseek (char const *filename, in
 /* Throw away RECORDS blocks of BLOCKSIZE bytes on file descriptor FDESC,
    which is open with read permission for FILE.  Store up to BLOCKSIZE
    bytes of the data at a time in BUF, if necessary.  RECORDS must be
-   nonzero.  */
+   nonzero.  If fdesc is STDIN_FILENO, advance the input offset.  */
 
 static void
 skip (int fdesc, char const *file, uintmax_t records, size_t blocksize,
@@ -815,26 +840,91 @@ skip (int fdesc, char const *file, uintm
      or if the the file offset is not representable as an off_t --
      fall back on using read.  */
 
-  if ((uintmax_t) offset / blocksize != records
-      || skip_via_lseek (file, fdesc, offset, SEEK_CUR) < 0)
+  errno = 0;
+  if ((uintmax_t) offset / blocksize == records
+      && 0 <= skip_via_lseek (file, fdesc, offset, SEEK_CUR))
+    {
+      if (fdesc == STDIN_FILENO)
+       advance_input_offset (offset);
+    }
+  else
     {
+      int lseek_errno = errno;
       while (records--)
        {
          size_t nread = safe_read (fdesc, buf, blocksize);
          if (nread == SAFE_READ_ERROR)
            {
-             error (0, errno, _("reading %s"), quote (file));
-             quit (1);
+             if (fdesc == STDIN_FILENO)
+               {
+                 error (0, errno, _("reading %s"), quote (file));
+                 if (conversions_mask & C_NOERROR)
+                   {
+                     print_stats ();
+                     continue;
+                   }
+               }
+             else
+               error (0, lseek_errno, _("%s: cannot seek"), quote (file));
+             quit (EXIT_FAILURE);
            }
          /* 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.
             FIXME: maybe give a warning.  */
          if (nread == 0)
            break;
+         if (fdesc == STDIN_FILENO)
+           advance_input_offset (nread);
        }
     }
 }
 
+/* Advance the input by one record if possible, after a read error.
+   The input file offset may or may not have advanced after the failed
+   read; adjust it to point just after the bad record regardless.
+   Return true if successful, or if the input is already known to not
+   be seekable.  */
+
+static bool
+advance_input_after_read_error (size_t blocksize)
+{
+  if (! input_seekable)
+    {
+      if (input_seek_errno == ESPIPE)
+       return true;
+      errno = input_seek_errno;
+    }
+  else
+    {
+      off_t offset;
+      advance_input_offset (blocksize);
+      input_offset_overflow |= (OFF_T_MAX < input_offset);
+      if (input_offset_overflow)
+       {
+         error (0, 0, _("offset overflow while reading file %s"),
+                quote (input_file));
+         return false;
+       }
+      offset = lseek (STDIN_FILENO, 0, SEEK_CUR);
+      if (0 <= offset)
+       {
+         off_t diff;
+         if (offset == input_offset)
+           return true;
+         diff = input_offset - offset;
+         if (! (0 <= diff && diff <= blocksize))
+           error (0, 0, _("warning: screwy file offset after failed read"));
+         if (0 <= skip_via_lseek (input_file, STDIN_FILENO, diff, SEEK_CUR))
+           return true;
+         if (errno == 0)
+           error (0, 0, _("cannot work around kernel bug after all"));
+       }
+    }
+
+  error (0, errno, _("%s: cannot seek"), quote (input_file));
+  return false;
+}
+
 /* Copy NREAD bytes of BUF, with no conversions.  */
 
 static void
@@ -983,16 +1073,7 @@ dd_copy (void)
     skip (STDIN_FILENO, input_file, skip_records, input_blocksize, ibuf);
 
   if (seek_records != 0)
-    {
-      /* FIXME: this loses for
-        % ./dd if=dd seek=1 |:
-        ./dd: standard output: Bad file descriptor
-        0+0 records in
-        0+0 records out
-        */
-
-      skip (STDOUT_FILENO, output_file, seek_records, output_blocksize, obuf);
-    }
+    skip (STDOUT_FILENO, output_file, seek_records, output_blocksize, obuf);
 
   if (max_records == 0)
     quit (exit_status);
@@ -1022,7 +1103,14 @@ dd_copy (void)
            {
              print_stats ();
              /* Seek past the bad block if possible. */
-             lseek (STDIN_FILENO, (off_t) input_blocksize, SEEK_CUR);
+             if (! advance_input_after_read_error (input_blocksize))
+               {
+                 exit_status = EXIT_FAILURE;
+
+                 /* Suppress duplicate diagnostics.  */
+                 input_seekable = false;
+                 input_seek_errno = ESPIPE;
+               }
              if (conversions_mask & C_SYNC)
                /* Replace the missing input with null bytes and
                   proceed normally.  */
@@ -1039,6 +1127,7 @@ dd_copy (void)
        }
 
       n_bytes_read = nread;
+      advance_input_offset (nread);
 
       if (n_bytes_read < input_blocksize)
        {
@@ -1062,7 +1151,7 @@ dd_copy (void)
          if (nwritten != n_bytes_read)
            {
              error (0, errno, _("writing %s"), quote (output_file));
-             quit (1);
+             quit (EXIT_FAILURE);
            }
          else if (n_bytes_read == input_blocksize)
            w_full++;
@@ -1123,7 +1212,7 @@ dd_copy (void)
       if (nwritten != oc)
        {
          error (0, errno, _("writing %s"), quote (output_file));
-         quit (1);
+         quit (EXIT_FAILURE);
        }
     }
 
@@ -1182,6 +1271,13 @@ main (int argc, char **argv)
     }
   else
     input_file = _("standard input");
+
+  {
+    off_t offset = lseek (STDIN_FILENO, 0, SEEK_CUR);
+    input_seekable = (0 <= offset);
+    input_offset = offset;
+    input_seek_errno = errno;
+  }
 
   if (output_file != NULL)
     {