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 #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 #include #include +#include #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 #include +#include #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,