commit a9b20ae6a38073afe91ae2b7d789ddfb7dabade8
Author: Tim Burke <tim.burke@gmail.com>
Date:   Thu Jul 6 00:21:01 2017 +0000

    Use zlib for CRC-32
    
    Previously, we had our own CRC that was almost but not quite like
    zlib's implementation. However,
    
    * it hasn't been subjected to the same rigor with regard to error-detection
      properties and
    * it may not even get used, depending upon whether zlib happens to get
      loaded before or after liberasurecode.
    
    Now, we'll use zlib's CRC-32 when writing new frags, while still
    tolerating frags that were created with the old implementation.
    
    Change-Id: Ib5ea2a830c7c23d66bf2ca404a3eb84ad00c5bc5
    Closes-Bug: 1666320

diff --git a/bindep.txt b/bindep.txt
index aaef309..7593290 100644
--- a/bindep.txt
+++ b/bindep.txt
@@ -1,6 +1,9 @@
 # This is a cross-platform list tracking distribution packages needed by tests;
 # see http://docs.openstack.org/infra/bindep/ for additional information.
 
+zlib1g-dev [platform:dpkg]
+zlib-devel [platform:rpm]
+
 build-essential [platform:dpkg]
 gcc [platform:rpm]
 make [platform:rpm]
diff --git a/erasurecode.pc.in b/erasurecode.pc.in
index ee6d82b..175367c 100644
--- a/erasurecode.pc.in
+++ b/erasurecode.pc.in
@@ -11,5 +11,5 @@ Version: @LIBERASURECODE_VERSION@
 Requires:
 Conflicts:
 Libs: -L${libdir} -lerasurecode
-Libs.private: @ERASURECODE_STATIC_LIBS@
+Libs.private: @ERASURECODE_STATIC_LIBS@ -lz
 Cflags: -I${includedir}/ -I${includedir}/liberasurecode
diff --git a/include/erasurecode/alg_sig.h b/include/erasurecode/alg_sig.h
index 52554a9..e250fb3 100644
--- a/include/erasurecode/alg_sig.h
+++ b/include/erasurecode/alg_sig.h
@@ -57,8 +57,7 @@ alg_sig_t *init_alg_sig(int sig_len, int gf_w);
 void destroy_alg_sig(alg_sig_t* alg_sig_handle);
 
 int compute_alg_sig(alg_sig_t* alg_sig_handle, char *buf, int len, char *sig);
-int crc32_build_fast_table();
-int crc32(int crc, const void *buf, int size);
+int liberasurecode_crc32_alt(int crc, const void *buf, int size);
 
 #endif
 
diff --git a/src/Makefile.am b/src/Makefile.am
index 8312dd0..693809e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -34,9 +34,10 @@ liberasurecode_la_SOURCES = \
 
 liberasurecode_la_CPPFLAGS = -Werror @GCOV_FLAGS@
 liberasurecode_la_LIBADD = \
-		builtin/null_code/libnullcode.la -lpthread -lm @GCOV_LDFLAGS@ \
-		builtin/xor_codes/libXorcode.la -lpthread -lm @GCOV_LDFLAGS@ \
-		builtin/rs_vand/liberasurecode_rs_vand.la -lpthread -lm @GCOV_LDFLAGS@
+		builtin/null_code/libnullcode.la \
+		builtin/xor_codes/libXorcode.la \
+		builtin/rs_vand/liberasurecode_rs_vand.la \
+		-lpthread -lm -lz @GCOV_LDFLAGS@
 
 # Version format  (C - A).(A).(R) for C:R:A input
 liberasurecode_la_LDFLAGS = -rpath '$(libdir)' -version-info @LIBERASURECODE_VERSION_INFO@
diff --git a/src/erasurecode.c b/src/erasurecode.c
index d4a06c2..20da457 100644
--- a/src/erasurecode.c
+++ b/src/erasurecode.c
@@ -26,6 +26,7 @@
  * vi: set noai tw=79 ts=4 sw=4:
  */
 
+#include <zlib.h>
 #include "assert.h"
 #include "list.h"
 #include "erasurecode.h"
@@ -1063,9 +1064,17 @@ int liberasurecode_get_fragment_metadata(char *fragment,
             uint32_t stored_chksum = fragment_hdr->meta.chksum[0];
             char *fragment_data = get_data_ptr_from_fragment(fragment);
             uint64_t fragment_size = fragment_hdr->meta.size;
-            computed_chksum = crc32(0, fragment_data, fragment_size);
+            computed_chksum = crc32(0, (unsigned char *) fragment_data, fragment_size);
             if (stored_chksum != computed_chksum) {
-                fragment_metadata->chksum_mismatch = 1;
+                // Try again with our "alternative" crc32; see
+                // https://bugs.launchpad.net/liberasurecode/+bug/1666320
+                computed_chksum = liberasurecode_crc32_alt(
+                    0, fragment_data, fragment_size);
+                if (stored_chksum != computed_chksum) {
+                    fragment_metadata->chksum_mismatch = 1;
+                } else {
+                    fragment_metadata->chksum_mismatch = 0;
+                }
             } else {
                 fragment_metadata->chksum_mismatch = 0;
             }
@@ -1095,7 +1104,13 @@ int is_invalid_fragment_header(fragment_header_t *header)
     stored_csum = get_metadata_chksum((char *) header);
     if (NULL == stored_csum)
         return 1; /* can't verify, possibly crc32 call error */
-    csum = crc32(0, &header->meta, sizeof(fragment_metadata_t));
+    csum = crc32(0, (unsigned char *) &header->meta, sizeof(fragment_metadata_t));
+    if (*stored_csum == csum) {
+        return 0;
+    }
+    // Else, try again with our "alternative" crc32; see
+    // https://bugs.launchpad.net/liberasurecode/+bug/1666320
+    csum = liberasurecode_crc32_alt(0, &header->meta, sizeof(fragment_metadata_t));
     return (*stored_csum != csum);
 }
 
