Author: asomers
Date: Fri Aug 25 15:31:55 2017
New Revision: 322893
URL: https://svnweb.freebsd.org/changeset/base/322893

Log:
  dd(1): Incorrect casting of arguments
  
  dd(1) casts many of its numeric arguments from uintmax_t to intmax_t and
  back again to detect whether or not the original arguments were negative.
  This is not correct, and causes problems with boundary cases, for example
  when count is SSIZE_MAX-1.
  
  PR:           191263
  Submitted by: w...@worrbase.com
  Reviewed by:  pi, asomers
  MFC after:    3 weeks

Modified:
  head/bin/dd/args.c
  head/bin/dd/conv.c
  head/bin/dd/dd.c
  head/bin/dd/dd.h
  head/bin/dd/position.c

Modified: head/bin/dd/args.c
==============================================================================
--- head/bin/dd/args.c  Fri Aug 25 14:42:11 2017        (r322892)
+++ head/bin/dd/args.c  Fri Aug 25 15:31:55 2017        (r322893)
@@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$");
 
 #include <sys/types.h>
 
+#include <ctype.h>
 #include <err.h>
 #include <errno.h>
 #include <inttypes.h>
@@ -184,7 +185,7 @@ f_bs(char *arg)
 
        res = get_num(arg);
        if (res < 1 || res > SSIZE_MAX)
-               errx(1, "bs must be between 1 and %jd", (intmax_t)SSIZE_MAX);
+               errx(1, "bs must be between 1 and %zd", SSIZE_MAX);
        in.dbsz = out.dbsz = (size_t)res;
 }
 
@@ -195,22 +196,22 @@ f_cbs(char *arg)
 
        res = get_num(arg);
        if (res < 1 || res > SSIZE_MAX)
-               errx(1, "cbs must be between 1 and %jd", (intmax_t)SSIZE_MAX);
+               errx(1, "cbs must be between 1 and %zd", SSIZE_MAX);
        cbsz = (size_t)res;
 }
 
 static void
 f_count(char *arg)
 {
-       intmax_t res;
+       uintmax_t res;
 
-       res = (intmax_t)get_num(arg);
-       if (res < 0)
-               errx(1, "count cannot be negative");
+       res = get_num(arg);
+       if (res == UINTMAX_MAX)
+               errc(1, ERANGE, "%s", oper);
        if (res == 0)
-               cpy_cnt = (uintmax_t)-1;
+               cpy_cnt = UINTMAX_MAX;
        else
-               cpy_cnt = (uintmax_t)res;
+               cpy_cnt = res;
 }
 
 static void
@@ -219,7 +220,7 @@ f_files(char *arg)
 
        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 %zu", SIZE_MAX);
 }
 
 static void
@@ -240,8 +241,8 @@ f_ibs(char *arg)
        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);
+                       errx(1, "ibs must be between 1 and %zd",
+                           SSIZE_MAX);
                in.dbsz = (size_t)res;
        }
 }
@@ -261,8 +262,8 @@ f_obs(char *arg)
        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);
+                       errx(1, "obs must be between 1 and %zd",
+                           SSIZE_MAX);
                out.dbsz = (size_t)res;
        }
 }

Modified: head/bin/dd/conv.c
==============================================================================
--- head/bin/dd/conv.c  Fri Aug 25 14:42:11 2017        (r322892)
+++ head/bin/dd/conv.c  Fri Aug 25 15:31:55 2017        (r322893)
@@ -133,7 +133,7 @@ block(void)
         */
        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)
@@ -146,7 +146,7 @@ block(void)
                 * 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;
                }
@@ -228,7 +228,7 @@ unblock(void)
         * 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) {

Modified: head/bin/dd/dd.c
==============================================================================
--- head/bin/dd/dd.c    Fri Aug 25 14:42:11 2017        (r322892)
+++ head/bin/dd/dd.c    Fri Aug 25 15:31:55 2017        (r322893)
@@ -204,10 +204,10 @@ setup(void)
         * 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");
 
@@ -405,7 +405,7 @@ dd_in(void)
                        ++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;
 
@@ -562,7 +562,7 @@ dd_out(int force)
                        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;

Modified: head/bin/dd/dd.h
==============================================================================
--- head/bin/dd/dd.h    Fri Aug 25 14:42:11 2017        (r322892)
+++ head/bin/dd/dd.h    Fri Aug 25 15:31:55 2017        (r322893)
@@ -38,10 +38,9 @@
 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) */

Modified: head/bin/dd/position.c
==============================================================================
--- head/bin/dd/position.c      Fri Aug 25 14:42:11 2017        (r322892)
+++ head/bin/dd/position.c      Fri Aug 25 15:31:55 2017        (r322893)
@@ -207,7 +207,7 @@ pos_out(void)
                        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;
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to