Commit 01b44c05 authored by Marko Mäkelä's avatar Marko Mäkelä

MDEV-25026 Various code paths are accessing freed pages

The test case encryption.innodb_encrypt_freed was failing in
MemorySanitizer builds.

recv_recover_page(): Mark non-recovered pages as freed.

fil_crypt_rotate_page(): Before comparing the block->frame contents,
check if the block was marked as freed.

Other places: Whenever using BUF_GET_POSSIBLY_FREED, check the
block->page.status before accessing the page frame.

(Both uses of BUF_GET_IF_IN_POOL should be correct now.)
parent 1f1f61a9
...@@ -807,11 +807,13 @@ btr_cur_optimistic_latch_leaves( ...@@ -807,11 +807,13 @@ btr_cur_optimistic_latch_leaves(
mode, nullptr, BUF_GET_POSSIBLY_FREED, mode, nullptr, BUF_GET_POSSIBLY_FREED,
__FILE__, __LINE__, mtr, &err); __FILE__, __LINE__, mtr, &err);
if (err == DB_DECRYPTION_FAILED) { if (!cursor->left_block) {
cursor->index->table->file_unreadable = true; cursor->index->table->file_unreadable = true;
} }
if (btr_page_get_next(cursor->left_block->frame) if (cursor->left_block->page.status
== buf_page_t::FREED
|| btr_page_get_next(cursor->left_block->frame)
!= curr_page_no) { != curr_page_no) {
/* release the left block */ /* release the left block */
btr_leaf_page_release( btr_leaf_page_release(
...@@ -6072,7 +6074,7 @@ btr_estimate_n_rows_in_range_on_level( ...@@ -6072,7 +6074,7 @@ btr_estimate_n_rows_in_range_on_level(
ut_ad((block != NULL) == (err == DB_SUCCESS)); ut_ad((block != NULL) == (err == DB_SUCCESS));
if (err != DB_SUCCESS) { if (!block) {
if (err == DB_DECRYPTION_FAILED) { if (err == DB_DECRYPTION_FAILED) {
ib_push_warning((void *)NULL, ib_push_warning((void *)NULL,
DB_DECRYPTION_FAILED, DB_DECRYPTION_FAILED,
......
...@@ -996,6 +996,9 @@ fil_crypt_read_crypt_data(fil_space_t* space) ...@@ -996,6 +996,9 @@ fil_crypt_read_crypt_data(fil_space_t* space)
nullptr, nullptr,
BUF_GET_POSSIBLY_FREED, BUF_GET_POSSIBLY_FREED,
__FILE__, __LINE__, &mtr)) { __FILE__, __LINE__, &mtr)) {
if (block->page.status == buf_page_t::FREED) {
goto func_exit;
}
mutex_enter(&fil_system.mutex); mutex_enter(&fil_system.mutex);
if (!space->crypt_data && !space->is_stopping()) { if (!space->crypt_data && !space->is_stopping()) {
space->crypt_data = fil_space_read_crypt_data( space->crypt_data = fil_space_read_crypt_data(
...@@ -1003,6 +1006,7 @@ fil_crypt_read_crypt_data(fil_space_t* space) ...@@ -1003,6 +1006,7 @@ fil_crypt_read_crypt_data(fil_space_t* space)
} }
mutex_exit(&fil_system.mutex); mutex_exit(&fil_system.mutex);
} }
func_exit:
mtr.commit(); mtr.commit();
} }
...@@ -1055,6 +1059,9 @@ static bool fil_crypt_start_encrypting_space(fil_space_t* space) ...@@ -1055,6 +1059,9 @@ static bool fil_crypt_start_encrypting_space(fil_space_t* space)
page_id_t(space->id, 0), space->zip_size(), page_id_t(space->id, 0), space->zip_size(),
RW_X_LATCH, NULL, BUF_GET_POSSIBLY_FREED, RW_X_LATCH, NULL, BUF_GET_POSSIBLY_FREED,
__FILE__, __LINE__, &mtr, &err)) { __FILE__, __LINE__, &mtr, &err)) {
if (block->page.status == buf_page_t::FREED) {
goto abort;
}
crypt_data->type = CRYPT_SCHEME_1; crypt_data->type = CRYPT_SCHEME_1;
crypt_data->min_key_version = 0; // all pages are unencrypted crypt_data->min_key_version = 0; // all pages are unencrypted
...@@ -1853,7 +1860,10 @@ fil_crypt_rotate_page( ...@@ -1853,7 +1860,10 @@ fil_crypt_rotate_page(
const lsn_t block_lsn = mach_read_from_8(FIL_PAGE_LSN + frame); const lsn_t block_lsn = mach_read_from_8(FIL_PAGE_LSN + frame);
uint kv = buf_page_get_key_version(frame, space->flags); uint kv = buf_page_get_key_version(frame, space->flags);
if (space->is_stopping()) { if (block->page.status == buf_page_t::FREED) {
/* Do not modify freed pages to avoid an assertion
failure on recovery.*/
} else if (space->is_stopping()) {
/* The tablespace is closing (in DROP TABLE or /* The tablespace is closing (in DROP TABLE or
TRUNCATE TABLE or similar): avoid further access */ TRUNCATE TABLE or similar): avoid further access */
} else if (!kv && !*reinterpret_cast<uint16_t*> } else if (!kv && !*reinterpret_cast<uint16_t*>
...@@ -1882,9 +1892,6 @@ fil_crypt_rotate_page( ...@@ -1882,9 +1892,6 @@ fil_crypt_rotate_page(
some dummy pages will be allocated, with 0 in some dummy pages will be allocated, with 0 in
the FIL_PAGE_TYPE. Those pages should be the FIL_PAGE_TYPE. Those pages should be
skipped from key rotation forever. */ skipped from key rotation forever. */
} else if (block->page.status == buf_page_t::FREED) {
/* Do not modify freed pages to avoid an assertion
failure on recovery.*/
} else if (fil_crypt_needs_rotation( } else if (fil_crypt_needs_rotation(
crypt_data, crypt_data,
kv, kv,
...@@ -2035,8 +2042,10 @@ fil_crypt_flush_space( ...@@ -2035,8 +2042,10 @@ fil_crypt_flush_space(
page_id_t(space->id, 0), space->zip_size(), page_id_t(space->id, 0), space->zip_size(),
RW_X_LATCH, NULL, BUF_GET_POSSIBLY_FREED, RW_X_LATCH, NULL, BUF_GET_POSSIBLY_FREED,
__FILE__, __LINE__, &mtr)) { __FILE__, __LINE__, &mtr)) {
mtr.set_named_space(space); if (block->page.status != buf_page_t::FREED) {
crypt_data->write_page0(block, &mtr); mtr.set_named_space(space);
crypt_data->write_page0(block, &mtr);
}
} }
mtr.commit(); mtr.commit();
......
...@@ -417,6 +417,10 @@ xdes_get_descriptor_const( ...@@ -417,6 +417,10 @@ xdes_get_descriptor_const(
__FILE__, __LINE__, mtr)) { __FILE__, __LINE__, mtr)) {
buf_block_dbg_add_level(block, SYNC_FSP_PAGE); buf_block_dbg_add_level(block, SYNC_FSP_PAGE);
if (block->page.status == buf_page_t::FREED) {
return nullptr;
}
ut_ad(page != 0 || space->free_limit == mach_read_from_4( ut_ad(page != 0 || space->free_limit == mach_read_from_4(
FSP_FREE_LIMIT + FSP_HEADER_OFFSET FSP_FREE_LIMIT + FSP_HEADER_OFFSET
+ block->frame)); + block->frame));
......
...@@ -4995,7 +4995,7 @@ static void lock_rec_block_validate(const page_id_t page_id) ...@@ -4995,7 +4995,7 @@ static void lock_rec_block_validate(const page_id_t page_id)
<< page_id << " err " << err; << page_id << " err " << err;
} }
if (block) { if (block && block->page.status != buf_page_t::FREED) {
buf_block_dbg_add_level(block, SYNC_NO_ORDER_CHECK); buf_block_dbg_add_level(block, SYNC_NO_ORDER_CHECK);
ut_ad(lock_rec_validate_page(block)); ut_ad(lock_rec_validate_page(block));
......
...@@ -2421,6 +2421,7 @@ static void recv_recover_page(buf_block_t* block, mtr_t& mtr, ...@@ -2421,6 +2421,7 @@ static void recv_recover_page(buf_block_t* block, mtr_t& mtr,
any buffered changes. */ any buffered changes. */
init->created = false; init->created = false;
ut_ad(!mtr.has_modifications()); ut_ad(!mtr.has_modifications());
block->page.status = buf_page_t::FREED;
} }
/* Make sure that committing mtr does not change the modification /* Make sure that committing mtr does not change the modification
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment