When ibs is an integral multiple of ibs, and there is a sequence of obs consecutive zeroes at the end of the last input block, the last character written by dd may be some different (non-zero) byte from the input stream. How-To-Repeat: $ perl -e 'print "ABCDEFGH\0\0\0\0\0\0\0\0"'|dd ibs=16 obs=8 conv=sparse of=toto 1+0 records in 2+0 records out 16 bytes transferred in 0.001373 secs (11653 bytes/sec) $ od -a toto 0000000 A B C D E F G H nul nul nul nul nul nul nul A 0000020
Author: thomas Date: Wed May 7 19:33:29 2014 New Revision: 265593 URL: http://svnweb.freebsd.org/changeset/base/265593 Log: (dd_out): Fix handling of all-zeroes block at end of input with conv=sparse. This change fixes two separate issues observed when the last output block is all zeroes, and conv=sparse is in use. In this case, care must be taken to roll back the last seek and write the entire last zero block at the original offset where it should have occurred: when the destination file is a block device, it is not possible to roll back by just one character as the write would then not be properly aligned. Furthermore, the buffer used to write this last all-zeroes block needs to be properly zeroed-out. This was not the case previously, resulting in a junk data byte appearing instead of a zero in the output stream. PR: bin/189174 PR: bin/189284 Reviewed by: kib MFC after: 2 weeks Modified: head/bin/dd/dd.c Modified: head/bin/dd/dd.c ============================================================================== --- head/bin/dd/dd.c Wed May 7 19:30:28 2014 (r265592) +++ head/bin/dd/dd.c Wed May 7 19:33:29 2014 (r265593) @@ -51,6 +51,7 @@ __FBSDID("$FreeBSD$"); #include <sys/disklabel.h> #include <sys/filio.h> +#include <assert.h> #include <ctype.h> #include <err.h> #include <errno.h> @@ -77,6 +78,7 @@ STAT st; /* statistics */ void (*cfunc)(void); /* conversion function */ uintmax_t cpy_cnt; /* # of blocks to copy */ static off_t pending = 0; /* pending seek if sparse */ +static off_t last_sp = 0; /* size of last added sparse block */ u_int ddflags = 0; /* conversion options */ size_t cbsz; /* conversion block size */ uintmax_t files_cnt = 1; /* # of files to copy */ @@ -174,6 +176,8 @@ setup(void) } else if ((in.db = malloc(MAX(in.dbsz, cbsz) + cbsz)) == NULL || (out.db = malloc(out.dbsz + cbsz)) == NULL) err(1, "output buffer"); + + /* dbp is the first free position in each buffer. */ in.dbp = in.db; out.dbp = out.db; @@ -436,8 +440,15 @@ dd_out(int force) * we play games with the buffer size, and it's usually a partial write. */ outp = out.db; + + /* + * If force, first try to write all pending data, else try to write + * just one block. Subsequently always write data one full block at + * a time at most. + */ for (n = force ? out.dbcnt : out.dbsz;; n = out.dbsz) { - for (cnt = n;; cnt -= nw) { + cnt = n; + do { sparse = 0; if (ddflags & C_SPARSE) { sparse = 1; /* Is buffer sparse? */ @@ -449,18 +460,24 @@ dd_out(int force) } if (sparse && !force) { pending += cnt; + last_sp = cnt; nw = cnt; } else { if (pending != 0) { - if (force) - pending--; + /* If forced to write, and we have no + * data left, we need to write the last + * sparse block explicitly. + */ + if (force && cnt == 0) { + pending -= last_sp; + assert(outp == out.db); + memset(outp, 0, cnt); + } if (lseek(out.fd, pending, SEEK_CUR) == -1) err(2, "%s: seek error creating sparse file", out.name); - if (force) - write(out.fd, outp, 1); - pending = 0; + pending = last_sp = 0; } if (cnt) nw = write(out.fd, outp, cnt); @@ -475,27 +492,29 @@ dd_out(int force) err(1, "%s", out.name); nw = 0; } + outp += nw; st.bytes += nw; - if ((size_t)nw == n) { - if (n != out.dbsz) - ++st.out_part; - else - ++st.out_full; - break; - } - ++st.out_part; - if ((size_t)nw == cnt) - break; - if (out.flags & ISTAPE) - errx(1, "%s: short write on tape device", - out.name); - if (out.flags & ISCHR && !warned) { - warned = 1; - warnx("%s: short write on character device", - out.name); + + if ((size_t)nw == n && n == out.dbsz) + ++st.out_full; + else + ++st.out_part; + + if ((size_t) nw != cnt) { + if (out.flags & ISTAPE) + errx(1, "%s: short write on tape device", + out.name); + if (out.flags & ISCHR && !warned) { + warned = 1; + warnx("%s: short write on character device", + out.name); + } } - } + + cnt -= nw; + } while (cnt != 0); + if ((out.dbcnt -= n) < out.dbsz) break; } _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
State Changed From-To: open->closed Fixed in head.
Author: thomas Date: Wed May 21 07:21:36 2014 New Revision: 266488 URL: http://svnweb.freebsd.org/changeset/base/266488 Log: MFC rev. 265593: (dd_out): Fix handling of all-zeroes block at end of input with conv=sparse. PR: bin/189174 PR: bin/189284 Reviewed by: kib Modified: stable/10/bin/dd/dd.c Directory Properties: stable/10/ (props changed) Modified: stable/10/bin/dd/dd.c ============================================================================== --- stable/10/bin/dd/dd.c Wed May 21 06:33:21 2014 (r266487) +++ stable/10/bin/dd/dd.c Wed May 21 07:21:36 2014 (r266488) @@ -52,6 +52,7 @@ __FBSDID("$FreeBSD$"); #include <sys/filio.h> #include <sys/time.h> +#include <assert.h> #include <ctype.h> #include <err.h> #include <errno.h> @@ -76,6 +77,7 @@ STAT st; /* statistics */ void (*cfunc)(void); /* conversion function */ uintmax_t cpy_cnt; /* # of blocks to copy */ static off_t pending = 0; /* pending seek if sparse */ +static off_t last_sp = 0; /* size of last added sparse block */ u_int ddflags = 0; /* conversion options */ size_t cbsz; /* conversion block size */ uintmax_t files_cnt = 1; /* # of files to copy */ @@ -173,6 +175,8 @@ setup(void) } else if ((in.db = malloc(MAX(in.dbsz, cbsz) + cbsz)) == NULL || (out.db = malloc(out.dbsz + cbsz)) == NULL) err(1, "output buffer"); + + /* dbp is the first free position in each buffer. */ in.dbp = in.db; out.dbp = out.db; @@ -434,8 +438,15 @@ dd_out(int force) * we play games with the buffer size, and it's usually a partial write. */ outp = out.db; + + /* + * If force, first try to write all pending data, else try to write + * just one block. Subsequently always write data one full block at + * a time at most. + */ for (n = force ? out.dbcnt : out.dbsz;; n = out.dbsz) { - for (cnt = n;; cnt -= nw) { + cnt = n; + do { sparse = 0; if (ddflags & C_SPARSE) { sparse = 1; /* Is buffer sparse? */ @@ -447,18 +458,24 @@ dd_out(int force) } if (sparse && !force) { pending += cnt; + last_sp = cnt; nw = cnt; } else { if (pending != 0) { - if (force) - pending--; + /* If forced to write, and we have no + * data left, we need to write the last + * sparse block explicitly. + */ + if (force && cnt == 0) { + pending -= last_sp; + assert(outp == out.db); + memset(outp, 0, cnt); + } if (lseek(out.fd, pending, SEEK_CUR) == -1) err(2, "%s: seek error creating sparse file", out.name); - if (force) - write(out.fd, outp, 1); - pending = 0; + pending = last_sp = 0; } if (cnt) nw = write(out.fd, outp, cnt); @@ -473,27 +490,29 @@ dd_out(int force) err(1, "%s", out.name); nw = 0; } + outp += nw; st.bytes += nw; - if ((size_t)nw == n) { - if (n != out.dbsz) - ++st.out_part; - else - ++st.out_full; - break; - } - ++st.out_part; - if ((size_t)nw == cnt) - break; - if (out.flags & ISTAPE) - errx(1, "%s: short write on tape device", - out.name); - if (out.flags & ISCHR && !warned) { - warned = 1; - warnx("%s: short write on character device", - out.name); + + if ((size_t)nw == n && n == out.dbsz) + ++st.out_full; + else + ++st.out_part; + + if ((size_t) nw != cnt) { + if (out.flags & ISTAPE) + errx(1, "%s: short write on tape device", + out.name); + if (out.flags & ISCHR && !warned) { + warned = 1; + warnx("%s: short write on character device", + out.name); + } } - } + + cnt -= nw; + } while (cnt != 0); + if ((out.dbcnt -= n) < out.dbsz) break; } _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: thomas Date: Wed May 21 07:42:42 2014 New Revision: 266489 URL: http://svnweb.freebsd.org/changeset/base/266489 Log: MFC rev. 265593: (dd_out): Fix handling of all-zeroes block at end of input with conv=sparse. PR: bin/189174 PR: bin/189284 Reviewed by: kib Modified: stable/9/bin/dd/dd.c Directory Properties: stable/9/ (props changed) stable/9/bin/ (props changed) stable/9/bin/dd/ (props changed) Modified: stable/9/bin/dd/dd.c ============================================================================== --- stable/9/bin/dd/dd.c Wed May 21 07:21:36 2014 (r266488) +++ stable/9/bin/dd/dd.c Wed May 21 07:42:42 2014 (r266489) @@ -52,6 +52,7 @@ __FBSDID("$FreeBSD$"); #include <sys/filio.h> #include <sys/time.h> +#include <assert.h> #include <ctype.h> #include <err.h> #include <errno.h> @@ -76,6 +77,7 @@ STAT st; /* statistics */ void (*cfunc)(void); /* conversion function */ uintmax_t cpy_cnt; /* # of blocks to copy */ static off_t pending = 0; /* pending seek if sparse */ +static off_t last_sp = 0; /* size of last added sparse block */ u_int ddflags = 0; /* conversion options */ size_t cbsz; /* conversion block size */ uintmax_t files_cnt = 1; /* # of files to copy */ @@ -173,6 +175,8 @@ setup(void) } else if ((in.db = malloc(MAX(in.dbsz, cbsz) + cbsz)) == NULL || (out.db = malloc(out.dbsz + cbsz)) == NULL) err(1, "output buffer"); + + /* dbp is the first free position in each buffer. */ in.dbp = in.db; out.dbp = out.db; @@ -434,8 +438,15 @@ dd_out(int force) * we play games with the buffer size, and it's usually a partial write. */ outp = out.db; + + /* + * If force, first try to write all pending data, else try to write + * just one block. Subsequently always write data one full block at + * a time at most. + */ for (n = force ? out.dbcnt : out.dbsz;; n = out.dbsz) { - for (cnt = n;; cnt -= nw) { + cnt = n; + do { sparse = 0; if (ddflags & C_SPARSE) { sparse = 1; /* Is buffer sparse? */ @@ -447,18 +458,24 @@ dd_out(int force) } if (sparse && !force) { pending += cnt; + last_sp = cnt; nw = cnt; } else { if (pending != 0) { - if (force) - pending--; + /* If forced to write, and we have no + * data left, we need to write the last + * sparse block explicitly. + */ + if (force && cnt == 0) { + pending -= last_sp; + assert(outp == out.db); + memset(outp, 0, cnt); + } if (lseek(out.fd, pending, SEEK_CUR) == -1) err(2, "%s: seek error creating sparse file", out.name); - if (force) - write(out.fd, outp, 1); - pending = 0; + pending = last_sp = 0; } if (cnt) nw = write(out.fd, outp, cnt); @@ -473,27 +490,29 @@ dd_out(int force) err(1, "%s", out.name); nw = 0; } + outp += nw; st.bytes += nw; - if ((size_t)nw == n) { - if (n != out.dbsz) - ++st.out_part; - else - ++st.out_full; - break; - } - ++st.out_part; - if ((size_t)nw == cnt) - break; - if (out.flags & ISTAPE) - errx(1, "%s: short write on tape device", - out.name); - if (out.flags & ISCHR && !warned) { - warned = 1; - warnx("%s: short write on character device", - out.name); + + if ((size_t)nw == n && n == out.dbsz) + ++st.out_full; + else + ++st.out_part; + + if ((size_t) nw != cnt) { + if (out.flags & ISTAPE) + errx(1, "%s: short write on tape device", + out.name); + if (out.flags & ISCHR && !warned) { + warned = 1; + warnx("%s: short write on character device", + out.name); + } } - } + + cnt -= nw; + } while (cnt != 0); + if ((out.dbcnt -= n) < out.dbsz) break; } _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
A commit references this bug: Author: thomas Date: Thu Feb 18 08:44:17 UTC 2016 New revision: 295749 URL: https://svnweb.freebsd.org/changeset/base/295749 Log: Reorganize the handling all-zeroes terminal block in sparse mode The intent of the previous code in that case was to force an explicit write, but the implementation was incorrect, and as a result the write was never performed. This new implementation instead uses ftruncate(2) to extend the file with a trailing hole. Also introduce regression tests for these cases. PR: 189284 (original PR whose fix introduced this bug) PR: 207092 Differential Revision: D5248 Reviewed by: sobomax,kib MFC after: 2 weeks Changes: head/bin/dd/Makefile head/bin/dd/dd.c head/bin/dd/dd.h head/bin/dd/gen.c head/bin/dd/ref.obs_zeroes
A commit references this bug: Author: thomas Date: Tue Apr 19 07:34:32 UTC 2016 New revision: 298258 URL: https://svnweb.freebsd.org/changeset/base/298258 Log: MFC r295749: Reorganize the handling all-zeroes terminal block in sparse mode PR: 189284 (original PR whose fix introduced this bug) PR: 207092 Changes: _U stable/10/ stable/10/bin/dd/Makefile stable/10/bin/dd/dd.c stable/10/bin/dd/dd.h stable/10/bin/dd/gen.c stable/10/bin/dd/ref.obs_zeroes