Note: "permalinks" may not be as permanent as we would like,
direct links of old sources may well be a few messages off.
On Thu, Oct 12, 2017 at 11:14:55AM +0200, Robert Altnoeder wrote:
> On 10/11/2017 11:30 PM, Eric Robinson wrote:
> > The TrimTester program consists of three parts. The main executable
> > (TrimTester) just writes loads of data to the drive and tests for file
> > corruption. My C++ consultant says, "It writes sequential numbers
> > wrapped at 256, spanning multiple files. It checks previously written
> > files, and if the file data is all zeroes, it is considered to be
> > corrupted."
> Are you referring to this program?
> https://github.com/algolia/trimtester/blob/master/trimtester.cpp
>
> One thing that I can tell you right away is that this program does not
> appear to be very trustworthy, because it may malfunction due to the use
> of incorrect datatypes for the purpose - apparently, it is attempting to
> memory-map quite large files (~ 70 GiB) and check using a byte-indexed
> offset declared as type 'unsigned', which is commonly only 32 bits wide,
> and therefore inadequate for the byte-wise indexing of anything that is
> larger than 4 GiB.
The test program has other issues as well,
like off-by-one (and thus stack corruption) when initializing the
"buffer" in its "writeAtomically",
unlinking known non-existent files, and other things.
Probably harmless.
But this 32bit index vs >= 4 GiByte file content
is the real bug here, thank you Robert for pointing that out.
Why it does not trigger if DRBD is not in the stack I cannot tell,
maybe the timing is just strangely scewed, and somehow your disk fills
up and everything terminates before the "DetectCorruption" thread tries
to check a >= 4GiB file for the first time.
Anyways: what happens is:
void _checkFile(const std::string &path, const char *file, std::string &filename) {
filename.resize(0);
filename.append(path);
filename.push_back('/');
filename.append(file);
MMapedFile mmap(filename.c_str());
if (mmap.loaded()) {
bool corrupted = false;
// Detect all 512-bytes page inside the file filled by 0 -> can be caused by a buggy Trim
for (unsigned i = 0; !corrupted && i < mmap.len(); i += 512) {
// after some number of iterrations,
// i = 4294966784, 2 ** 32 - 512;
// mmap.len however is *larger*.
// in the "i << mmpa.len()", the 32bit integer i is "upscaled",
// size-extended, before the comparison, so that remains true.
if (mmap.len() - i > 4) { // only check page > 4-bytes to avoid false positive
// again, size-extension to 64bit, condition is true
bool pagecorrupted = true;
// *assume* that the "page" was corrupted,
for (unsigned j = i; j < mmap.len() && j < (i + 512); ++j) {
// j = i, which is j = 4294966784, (i << mmap.len) is again true because
// of the size-extension of i to 64bit in that term,
// but for the (j < i+ 512) term, neither j nor i is size-extended,
// i + 512 wraps to 0, j < 0 is false,
// loop will not execute even once,
// which means no single byte is checked
if (mmap.content()[j] != 0)
pagecorrupted = false;
}
if (pagecorrupted)
corrupted = true;
// any we "won" a "corrupted" flag by simply "assuming"
// no bytes are bad bytes.
// "So sad." ;-)
}
}
if (corrupted) {
std::cerr << "Corrupted file found: " << filename << std::endl;
exit(1);
}
}
}
Just change "unsigned" to "uint64_t" there, and be happy.
Don't believe it?
Create any file of 4 GiB or larger,
make sure it does not contain 512 (aligned) consecutive zeros,
and "check" it for "corruption" with that logic of trimtester.
It will report that file as corrupted each time.
rm -rf trimtester-is-broken/
mkdir trimtester-is-broken
o=trimtester-is-broken/x1
echo X > $o
l=$o
for i in `seq 2 32`; do
o=trimtester-is-broken/x$i;
cat $l $l > $o ;
rm -f $l;
l=$o;
done
./TrimTester trimtester-is-broken
Wahwahwa Corrupted file found: trimtester-is-broken/x32 mimimimi
Thanks,
that was a nice excercise in proofreading cpp code.
--
: Lars Ellenberg
: LINBIT | Keeping the Digital World Running
: DRBD -- Heartbeat -- Corosync -- Pacemaker
DRBD® and LINBIT® are registered trademarks of LINBIT
__
please don't Cc me, but send to list -- I'm subscribed