Thirunarayanan Balathandayuthapani added a comment - 2018-12-10 12:56 Patch is present in bb-10.1-thiru https://github.com/MariaDB/server/commit/6c8538ac219cd9248ddaaaa59251de937d354b79

Marko Mäkelä added a comment - 2018-12-11 08:10 Thanks! I think that we can simplify this a little more by assuming that when the crc32 checksum is being used, it has been written by MySQL 5.6.3 or later. Please submit a 10.0 version as well, and observe my review comments in GitHub. For the 10.1 version, please implement strict checksum variants also for fil_space_verify_crypt_checksum() .

Thirunarayanan Balathandayuthapani added a comment - 2018-12-11 10:57 Put the patch for 10.0 and addressed the review comments for 10.1 patch as well. Both are present in bb-10.0-thiru and bb-10.1-thiru

Thirunarayanan Balathandayuthapani added a comment - 2018-12-13 06:32 In 10.2, there are few dead code: bool buf_page_is_checksum_valid_crc32( const byte* read_buf, ulint checksum_field1, ulint checksum_field2, bool use_legacy_big_endian) { const uint32_t crc32 = buf_calc_page_crc32(read_buf, use_legacy_big_endian);     if (checksum_field1 != checksum_field2) { goto invalid; }   if (checksum_field1 == crc32) { return(true); } else { const uint32_t crc32_legacy = buf_calc_page_crc32(read_buf, true);   if (checksum_field1 == crc32_legacy) { return(true); } }   It checks for both little endian and big endian variant of crc32 algorithm.   But in buf_page_is_corrupted():   if (buf_page_is_checksum_valid_crc32(read_buf, checksum_field1, checksum_field2, false)) { return(false); }   ............... .................   /* We need to check whether the stored checksum matches legacy big endian checksum or Innodb checksum. We optimize the order based on earlier results. if earlier we have found pages matching legacy big endian checksum, we try to match it first. Otherwise we check innodb checksum first. */ if (legacy_big_endian_checksum) { if (buf_page_is_checksum_valid_crc32(read_buf, checksum_field1, checksum_field2, true)) {   return(false); } legacy_checksum_checked = true; }   .............. ..............   /* If legacy checksum is not checked, do it now. */ if (!legacy_checksum_checked && buf_page_is_checksum_valid_crc32( read_buf, checksum_field1, checksum_field2, true)) {   legacy_big_endian_checksum = true; return(false); }   Above code can never be executed because these condition already checked as a part of buf_page_is_checksum_valid_crc32(). Another issue with page_zip_verify_checksum(): /* We need to check whether the stored checksum matches legacy big endian checksum or Innodb checksum. We optimize the order based on earlier results. if earlier we have found pages matching legacy big endian checksum, we try to match it first. Otherwise we check innodb checksum first. */ if (legacy_big_endian_checksum) { const uint32_t calculated = page_zip_calc_checksum(data, size, curr_algo, true); if (stored == calculated) {   return(TRUE); } legacy_checksum_checked = true; }   ............ .............   /* If legacy checksum is not checked, do it now. */ if ((legacy_checksum_checked && stored == calculated)) { legacy_big_endian_checksum = true; return(TRUE); }   It is also kind of dead code because it should check if (!legacy_checksum_checked). may be merging error from mysql-5.7

Thirunarayanan Balathandayuthapani added a comment - 2018-12-13 06:33 Fixed the above issues in bb-10.2-23578 branch.

Marko Mäkelä added a comment - 2018-12-13 14:00 I merged this up to 10.2 until now.