diff --git a/src/erasurecode_helpers.c b/src/erasurecode_helpers.c
index fd14298..4a49786 100644
--- a/src/erasurecode_helpers.c
+++ b/src/erasurecode_helpers.c
@@ -28,6 +28,7 @@
 #include <assert.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <zlib.h>
 #include "erasurecode_backend.h"
 #include "erasurecode_helpers.h"
 #include "erasurecode_helpers_ext.h"
@@ -474,7 +475,7 @@ inline int set_checksum(ec_checksum_type_t ct, char *buf, int blocksize)
 
     switch(header->meta.chksum_type) {
         case CHKSUM_CRC32:
-            header->meta.chksum[0] = crc32(0, data, blocksize);
+            header->meta.chksum[0] = crc32(0, (unsigned char *) data, blocksize);
             break;
         case CHKSUM_MD5:
             break;
@@ -512,7 +513,7 @@ inline int set_metadata_chksum(char *buf)
         return -1;
     }
 
-    header->metadata_chksum = crc32(0, &header->meta,
+    header->metadata_chksum = crc32(0, (unsigned char *) &header->meta,
                                     sizeof(fragment_metadata_t));
     return 0;
 }
diff --git a/src/utils/chksum/crc32.c b/src/utils/chksum/crc32.c
index 6bc844d..b11dec9 100644
--- a/src/utils/chksum/crc32.c
+++ b/src/utils/chksum/crc32.c
@@ -89,7 +89,7 @@ static int crc32_tab[] = {
 };
 
 int
-crc32(int crc, const void *buf, size_t size)
+liberasurecode_crc32_alt(int crc, const void *buf, size_t size)
 {
   const char *p;
 
diff --git a/test/Makefile.am b/test/Makefile.am
index d8ffa79..2e33bdc 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -15,7 +15,7 @@ check_PROGRAMS += alg_sig_test
 
 liberasurecode_test_SOURCES = liberasurecode_test.c
 liberasurecode_test_CPPFLAGS = -I$(top_srcdir)/include -I$(top_srcdir)/include/erasurecode  @GCOV_FLAGS@
-liberasurecode_test_LDFLAGS = @GCOV_LDFLAGS@ $(top_builddir)/src/liberasurecode.la -ldl -lpthread
+liberasurecode_test_LDFLAGS = @GCOV_LDFLAGS@ $(top_builddir)/src/liberasurecode.la -ldl -lpthread -lz
 check_PROGRAMS += liberasurecode_test
 
 libec_slap_SOURCES = libec_slap.c
diff --git a/test/liberasurecode_test.c b/test/liberasurecode_test.c
index 4791ca9..68c1c13 100644
--- a/test/liberasurecode_test.c
+++ b/test/liberasurecode_test.c
@@ -28,6 +28,7 @@
 
 #include <assert.h>
 #include <stdbool.h>
+#include <zlib.h>
 #include "erasurecode.h"
 #include "erasurecode_helpers.h"
 #include "erasurecode_helpers_ext.h"
@@ -475,7 +476,7 @@ static void validate_fragment_checksum(struct ec_args *args,
             assert(false); //currently only have support crc32
             break;
         case CHKSUM_CRC32:
-            computed = crc32(0, fragment_data, size);
+            computed = crc32(0, (unsigned char *) fragment_data, size);
             break;
         case CHKSUM_NONE:
             assert(metadata->chksum_mismatch == 0);
@@ -1745,6 +1746,35 @@ static void test_verify_stripe_metadata_frag_idx_invalid(
     verify_fragment_metadata_mismatch_impl(be_id, args, FRAGIDX_OUT_OF_RANGE);
 }
 
+static void test_metadata_crcs()
+{
+    // We've observed headers like this in the wild, using our busted crc32
+    char header[] =
+        "\x03\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x10\x00"
+        "\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+        "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+        "\x00\x00\x00\x00\x00\x00\x07\x01\x0e\x02\x00\xcc\x5e\x0c\x0b\x00"
+        "\x04\x01\x00\x22\xee\x45\xb9\x00\x00\x00\x00\x00\x00\x00\x00\x00";
+
+    fragment_metadata_t res;
+
+    assert(liberasurecode_get_fragment_metadata(header, &res) == 0);
+    assert(is_invalid_fragment_header((fragment_header_t *) header) == 0);
+
+    // Switch it to zlib's implementation
+    header[70] = '\x18';
+    header[69] = '\x73';
+    header[68] = '\xf8';
+    header[67] = '\xec';
+
+    assert(liberasurecode_get_fragment_metadata(header, &res) == 0);
+    assert(is_invalid_fragment_header((fragment_header_t *) header) == 0);
+
+    // Write down the wrong thing
+    header[70] = '\xff';
+    assert(liberasurecode_get_fragment_metadata(header, &res) == -EBADHEADER);
+    assert(is_invalid_fragment_header((fragment_header_t *) header) == 1);
+}
 
 //static void test_verify_str
 
@@ -1805,6 +1835,10 @@ struct testcase testcases[] = {
         test_liberasurecode_get_version,
         EC_BACKENDS_MAX, CHKSUM_TYPES_MAX,
         .skip = false},
+    {"test_metadata_crcs",
+        test_metadata_crcs,
+        EC_BACKENDS_MAX, 0,
+        .skip = false},
     // NULL backend test
     {"create_and_destroy_backend",
         test_create_and_destroy_backend,