[DRBD-user] Warning: Data Corruption Issue Discovered in DRBD 8.4 and 9.0

[Robert L]

Greetings, I am a new member on this list and I've been looking into
this problem also.  I understand that this is not a bug in DRBD, but
that the resource usage patterns of DRBD can affect the outcome of this
trimtester program.  My main goal is to eliminate false positives.

I've implemented a different strategy for checking corruption with only
some minor changes to trimtester.cpp:

By ensuring that no zero bytes are written to the files, checking for
corruption can be detected by looking for a single zero byte.  I also
corrected some problems with the buffer overrun in writeAtomically().

Since many of the original problems were pointed out on this list, I
post the differences here for anyone who might be interested.

Comments graciously requested and accepted.

This is a diff with the original trimtester.cpp forked from Algoria's repo:

$ git diff 48c44d5beb88c56e003f776d79ab30781fa90c8e 
97f7c179e5220c94ccc486fe3f49c21ad9c4a8c3 trimtester.cpp
diff --git a/trimtester.cpp b/trimtester.cpp
index d753741..2930683 100644
--- a/trimtester.cpp
+++ b/trimtester.cpp
@@ -217,25 +217,14 @@ private:
         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) {
-                if (mmap.len() - i > 4) { // only check page > 4-bytes
to avoid false positive
-                    bool pagecorrupted = true;
-                    for (unsigned j = i; j < mmap.len() && j < (i +
512); ++j) {                   
-                        if (mmap.content()[j] != 0)
-                            pagecorrupted = false;
-                    }
-                    if (pagecorrupted)
-                        corrupted = true;
-
+            for (const char *p = mmap.content(); p < mmap.content() +
mmap.len(); p++) {
+                if (*p == 0) {
+                    // no zero bytes should be in file because none
were written
+                    std::cerr << "Corrupted file found: " << filename
+                        << ", zero byte in file at byte address: " << p
- mmap.content() <<std::endl;
+                    exit(1);
                 }
             }
-            if (corrupted) {
-                std::cerr << "Corrupted file found: " << filename <<
std::endl;
-                exit(1);
-            }
-
         }
     }
 
@@ -266,19 +255,21 @@ void writeAtomically(const std::string &dataDir,
unsigned folder, unsigned file,
     std::string destFile = ss.str();
     ss << ".tmp";
     std::string tmpFile = ss.str();
-    unsigned char buff[65535];
+    unsigned char buff[64*1024];
 
-    for (unsigned i = 0; i < 65536; ++i) {
+    for (unsigned i = 0; i < sizeof(buff); ++i) {
         buff[i] = (seed + i) % 256;
+        if (buff[i] == 0)
+            buff[i] = 0xff;     // no zero bytes will be written
     }
     { // write file1.bin.tmp
         FILE *file = fopen(tmpFile.c_str(), "wb");
         assert(file != NULL);
-        uint64_t sizeToWrite = size;
+        int64_t sizeToWrite = size; // because uint64_t is ALWAYS >= zero
 
         while (sizeToWrite > 0) {
-            uint64_t toWrite = (sizeToWrite > 65536 ? 65536 : sizeToWrite);
-            uint64_t nb = fwrite(buff, 1, toWrite, file);
+            size_t toWrite = (sizeToWrite > sizeof(buff) ? sizeof(buff)
: sizeToWrite);
+            size_t nb = fwrite(buff, 1, toWrite, file);
             if (nb != toWrite) {
                 std::cerr << "Disk full, rm folder & restart the test"
<< std::endl;
                 exit(0);
$

This code can be found at https://github.com/artisan-digital/trimtester
There were other mods in between the fork and this version.

Thanks for reading,
Robert L