Skip to content

Commit

Permalink
MDEV-25026 Various code paths are accessing freed pages
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
dr-m committed Mar 2, 2021
1 parent 1f1f61a commit 01b44c0
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 10 deletions.
8 changes: 5 additions & 3 deletions storage/innobase/btr/btr0cur.cc
Original file line number Diff line number Diff line change
Expand Up @@ -807,11 +807,13 @@ btr_cur_optimistic_latch_leaves(
mode, nullptr, BUF_GET_POSSIBLY_FREED,
__FILE__, __LINE__, mtr, &err);

if (err == DB_DECRYPTION_FAILED) {
if (!cursor->left_block) {
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) {
/* release the left block */
btr_leaf_page_release(
Expand Down Expand Up @@ -6072,7 +6074,7 @@ btr_estimate_n_rows_in_range_on_level(

ut_ad((block != NULL) == (err == DB_SUCCESS));

if (err != DB_SUCCESS) {
if (!block) {
if (err == DB_DECRYPTION_FAILED) {
ib_push_warning((void *)NULL,
DB_DECRYPTION_FAILED,
Expand Down
21 changes: 15 additions & 6 deletions storage/innobase/fil/fil0crypt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -996,13 +996,17 @@ fil_crypt_read_crypt_data(fil_space_t* space)
nullptr,
BUF_GET_POSSIBLY_FREED,
__FILE__, __LINE__, &mtr)) {
if (block->page.status == buf_page_t::FREED) {
goto func_exit;
}
mutex_enter(&fil_system.mutex);
if (!space->crypt_data && !space->is_stopping()) {
space->crypt_data = fil_space_read_crypt_data(
zip_size, block->frame);
}
mutex_exit(&fil_system.mutex);
}
func_exit:
mtr.commit();
}

Expand Down Expand Up @@ -1055,6 +1059,9 @@ static bool fil_crypt_start_encrypting_space(fil_space_t* space)
page_id_t(space->id, 0), space->zip_size(),
RW_X_LATCH, NULL, BUF_GET_POSSIBLY_FREED,
__FILE__, __LINE__, &mtr, &err)) {
if (block->page.status == buf_page_t::FREED) {
goto abort;
}

crypt_data->type = CRYPT_SCHEME_1;
crypt_data->min_key_version = 0; // all pages are unencrypted
Expand Down Expand Up @@ -1853,7 +1860,10 @@ fil_crypt_rotate_page(
const lsn_t block_lsn = mach_read_from_8(FIL_PAGE_LSN + frame);
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
TRUNCATE TABLE or similar): avoid further access */
} else if (!kv && !*reinterpret_cast<uint16_t*>
Expand Down Expand Up @@ -1882,9 +1892,6 @@ fil_crypt_rotate_page(
some dummy pages will be allocated, with 0 in
the FIL_PAGE_TYPE. Those pages should be
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(
crypt_data,
kv,
Expand Down Expand Up @@ -2035,8 +2042,10 @@ fil_crypt_flush_space(
page_id_t(space->id, 0), space->zip_size(),
RW_X_LATCH, NULL, BUF_GET_POSSIBLY_FREED,
__FILE__, __LINE__, &mtr)) {
mtr.set_named_space(space);
crypt_data->write_page0(block, &mtr);
if (block->page.status != buf_page_t::FREED) {
mtr.set_named_space(space);
crypt_data->write_page0(block, &mtr);
}
}

mtr.commit();
Expand Down
4 changes: 4 additions & 0 deletions storage/innobase/fsp/fsp0fsp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,10 @@ xdes_get_descriptor_const(
__FILE__, __LINE__, mtr)) {
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(
FSP_FREE_LIMIT + FSP_HEADER_OFFSET
+ block->frame));
Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/lock/lock0lock.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4995,7 +4995,7 @@ static void lock_rec_block_validate(const page_id_t page_id)
<< 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);

ut_ad(lock_rec_validate_page(block));
Expand Down
1 change: 1 addition & 0 deletions storage/innobase/log/log0recv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2421,6 +2421,7 @@ static void recv_recover_page(buf_block_t* block, mtr_t& mtr,
any buffered changes. */
init->created = false;
ut_ad(!mtr.has_modifications());
block->page.status = buf_page_t::FREED;
}

/* Make sure that committing mtr does not change the modification
Expand Down

0 comments on commit 01b44c0

Please sign in to comment.