From 08b0b2b6fb849e7b440bfd104f8121b5d7d819e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 17 Jan 2020 16:22:13 +0200 Subject: [PATCH] MDEV-21513: Avoid some crashes in ALTER TABLE...IMPORT TABLESPACE IndexPurge::next(): Replace btr_pcur_move_to_next_user_rec() with some equivalent code that performs sanity checks without killing the server. Perform some additional sanity checks as well. This change is motivated by mysql/mysql-server@48de4d74f4d2f10cd01b129753c7dfa908cf36b5 which unnecessarily introduces storage overhead to btr_pcur_t and uses a test case that injects a fault somewhere else, not in the code path that was modified. --- storage/innobase/row/row0import.cc | 67 +++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 5 deletions(-) diff --git a/storage/innobase/row/row0import.cc b/storage/innobase/row/row0import.cc index e8933ea5419..751cd4e5293 100644 --- a/storage/innobase/row/row0import.cc +++ b/storage/innobase/row/row0import.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 2012, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2015, 2019, MariaDB Corporation. +Copyright (c) 2015, 2020, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -1524,13 +1524,70 @@ IndexPurge::next() UNIV_NOTHROW mtr_set_log_mode(&m_mtr, MTR_LOG_NO_REDO); btr_pcur_restore_position(BTR_MODIFY_LEAF, &m_pcur, &m_mtr); + /* The following is based on btr_pcur_move_to_next_user_rec(). */ + m_pcur.old_stored = false; + ut_ad(m_pcur.latch_mode == BTR_MODIFY_LEAF); + do { + if (btr_pcur_is_after_last_on_page(&m_pcur)) { + if (btr_pcur_is_after_last_in_tree(&m_pcur, &m_mtr)) { + return DB_END_OF_INDEX; + } - if (!btr_pcur_move_to_next_user_rec(&m_pcur, &m_mtr)) { + buf_block_t* block = btr_pcur_get_block(&m_pcur); + uint32_t next_page = btr_page_get_next(block->frame); - return(DB_END_OF_INDEX); - } + /* MDEV-13542 FIXME: Make these checks part of + btr_pcur_move_to_next_page(), and introduce a + return status that will be checked in all callers! */ + switch (next_page) { + default: + if (next_page != block->page.id.page_no()) { + break; + } + /* MDEV-20931 FIXME: Check that + next_page is within the tablespace + bounds! Also check that it is not a + change buffer bitmap page. */ + /* fall through */ + case 0: + case 1: + case FIL_NULL: + return DB_CORRUPTION; + } - return(DB_SUCCESS); + dict_index_t* index = m_pcur.btr_cur.index; + buf_block_t* next_block = btr_block_get( + page_id_t(block->page.id.space(), next_page), + block->page.size, BTR_MODIFY_LEAF, index, + &m_mtr); + + if (UNIV_UNLIKELY(!next_block + || !fil_page_index_page_check( + next_block->frame) + || !!dict_index_is_spatial(index) + != (fil_page_get_type( + next_block->frame) + == FIL_PAGE_RTREE) + || page_is_comp(next_block->frame) + != page_is_comp(block->frame) + || btr_page_get_prev( + next_block->frame) + != block->page.id.page_no())) { + return DB_CORRUPTION; + } + + btr_leaf_page_release(block, BTR_MODIFY_LEAF, &m_mtr); + + page_cur_set_before_first(next_block, + &m_pcur.btr_cur.page_cur); + + ut_d(page_check_dir(next_block->frame)); + } else { + btr_pcur_move_to_next_on_page(&m_pcur); + } + } while (!btr_pcur_is_on_user_rec(&m_pcur)); + + return DB_SUCCESS; } /**