[PATCH] Fix integer overflow handling in dd(1)

Hey,

I've submitted this patch before, and it's gotten comments and fixes, 
but still hasn't been merged. Any thoughts? Does it need more work?

Thanks,
William Orr

Index: args.c
===================================================================
--- args.c    (revision 270645)
+++ args.c    (working copy)
_at__at_ -41,6 +41,7 _at__at_

  #include <sys/types.h>

+#include <ctype.h>
  #include <err.h>
  #include <errno.h>
  #include <inttypes.h>
_at__at_ -171,8 +172,7 _at__at_
       */
      if (in.offset > OFF_MAX / (ssize_t)in.dbsz ||
          out.offset > OFF_MAX / (ssize_t)out.dbsz)
-        errx(1, "seek offsets cannot be larger than %jd",
-            (intmax_t)OFF_MAX);
+        errx(1, "seek offsets cannot be larger than %jd", OFF_MAX);
  }

  static int
_at__at_ -186,37 +186,28 _at__at_
  static void
  f_bs(char *arg)
  {
-    uintmax_t res;

-    res = get_num(arg);
-    if (res < 1 || res > SSIZE_MAX)
-        errx(1, "bs must be between 1 and %jd", (intmax_t)SSIZE_MAX);
-    in.dbsz = out.dbsz = (size_t)res;
+    in.dbsz = out.dbsz = get_num(arg);
+    if (out.dbsz < 1 || out.dbsz > SSIZE_MAX)
+        errx(1, "bs must be between 1 and %jd", SSIZE_MAX);
  }

  static void
  f_cbs(char *arg)
  {
-    uintmax_t res;

-    res = get_num(arg);
-    if (res < 1 || res > SSIZE_MAX)
-        errx(1, "cbs must be between 1 and %jd", (intmax_t)SSIZE_MAX);
-    cbsz = (size_t)res;
+    cbsz = get_num(arg);
+    if (cbsz < 1 || cbsz > SSIZE_MAX)
+        errx(1, "cbs must be between 1 and %jd", SSIZE_MAX);
  }

  static void
  f_count(char *arg)
  {
-    intmax_t res;

-    res = (intmax_t)get_num(arg);
-    if (res < 0)
-        errx(1, "count cannot be negative");
-    if (res == 0)
-        cpy_cnt = (uintmax_t)-1;
-    else
-        cpy_cnt = (uintmax_t)res;
+    cpy_cnt = get_num(arg);
+    if (cpy_cnt == 0)
+        cpy_cnt = -1;
  }

  static void
_at__at_ -225,7 +216,7 _at__at_

      files_cnt = get_num(arg);
      if (files_cnt < 1)
-        errx(1, "files must be between 1 and %jd", (uintmax_t)-1);
+        errx(1, "files must be between 1 and %ju", SIZE_MAX);
  }

  static void
_at__at_ -241,14 +232,11 _at__at_
  static void
  f_ibs(char *arg)
  {
-    uintmax_t res;

      if (!(ddflags & C_BS)) {
-        res = get_num(arg);
-        if (res < 1 || res > SSIZE_MAX)
-            errx(1, "ibs must be between 1 and %jd",
-                (intmax_t)SSIZE_MAX);
-        in.dbsz = (size_t)res;
+        in.dbsz = get_num(arg);
+        if (in.dbsz < 1 || in.dbsz > SSIZE_MAX)
+            errx(1, "ibs must be between 1 and %ju", SSIZE_MAX);
      }
  }

_at__at_ -262,14 +250,11 _at__at_
  static void
  f_obs(char *arg)
  {
-    uintmax_t res;

      if (!(ddflags & C_BS)) {
-        res = get_num(arg);
-        if (res < 1 || res > SSIZE_MAX)
-            errx(1, "obs must be between 1 and %jd",
-                (intmax_t)SSIZE_MAX);
-        out.dbsz = (size_t)res;
+        out.dbsz = get_num(arg);
+        if (out.dbsz < 1 || out.dbsz > SSIZE_MAX)
+            errx(1, "obs must be between 1 and %jd", SSIZE_MAX);
      }
  }

_at__at_ -378,11 +363,17 _at__at_
      uintmax_t num, mult, prevnum;
      char *expr;

+    while (isspace(val[0]))
+        val++;
+
+    if (val[0] == '-')
+        errx(1, "%s: cannot be negative", oper);
+
      errno = 0;
-    num = strtouq(val, &expr, 0);
+    num = strtoull(val, &expr, 0);
      if (errno != 0)                /* Overflow or underflow. */
          err(1, "%s", oper);
-
+
      if (expr == val)            /* No valid digits. */
          errx(1, "%s: illegal numeric value", oper);

