diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2022-03-28 13:36:36 +0300 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2022-03-28 13:36:36 +0300 |
commit | 303448bc912486f4766129cc407a5077a3ca4359 (patch) | |
tree | 5741d36c3b700147248a57ca7f606c243186f42f | |
parent | 7af133cc111c0fa8f5afa99055d2a22eaac6d94e (diff) | |
download | mariadb-git-303448bc912486f4766129cc407a5077a3ca4359.tar.gz |
MDEV-27931: buf_page_is_corrupted() wrongly claims corruption
In commit 437da7bc54daa131b46900128ebe3ad2ca25c11a (MDEV-19534),
the default value of the global variable srv_checksum_algorithm
in innochecksum was changed from SRV_CHECKSUM_ALGORITHM_INNODB
to implied 0 (innodb_checksum_algorithm=crc32). As a result,
the function buf_page_is_corrupted() would by default invoke
buf_calc_page_crc32() in innochecksum, and crc32_inited would hold.
This would cause "innochecksum" to fail on a particular page.
The actual problem is older, introduced in 2011 in
mysql/mysql-server@17e497bdb793bc6b8360aa1c626dcd8bb5cfad1b
(MySQL 5.6.3). It should affect the validation of pages of old
data files that were written with innodb_checksum_algorithm=innodb.
When using innodb_checksum_algorithm=crc32 (the default setting
since MariaDB Server 10.2), some valid pages would be rejected
only because exactly one of the two checksum fields accidentally
matches the innodb_checksum_algorithm=crc32 value.
buf_page_is_corrupted(): Simplify the logic of non-strict
checksum validation, by always invoking buf_calc_page_crc32().
Remove a bogus condition that if only one of the checksum fields
contains the value returned by buf_calc_page_crc32(), the page
is corrupted.
-rw-r--r-- | storage/innobase/buf/buf0buf.cc | 96 |
1 files changed, 24 insertions, 72 deletions
diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index 505539f0217..679f12ca6d1 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -977,8 +977,6 @@ buf_page_is_corrupted( #endif size_t checksum_field1 = 0; size_t checksum_field2 = 0; - uint32_t crc32 = 0; - bool crc32_inited = false; ulint page_type = mach_read_from_2(read_buf + FIL_PAGE_TYPE); @@ -1104,8 +1102,13 @@ buf_page_is_corrupted( case SRV_CHECKSUM_ALGORITHM_STRICT_NONE: return !buf_page_is_checksum_valid_none( read_buf, checksum_field1, checksum_field2); + case SRV_CHECKSUM_ALGORITHM_NONE: + /* should have returned false earlier */ + break; case SRV_CHECKSUM_ALGORITHM_CRC32: case SRV_CHECKSUM_ALGORITHM_INNODB: + const uint32_t crc32 = buf_calc_page_crc32(read_buf); + if (buf_page_is_checksum_valid_none(read_buf, checksum_field1, checksum_field2)) { #ifdef UNIV_INNOCHECKSUM @@ -1121,7 +1124,7 @@ buf_page_is_corrupted( " crc32 = " UINT32PF "; recorded = " ULINTPF ";\n", cur_page_num, buf_calc_page_new_checksum(read_buf), - buf_calc_page_crc32(read_buf), + crc32, checksum_field1); } #endif /* UNIV_INNOCHECKSUM */ @@ -1138,85 +1141,34 @@ buf_page_is_corrupted( != mach_read_from_4(read_buf + FIL_PAGE_LSN) && checksum_field2 != BUF_NO_CHECKSUM_MAGIC) { - if (curr_algo == SRV_CHECKSUM_ALGORITHM_CRC32) { - DBUG_EXECUTE_IF( - "page_intermittent_checksum_mismatch", { - static int page_counter; - if (page_counter++ == 2) { - checksum_field2++; - } - }); - - crc32 = buf_page_check_crc32(read_buf, - checksum_field2); - crc32_inited = true; - - if (checksum_field2 != crc32 - && checksum_field2 - != buf_calc_page_old_checksum(read_buf)) { - return true; - } - } else { - ut_ad(curr_algo - == SRV_CHECKSUM_ALGORITHM_INNODB); - - if (checksum_field2 - != buf_calc_page_old_checksum(read_buf)) { - crc32 = buf_page_check_crc32( - read_buf, checksum_field2); - crc32_inited = true; - - if (checksum_field2 != crc32) { - return true; - } - } + DBUG_EXECUTE_IF( + "page_intermittent_checksum_mismatch", { + static int page_counter; + if (page_counter++ == 2) return true; + }); + + if ((checksum_field1 != crc32 + || checksum_field2 != crc32) + && checksum_field2 + != buf_calc_page_old_checksum(read_buf)) { + return true; } } - if (checksum_field1 == 0 - || checksum_field1 == BUF_NO_CHECKSUM_MAGIC) { - } else if (curr_algo == SRV_CHECKSUM_ALGORITHM_CRC32) { - if (!crc32_inited) { - crc32 = buf_page_check_crc32( - read_buf, checksum_field2); - crc32_inited = true; - } - - if (checksum_field1 != crc32 + switch (checksum_field1) { + case 0: + case BUF_NO_CHECKSUM_MAGIC: + break; + default: + if ((checksum_field1 != crc32 + || checksum_field2 != crc32) && checksum_field1 != buf_calc_page_new_checksum(read_buf)) { return true; } - } else { - ut_ad(curr_algo == SRV_CHECKSUM_ALGORITHM_INNODB); - - if (checksum_field1 - != buf_calc_page_new_checksum(read_buf)) { - - if (!crc32_inited) { - crc32 = buf_page_check_crc32( - read_buf, checksum_field2); - crc32_inited = true; - } - - if (checksum_field1 != crc32) { - return true; - } - } - } - - if (crc32_inited - && ((checksum_field1 == crc32 - && checksum_field2 != crc32) - || (checksum_field1 != crc32 - && checksum_field2 == crc32))) { - return true; } break; - case SRV_CHECKSUM_ALGORITHM_NONE: - /* should have returned false earlier */ - break; } return false; |