[Commits] 985daeb: MDEV-10886: encryption.innodb-bad-key-change fails (crashes) in buildbot

Jan Lindström jan.lindstrom at mariadb.com
Mon Sep 26 12:31:42 EEST 2016


revision-id: 985daeb2406ffa512ef0db76a08ff1e8d30c74c4 (mariadb-10.1.17-23-g985daeb)
parent(s): fd5b930a1fce9ef08941fa2edd767f7e4762904a
committer: Jan Lindström
timestamp: 2016-09-26 12:29:31 +0300
message:

MDEV-10886: encryption.innodb-bad-key-change fails (crashes) in buildbot

Problem was that NULL-pointer was accessed inside a macro when
page read from tablespace is encrypted but decrypt fails because
of incorrect key file.

Removed unsafe macro using inlined function where used pointers
are checked.

---
 storage/innobase/btr/btr0cur.cc     |  9 ++++++---
 storage/innobase/btr/btr0scrub.cc   | 20 +++++++++++++-------
 storage/innobase/ibuf/ibuf0ibuf.cc  | 14 ++++++++++++--
 storage/innobase/include/btr0btr.h  | 14 +++++++++++---
 storage/innobase/include/btr0btr.ic | 32 ++++++++++++++++++++++++++++++++
 storage/xtradb/btr/btr0cur.cc       |  9 ++++++---
 storage/xtradb/btr/btr0scrub.cc     | 20 +++++++++++++-------
 storage/xtradb/ibuf/ibuf0ibuf.cc    | 14 ++++++++++++--
 storage/xtradb/include/btr0btr.h    | 14 +++++++++++---
 storage/xtradb/include/btr0btr.ic   | 32 ++++++++++++++++++++++++++++++++
 10 files changed, 148 insertions(+), 30 deletions(-)

diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc
index eca232d..18f1930 100644
--- a/storage/innobase/btr/btr0cur.cc
+++ b/storage/innobase/btr/btr0cur.cc
@@ -2138,9 +2138,12 @@ btr_cur_update_in_place(
 	if (page_zip
 	    && !(flags & BTR_KEEP_IBUF_BITMAP)
 	    && !dict_index_is_clust(index)
-	    && page_is_leaf(buf_block_get_frame(block))) {
-		/* Update the free bits in the insert buffer. */
-		ibuf_update_free_bits_zip(block, mtr);
+	    && block) {
+		buf_frame_t* frame = buf_block_get_frame(block);
+		if (frame && page_is_leaf(frame)) {
+			/* Update the free bits in the insert buffer. */
+			ibuf_update_free_bits_zip(block, mtr);
+		}
 	}
 
 	return(err);
diff --git a/storage/innobase/btr/btr0scrub.cc b/storage/innobase/btr/btr0scrub.cc
index e6acb78..62a41d1 100644
--- a/storage/innobase/btr/btr0scrub.cc
+++ b/storage/innobase/btr/btr0scrub.cc
@@ -368,12 +368,17 @@ btr_optimistic_scrub(
 
 	/* We play safe and reset the free bits */
 	if (!dict_index_is_clust(index) &&
-	    page_is_leaf(buf_block_get_frame(block))) {
+	    block != NULL) {
+		buf_frame_t* frame = buf_block_get_frame(block);
+		if (frame &&
+		    page_is_leaf(frame)) {
 
 			ibuf_reset_free_bits(block);
+		}
 	}
 
 	scrub_data->scrub_stat.page_reorganizations++;
+
 	return DB_SUCCESS;
 }
 
@@ -488,9 +493,13 @@ btr_pessimistic_scrub(
 		/* We play safe and reset the free bits
 		* NOTE: need to call this prior to btr_page_split_and_insert */
 		if (!dict_index_is_clust(index) &&
-		    page_is_leaf(buf_block_get_frame(block))) {
+		    block != NULL) {
+			buf_frame_t* frame = buf_block_get_frame(block);
+			if (frame &&
+			    page_is_leaf(frame)) {
 
-			ibuf_reset_free_bits(block);
+				ibuf_reset_free_bits(block);
+			}
 		}
 
 		rec = btr_page_split_and_insert(
@@ -788,11 +797,8 @@ btr_scrub_page(
 		return BTR_SCRUB_SKIP_PAGE_AND_CLOSE_TABLE;
 	}
 
-	buf_frame_t* frame = NULL;
+	buf_frame_t* frame = buf_block_get_frame(block);
 
-	if (block) {
-		frame = buf_block_get_frame(block);
-	}
 	if (!frame || btr_page_get_index_id(frame) !=
 	    scrub_data->current_index->id) {
 		/* page has been reallocated to new index */
diff --git a/storage/innobase/ibuf/ibuf0ibuf.cc b/storage/innobase/ibuf/ibuf0ibuf.cc
index 0a2140c..fcf0388 100644
--- a/storage/innobase/ibuf/ibuf0ibuf.cc
+++ b/storage/innobase/ibuf/ibuf0ibuf.cc
@@ -915,9 +915,15 @@ ibuf_set_free_bits_low(
 	page_t*	bitmap_page;
 	ulint	space;
 	ulint	page_no;
+	buf_frame_t* frame;
 
-	if (!page_is_leaf(buf_block_get_frame(block))) {
+	if (!block) {
+		return;
+	}
+
+	frame = buf_block_get_frame(block);
 
+	if (!frame || !page_is_leaf(frame)) {
 		return;
 	}
 
@@ -1091,7 +1097,11 @@ ibuf_update_free_bits_zip(
 	page_no = buf_block_get_page_no(block);
 	zip_size = buf_block_get_zip_size(block);
 
-	ut_a(page_is_leaf(buf_block_get_frame(block)));
+	ut_a(block);
+
+	buf_frame_t* frame = buf_block_get_frame(block);
+
+	ut_a(frame && page_is_leaf(frame));
 	ut_a(zip_size);
 
 	bitmap_page = ibuf_bitmap_get_map_page(space, page_no, zip_size, mtr);
diff --git a/storage/innobase/include/btr0btr.h b/storage/innobase/include/btr0btr.h
index bf3f4a7..a1882cd 100644
--- a/storage/innobase/include/btr0btr.h
+++ b/storage/innobase/include/btr0btr.h
@@ -295,9 +295,17 @@ btr_block_get_func(
 @param idx	index tree, may be NULL if not the insert buffer tree
 @param mtr	mini-transaction handle
 @return the uncompressed page frame */
-# define btr_page_get(space,zip_size,page_no,mode,idx,mtr)		\
-	buf_block_get_frame(btr_block_get(space,zip_size,page_no, \
-			mode,idx,mtr))
+UNIV_INLINE
+page_t*
+btr_page_get(
+/*=========*/
+	ulint		space,
+	ulint		zip_size,
+	ulint		root_page_no,
+	ulint		mode,
+	dict_index_t*	index,
+	mtr_t*		mtr)
+	MY_ATTRIBUTE((warn_unused_result));
 #endif /* !UNIV_HOTBACKUP */
 /**************************************************************//**
 Gets the index id field of a page.
diff --git a/storage/innobase/include/btr0btr.ic b/storage/innobase/include/btr0btr.ic
index e941031..5acbee1 100644
--- a/storage/innobase/include/btr0btr.ic
+++ b/storage/innobase/include/btr0btr.ic
@@ -98,6 +98,38 @@ btr_page_set_index_id(
 		mlog_write_ull(page + (PAGE_HEADER + PAGE_INDEX_ID), id, mtr);
 	}
 }
+
+/** Gets a buffer page and declares its latching order level.
+ at param space	tablespace identifier
+ at param zip_size	compressed page size in bytes or 0 for uncompressed pages
+ at param page_no	page number
+ at param mode	latch mode
+ at param idx	index tree, may be NULL if not the insert buffer tree
+ at param mtr	mini-transaction handle
+ at return the uncompressed page frame */
+UNIV_INLINE
+page_t*
+btr_page_get(
+/*=========*/
+	ulint		space,
+	ulint		zip_size,
+	ulint		root_page_no,
+	ulint		mode,
+	dict_index_t*	index,
+	mtr_t*		mtr)
+{
+	buf_block_t* block=NULL;
+	buf_frame_t* frame=NULL;
+
+	block = btr_block_get(space, zip_size, root_page_no, mode, index, mtr);
+
+	if (block) {
+		frame = buf_block_get_frame(block);
+	}
+
+	return ((page_t*)frame);
+}
+
 #endif /* !UNIV_HOTBACKUP */
 
 /**************************************************************//**
diff --git a/storage/xtradb/btr/btr0cur.cc b/storage/xtradb/btr/btr0cur.cc
index 0d117e6..5449397 100644
--- a/storage/xtradb/btr/btr0cur.cc
+++ b/storage/xtradb/btr/btr0cur.cc
@@ -2281,9 +2281,12 @@ btr_cur_update_in_place(
 	if (page_zip
 	    && !(flags & BTR_KEEP_IBUF_BITMAP)
 	    && !dict_index_is_clust(index)
-	    && page_is_leaf(buf_block_get_frame(block))) {
-		/* Update the free bits in the insert buffer. */
-		ibuf_update_free_bits_zip(block, mtr);
+	    && block) {
+		buf_frame_t* frame = buf_block_get_frame(block);
+		if (frame && page_is_leaf(frame)) {
+			/* Update the free bits in the insert buffer. */
+			ibuf_update_free_bits_zip(block, mtr);
+		}
 	}
 
 	return(err);
diff --git a/storage/xtradb/btr/btr0scrub.cc b/storage/xtradb/btr/btr0scrub.cc
index e6acb78..62a41d1 100644
--- a/storage/xtradb/btr/btr0scrub.cc
+++ b/storage/xtradb/btr/btr0scrub.cc
@@ -368,12 +368,17 @@ btr_optimistic_scrub(
 
 	/* We play safe and reset the free bits */
 	if (!dict_index_is_clust(index) &&
-	    page_is_leaf(buf_block_get_frame(block))) {
+	    block != NULL) {
+		buf_frame_t* frame = buf_block_get_frame(block);
+		if (frame &&
+		    page_is_leaf(frame)) {
 
 			ibuf_reset_free_bits(block);
+		}
 	}
 
 	scrub_data->scrub_stat.page_reorganizations++;
+
 	return DB_SUCCESS;
 }
 
@@ -488,9 +493,13 @@ btr_pessimistic_scrub(
 		/* We play safe and reset the free bits
 		* NOTE: need to call this prior to btr_page_split_and_insert */
 		if (!dict_index_is_clust(index) &&
-		    page_is_leaf(buf_block_get_frame(block))) {
+		    block != NULL) {
+			buf_frame_t* frame = buf_block_get_frame(block);
+			if (frame &&
+			    page_is_leaf(frame)) {
 
-			ibuf_reset_free_bits(block);
+				ibuf_reset_free_bits(block);
+			}
 		}
 
 		rec = btr_page_split_and_insert(
@@ -788,11 +797,8 @@ btr_scrub_page(
 		return BTR_SCRUB_SKIP_PAGE_AND_CLOSE_TABLE;
 	}
 
-	buf_frame_t* frame = NULL;
+	buf_frame_t* frame = buf_block_get_frame(block);
 
-	if (block) {
-		frame = buf_block_get_frame(block);
-	}
 	if (!frame || btr_page_get_index_id(frame) !=
 	    scrub_data->current_index->id) {
 		/* page has been reallocated to new index */
diff --git a/storage/xtradb/ibuf/ibuf0ibuf.cc b/storage/xtradb/ibuf/ibuf0ibuf.cc
index dc6ad49..13597d3 100644
--- a/storage/xtradb/ibuf/ibuf0ibuf.cc
+++ b/storage/xtradb/ibuf/ibuf0ibuf.cc
@@ -956,9 +956,15 @@ ibuf_set_free_bits_low(
 	page_t*	bitmap_page;
 	ulint	space;
 	ulint	page_no;
+	buf_frame_t* frame;
 
-	if (!page_is_leaf(buf_block_get_frame(block))) {
+	if (!block) {
+		return;
+	}
+
+	frame = buf_block_get_frame(block);
 
+	if (!frame || !page_is_leaf(frame)) {
 		return;
 	}
 
@@ -1132,7 +1138,11 @@ ibuf_update_free_bits_zip(
 	page_no = buf_block_get_page_no(block);
 	zip_size = buf_block_get_zip_size(block);
 
-	ut_a(page_is_leaf(buf_block_get_frame(block)));
+	ut_a(block);
+
+	buf_frame_t* frame = buf_block_get_frame(block);
+
+	ut_a(frame && page_is_leaf(frame));
 	ut_a(zip_size);
 
 	bitmap_page = ibuf_bitmap_get_map_page(space, page_no, zip_size, mtr);
diff --git a/storage/xtradb/include/btr0btr.h b/storage/xtradb/include/btr0btr.h
index 5047d1b..9ab62f7 100644
--- a/storage/xtradb/include/btr0btr.h
+++ b/storage/xtradb/include/btr0btr.h
@@ -298,9 +298,17 @@ btr_block_get_func(
 @param idx	index tree, may be NULL if not the insert buffer tree
 @param mtr	mini-transaction handle
 @return the uncompressed page frame */
-# define btr_page_get(space,zip_size,page_no,mode,idx,mtr)		\
-	buf_block_get_frame(btr_block_get(space,zip_size,page_no, \
-			mode,idx,mtr))
+UNIV_INLINE
+page_t*
+btr_page_get(
+/*=========*/
+	ulint		space,
+	ulint		zip_size,
+	ulint		root_page_no,
+	ulint		mode,
+	dict_index_t*	index,
+	mtr_t*		mtr)
+	MY_ATTRIBUTE((warn_unused_result));
 #endif /* !UNIV_HOTBACKUP */
 /**************************************************************//**
 Gets the index id field of a page.
diff --git a/storage/xtradb/include/btr0btr.ic b/storage/xtradb/include/btr0btr.ic
index 34e0d36..62a2487 100644
--- a/storage/xtradb/include/btr0btr.ic
+++ b/storage/xtradb/include/btr0btr.ic
@@ -98,6 +98,38 @@ btr_page_set_index_id(
 		mlog_write_ull(page + (PAGE_HEADER + PAGE_INDEX_ID), id, mtr);
 	}
 }
+
+/** Gets a buffer page and declares its latching order level.
+ at param space	tablespace identifier
+ at param zip_size	compressed page size in bytes or 0 for uncompressed pages
+ at param page_no	page number
+ at param mode	latch mode
+ at param idx	index tree, may be NULL if not the insert buffer tree
+ at param mtr	mini-transaction handle
+ at return the uncompressed page frame */
+UNIV_INLINE
+page_t*
+btr_page_get(
+/*=========*/
+	ulint		space,
+	ulint		zip_size,
+	ulint		root_page_no,
+	ulint		mode,
+	dict_index_t*	index,
+	mtr_t*		mtr)
+{
+	buf_block_t* block=NULL;
+	buf_frame_t* frame=NULL;
+
+	block = btr_block_get(space, zip_size, root_page_no, mode, index, mtr);
+
+	if (block) {
+		frame = buf_block_get_frame(block);
+	}
+
+	return ((page_t*)frame);
+}
+
 #endif /* !UNIV_HOTBACKUP */
 
 /**************************************************************//**


More information about the commits mailing list