Index: conv.c
===================================================================
--- conv.c    (revision 270645)
+++ conv.c    (working copy)
_at__at_ -133,7 +133,7 _at__at_
       */
      ch = 0;
      for (inp = in.dbp - in.dbcnt, outp = out.dbp; in.dbcnt;) {
-        maxlen = MIN(cbsz, in.dbcnt);
+        maxlen = MIN(cbsz, (size_t)in.dbcnt);
          if ((t = ctab) != NULL)
              for (cnt = 0; cnt < maxlen && (ch = *inp++) != '\n';
                  ++cnt)
_at__at_ -146,7 +146,7 _at__at_
           * Check for short record without a newline.  Reassemble the
           * input block.
           */
-        if (ch != '\n' && in.dbcnt < cbsz) {
+        if (ch != '\n' && (size_t)in.dbcnt < cbsz) {
              (void)memmove(in.db, in.dbp - in.dbcnt, in.dbcnt);
              break;
          }
_at__at_ -228,7 +228,7 _at__at_
       * translation has to already be done or we might not recognize the
       * spaces.
       */
-    for (inp = in.db; in.dbcnt >= cbsz; inp += cbsz, in.dbcnt -= cbsz) {
+    for (inp = in.db; (size_t)in.dbcnt >= cbsz; inp += cbsz, in.dbcnt
-= cbsz) {
          for (t = inp + cbsz - 1; t >= inp && *t == ' '; --t)
              ;
          if (t >= inp) {
Index: dd.c
===================================================================
--- dd.c    (revision 270645)
+++ dd.c    (working copy)
_at__at_ -168,10 +168,10 _at__at_
       * record oriented I/O, only need a single buffer.
       */
      if (!(ddflags & (C_BLOCK | C_UNBLOCK))) {
-        if ((in.db = malloc(out.dbsz + in.dbsz - 1)) == NULL)
+        if ((in.db = malloc((size_t)out.dbsz + in.dbsz - 1)) == NULL)
              err(1, "input buffer");
          out.db = in.db;
-    } else if ((in.db = malloc(MAX(in.dbsz, cbsz) + cbsz)) == NULL ||
+    } else if ((in.db = malloc(MAX((size_t)in.dbsz, cbsz) + cbsz)) ==
NULL ||
          (out.db = malloc(out.dbsz + cbsz)) == NULL)
          err(1, "output buffer");

_at__at_ -343,7 +343,7 _at__at_
              ++st.in_full;

          /* Handle full input blocks. */
-        } else if ((size_t)n == in.dbsz) {
+        } else if ((size_t)n == (size_t)in.dbsz) {
              in.dbcnt += in.dbrcnt = n;
              ++st.in_full;

_at__at_ -493,7 +493,7 _at__at_
              outp += nw;
              st.bytes += nw;

-            if ((size_t)nw == n && n == out.dbsz)
+            if ((size_t)nw == n && n == (size_t)out.dbsz)
                  ++st.out_full;
              else
                  ++st.out_part;
Index: dd.h
===================================================================
--- dd.h    (revision 270645)
+++ dd.h    (working copy)
_at__at_ -38,10 +38,9 _at__at_
  typedef struct {
      u_char        *db;        /* buffer address */
      u_char        *dbp;        /* current buffer I/O address */
-    /* XXX ssize_t? */
-    size_t        dbcnt;        /* current buffer byte count */
-    size_t        dbrcnt;        /* last read byte count */
-    size_t        dbsz;        /* block size */
+    ssize_t        dbcnt;        /* current buffer byte count */
+    ssize_t        dbrcnt;        /* last read byte count */
+    ssize_t        dbsz;        /* block size */

  #define    ISCHR        0x01        /* character device (warn on short) */
  #define    ISPIPE        0x02        /* pipe-like (see position.c) */
_at__at_ -57,13 +56,13 _at__at_
  } IO;

  typedef struct {
-    uintmax_t    in_full;    /* # of full input blocks */
-    uintmax_t    in_part;    /* # of partial input blocks */
-    uintmax_t    out_full;    /* # of full output blocks */
-    uintmax_t    out_part;    /* # of partial output blocks */
-    uintmax_t    trunc;        /* # of truncated records */
-    uintmax_t    swab;        /* # of odd-length swab blocks */
-    uintmax_t    bytes;        /* # of bytes written */
+    size_t    in_full;    /* # of full input blocks */
+    size_t    in_part;    /* # of partial input blocks */
+    size_t    out_full;    /* # of full output blocks */
+    size_t    out_part;    /* # of partial output blocks */
+    size_t    trunc;        /* # of truncated records */
+    size_t    swab;        /* # of odd-length swab blocks */
+    size_t    bytes;        /* # of bytes written */
      struct timespec    start;        /* start time of dd */
  } STAT;

Index: position.c
===================================================================
--- position.c    (revision 270645)
+++ position.c    (working copy)
_at__at_ -178,7 +178,7 _at__at_
              n = write(out.fd, out.db, out.dbsz);
              if (n == -1)
                  err(1, "%s", out.name);
-            if ((size_t)n != out.dbsz)
+            if (n != out.dbsz)
                  errx(1, "%s: write failure", out.name);
          }
          break;