Since the previous patch renabled direct I/O that was disabled on most (file) systems in coreutils 6.0, it reactivates a bug that was present since 5.3.0 when direct I/O was added.
I.E. there can now be an odd sized direct I/O write at the end of file, which will trigger an EINVAL write error which is not currently handled. The attached patch should fix it up. thanks, Pádraig.
>From 94e3de559d5366630ae350dea53719d95d1ffee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Thu, 7 Nov 2013 13:26:25 +0000 Subject: [PATCH] shred: fix direct I/O failures for last write to file Since direct I/O is now enabled with commit v8.21-139-gebaf961 we must handle the case where we write an odd size at the end of a file (with --exact), or we specify an odd --size that is larger than 64KiB, or in the very unlikely case of a device with an odd size. This issue was present since direct I/O support was first added in v5.3.0, but latent since v6.0. Theoretically this could have also been an issue after that on systems which didn't have alignment constraints, but did have size constraints for direct I/O. * src/shred.c (dopass): On the first pass for a file, always retry a write that fails with EINVAL, so we handle direct I/O failure at either the start or end of the file. Adjust the comment as the original case is out of date and implicitly handled by this more general fix. * tests/misc/shred-exact.sh: Add a test case. * NEWS: Add a "bug fix" entry for shred since there are two related issues now fixed. --- NEWS | 10 +++++++--- src/shred.c | 21 ++++++++++----------- tests/misc/shred-exact.sh | 11 ++++++++++- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/NEWS b/NEWS index bb35583..f36f7bc 100644 --- a/NEWS +++ b/NEWS @@ -37,6 +37,11 @@ GNU coreutils NEWS -*- outline -*- rm -I now prompts for confirmation before removing a write protected file. [Bug introduced in coreutils-6.8] + shred once again uses direct I/O on systems requiring aligned buffers. + Also direct I/O failures for odd sized writes at end of file are now handled. + [The "last write" bug was introduced in coreutils-5.3.0 but masked + by the alignment bug introduced in coreutils-6.0] + tail --retry -f now waits for the files specified to appear. Before, tail would immediately exit when such a file is inaccessible during the initial open. @@ -87,9 +92,8 @@ GNU coreutils NEWS -*- outline -*- Reservoir sampling is used to limit memory usage based on the number of outputs, rather than the number of inputs. - shred once again uses direct I/O where available, and increases write block - size from 12KiB to 64KiB when possible. - [Direct I/O regression introduced in coreutils-6.0] + shred increases write block size from 12KiB to 64KiB when possible, + to align with other utilities and reduce the system call overhead. split --line-bytes=SIZE, now only allocates memory as needed rather than allocating SIZE bytes at program start. diff --git a/src/shred.c b/src/shred.c index 9ff7238..98dd872 100644 --- a/src/shred.c +++ b/src/shred.c @@ -394,7 +394,7 @@ dopass (int fd, char const *qname, off_t *sizep, int type, char pass_string[PASS_NAME_SIZE]; /* Name of current pass */ bool write_error = false; bool other_error = false; - bool first_write = true; + bool tried_without_directio = false; /* Printable previous offset into the file */ char previous_offset_buf[LONGEST_HUMAN_READABLE + 1]; @@ -443,7 +443,7 @@ dopass (int fd, char const *qname, off_t *sizep, int type, if (type < 0) randread (s, pbuf, lim); /* Loop to retry partial writes. */ - for (soff = 0; soff < lim; soff += ssize, first_write = false) + for (soff = 0; soff < lim; soff += ssize) { ssize = write (fd, pbuf + soff, lim - soff); if (ssize <= 0) @@ -459,17 +459,15 @@ dopass (int fd, char const *qname, off_t *sizep, int type, int errnum = errno; char buf[INT_BUFSIZE_BOUND (uintmax_t)]; - /* If the first write of the first pass for a given file - has just failed with EINVAL, turn off direct mode I/O - and try again. This works around a bug in Linux kernel - 2.4 whereby opening with O_DIRECT would succeed for some - file system types (e.g., ext3), but any attempt to - access a file through the resulting descriptor would - fail with EINVAL. */ - if (k == 1 && first_write && errno == EINVAL) + /* Retry without direct I/O since this may not be supported + at all on some (file) systems, or with the current size. + I.E. a specified --size that is not aligned, or when + dealing with slop at the end of a file with --exact. */ + if (k == 1 && !tried_without_directio && errno == EINVAL) { direct_mode (fd, false); ssize = 0; + tried_without_directio = true; continue; } error (0, errnum, _("%s: error writing at offset %s"), @@ -478,7 +476,8 @@ dopass (int fd, char const *qname, off_t *sizep, int type, /* 'shred' is often used on bad media, before throwing it out. Thus, it shouldn't give up on bad blocks. This code works because lim is always a multiple of - SECTOR_SIZE, except at the end. */ + SECTOR_SIZE, except at the end. This size constraint + also enables direct I/O on some (file) systems. */ verify (PERIODIC_OUTPUT_SIZE % SECTOR_SIZE == 0); verify (NONPERIODIC_OUTPUT_SIZE % SECTOR_SIZE == 0); if (errnum == EIO && 0 <= size && (soff | SECTOR_MASK) < lim) diff --git a/tests/misc/shred-exact.sh b/tests/misc/shred-exact.sh index 0cdc91f..eb30a7d 100755 --- a/tests/misc/shred-exact.sh +++ b/tests/misc/shred-exact.sh @@ -1,5 +1,5 @@ #!/bin/sh -# make sure that neither --exact nor --zero gobbles a command line argument +# Test functionality of --exact # Copyright (C) 2000-2013 Free Software Foundation, Inc. @@ -20,6 +20,7 @@ print_ver_ shred +# make sure that neither --exact nor --zero gobbles a command line argument for opt in --exact --zero; do echo a > a || fail=1 echo bb > b || fail=1 @@ -33,4 +34,12 @@ for opt in --exact --zero; do test -f c && fail=1 done + +# make sure direct I/O is handled appropriately at end of file +# Create a 1MiB file as we'll probably not be using blocks larger than that +# (i.e. we want to test failed writes not at the start). +truncate -s1MiB file.slop || framework_failure_ +truncate -s+1 file.slop || framework_failure_ +shred --exact -n1 file.slop || fail=1 + Exit $fail -- 1.7.7.6
