MDEV-13542: Do not crash on decryption failure

fil_page_type_validate(): Remove. This debug check was mostly redundant
and added little value to the code paths that deal with page_compressed
or encrypted pages.

fil_get_page_type_name(): Remove; unused function.

fil_space_decrypt(): Return an error if the page is not
supposed to be encrypted. It is possible that an unencrypted page
contains a nonzero key_version field even though it is not supposed
to be encrypted. Previously we would crash in such a situation.

buf_page_decrypt_after_read(): Simplify the code. Remove some
unnecessary error message about temporary tablespace corruption.
This is where we would usually invoke fil_space_decrypt().

7 files changed, 22 insertions, 188 deletions

diff --git a/storage/innobase/CMakeLists.txt b/storage/innobase/CMakeLists.txt
index cc31b3c5dcc..bb10160370a 100644
--- a/storage/innobase/CMakeLists.txt
+++ b/storage/innobase/CMakeLists.txt
@@ -128,7 +128,6 @@ SET(INNOBASE_SOURCES
 	include/fil0crypt.h
 	include/fil0crypt.inl
 	include/fil0fil.h
-	include/fil0fil.inl
 	include/fil0pagecompress.h
 	include/fsp0file.h
 	include/fsp0fsp.h
diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc
index 6eb1b6d463a..e6a6bd8cbb7 100644
--- a/storage/innobase/buf/buf0buf.cc
+++ b/storage/innobase/buf/buf0buf.cc
@@ -403,28 +403,22 @@ static bool buf_page_decrypt_after_read(buf_page_t *bpage,
 		return (true);
 	}
 
-	if (node.space->purpose == FIL_TYPE_TEMPORARY
+	buf_tmp_buffer_t* slot;
+
+	if (id.space() == SRV_TMP_SPACE_ID
 	    && innodb_encrypt_temporary_tables) {
-		buf_tmp_buffer_t* slot = buf_pool.io_buf_reserve();
+		slot = buf_pool.io_buf_reserve();
 		ut_a(slot);
 		slot->allocate();
-
-		if (!buf_tmp_page_decrypt(slot->crypt_buf, dst_frame)) {
-			slot->release();
-			ib::error() << "Encrypted page " << id
-				    << " in file " << node.name;
-			return false;
-		}
-
+		bool ok = buf_tmp_page_decrypt(slot->crypt_buf, dst_frame);
 		slot->release();
-		return true;
+		return ok;
 	}
 
 	/* Page is encrypted if encryption information is found from
 	tablespace and page contains used key_version. This is true
 	also for pages first compressed and then encrypted. */
 
-	buf_tmp_buffer_t* slot;
 	uint key_version = buf_page_get_key_version(dst_frame, flags);
 
 	if (page_compressed && !key_version) {
@@ -441,13 +435,9 @@ decompress:
 		slot->allocate();
 
 decompress_with_slot:
-		ut_d(fil_page_type_validate(node.space, dst_frame));
-
 		ulint write_size = fil_page_decompress(
 			slot->crypt_buf, dst_frame, flags);
 		slot->release();
-		ut_ad(!write_size
-		      || fil_page_type_validate(node.space, dst_frame));
 		ut_ad(node.space->referenced());
 		return write_size != 0;
 	}
@@ -467,7 +457,6 @@ decrypt_failed:
 		slot = buf_pool.io_buf_reserve();
 		ut_a(slot);
 		slot->allocate();
-		ut_d(fil_page_type_validate(node.space, dst_frame));
 
 		/* decrypt using crypt_buf to dst_frame */
 		if (!fil_space_decrypt(node.space, slot->crypt_buf, dst_frame)) {
@@ -475,8 +464,6 @@ decrypt_failed:
 			goto decrypt_failed;
 		}
 
-		ut_d(fil_page_type_validate(node.space, dst_frame));
-
 		if ((fil_space_t::full_crc32(flags) && page_compressed)
 		    || fil_page_get_type(dst_frame)
 		    == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED) {
diff --git a/storage/innobase/buf/buf0flu.cc b/storage/innobase/buf/buf0flu.cc
index cc2f72c9a62..17cca04b3a2 100644
--- a/storage/innobase/buf/buf0flu.cc
+++ b/storage/innobase/buf/buf0flu.cc
@@ -625,7 +625,6 @@ static byte *buf_page_encrypt(fil_space_t* space, buf_page_t* bpage, byte* s,
   ut_ad(space->id == bpage->id().space());
   ut_ad(!*slot);
 
-  ut_d(fil_page_type_validate(space, s));
   const uint32_t page_no= bpage->id().page_no();
 
   switch (page_no) {
@@ -722,7 +721,6 @@ not_compressed:
 
     /* Workaround for MDEV-15527. */
     memset(tmp + len, 0 , srv_page_size - len);
-    ut_d(fil_page_type_validate(space, tmp));
 
     if (encrypted)
       tmp= fil_space_encrypt(space, page_no, tmp, d);
@@ -737,7 +735,6 @@ not_compressed:
     d= tmp;
   }
 
-  ut_d(fil_page_type_validate(space, d));
   (*slot)->out_buf= d;
   return d;
 }
diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc
index 516629fd98a..0bd5467cbfa 100644
--- a/storage/innobase/fil/fil0crypt.cc
+++ b/storage/innobase/fil/fil0crypt.cc
@@ -640,10 +640,7 @@ static dberr_t fil_space_decrypt_full_crc32(
 	lsn_t lsn = mach_read_from_8(src_frame + FIL_PAGE_LSN);
 	uint offset = mach_read_from_4(src_frame + FIL_PAGE_OFFSET);
 
-	ut_a(key_version != ENCRYPTION_KEY_NOT_ENCRYPTED);
-
-	ut_ad(crypt_data);
-	ut_ad(crypt_data->is_encrypted());
+	ut_ad(key_version != ENCRYPTION_KEY_NOT_ENCRYPTED);
 
 	memcpy(tmp_frame, src_frame, FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION);
 
@@ -699,8 +696,7 @@ static dberr_t fil_space_decrypt_for_non_full_checksum(
 			src_frame + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID);
 	ib_uint64_t lsn = mach_read_from_8(src_frame + FIL_PAGE_LSN);
 
-	ut_a(key_version != ENCRYPTION_KEY_NOT_ENCRYPTED);
-	ut_a(crypt_data != NULL && crypt_data->is_encrypted());
+	ut_ad(key_version != ENCRYPTION_KEY_NOT_ENCRYPTED);
 
 	/* read space & lsn */
 	uint header_len = FIL_PAGE_DATA;
@@ -753,8 +749,8 @@ static dberr_t fil_space_decrypt_for_non_full_checksum(
 @param[in]	physical_size		page size
 @param[in]	fsp_flags		Tablespace flags
 @param[in,out]	src_frame		Page to decrypt
-@param[out]	err			DB_SUCCESS or DB_DECRYPTION_FAILED
-@return DB_SUCCESS or error */
+@retval DB_SUCCESS on success
+@retval DB_DECRYPTION_FAILED on error */
 dberr_t
 fil_space_decrypt(
 	ulint			space_id,
@@ -764,6 +760,10 @@ fil_space_decrypt(
 	ulint			fsp_flags,
 	byte*			src_frame)
 {
+	if (!crypt_data || !crypt_data->is_encrypted()) {
+		return DB_DECRYPTION_FAILED;
+	}
+
 	if (fil_space_t::full_crc32(fsp_flags)) {
 		return fil_space_decrypt_full_crc32(
 			space_id, crypt_data, tmp_frame, src_frame);
@@ -780,7 +780,8 @@ Decrypt a page.
 @param[in]	tmp_frame		Temporary buffer used for decrypting
 @param[in,out]	src_frame		Page to decrypt
 @return decrypted page, or original not encrypted page if decryption is
-not needed.*/
+not needed.
+@retval nullptr on failure */
 byte*
 fil_space_decrypt(
 	const fil_space_t* space,
@@ -789,7 +790,6 @@ fil_space_decrypt(
 {
 	const ulint physical_size = space->physical_size();
 
-	ut_ad(space->crypt_data != NULL && space->crypt_data->is_encrypted());
 	ut_ad(space->referenced());
 
 	if (DB_SUCCESS != fil_space_decrypt(space->id, space->crypt_data,
@@ -800,9 +800,7 @@ fil_space_decrypt(
 
 	/* Copy the decrypted page back to page buffer, not
 	really any other options. */
-	memcpy(src_frame, tmp_frame, physical_size);
-
-	return src_frame;
+	return static_cast<byte*>(memcpy(src_frame, tmp_frame, physical_size));
 }
 
 /***********************************************************************/
diff --git a/storage/innobase/include/fil0crypt.h b/storage/innobase/include/fil0crypt.h
index c3abdad90ed..26272761f43 100644
--- a/storage/innobase/include/fil0crypt.h
+++ b/storage/innobase/include/fil0crypt.h
@@ -296,7 +296,8 @@ byte* fil_space_encrypt(
 @param[in]	physical_size		page size
 @param[in]	fsp_flags		Tablespace flags
 @param[in,out]	src_frame		Page to decrypt
-@return DB_SUCCESS or error */
+@retval DB_SUCCESS on success
+@retval DB_DECRYPTION_FAILED on error */
 dberr_t
 fil_space_decrypt(
 	ulint			space_id,
@@ -312,7 +313,8 @@ Decrypt a page
 @param[in]	tmp_frame		Temporary buffer used for decrypting
 @param[in,out]	src_frame		Page to decrypt
 @return decrypted page, or original not encrypted page if decryption is
-not needed.*/
+not needed.
+@retval nullptr on failure */
 byte*
 fil_space_decrypt(
 	const fil_space_t* space,
diff --git a/storage/innobase/include/fil0fil.h b/storage/innobase/include/fil0fil.h
index 8889604a919..fadaf36f83b 100644
--- a/storage/innobase/include/fil0fil.h
+++ b/storage/innobase/include/fil0fil.h
@@ -24,8 +24,7 @@ The low-level file system
 Created 10/25/1995 Heikki Tuuri
 *******************************************************/
 
-#ifndef fil0fil_h
-#define fil0fil_h
+#pragma once
 
 #include "fsp0types.h"
 #include "mach0data.h"
@@ -1902,7 +1901,4 @@ void test_make_filepath();
 @return	block size */
 ulint fil_space_get_block_size(const fil_space_t* space, unsigned offset);
 
-#include "fil0fil.inl"
 #endif /* UNIV_INNOCHECKSUM */
-
-#endif /* fil0fil_h */
diff --git a/storage/innobase/include/fil0fil.inl b/storage/innobase/include/fil0fil.inl
deleted file mode 100644
index 3194e54c5b5..00000000000
--- a/storage/innobase/include/fil0fil.inl
+++ /dev/null
@@ -1,145 +0,0 @@
-/*****************************************************************************
-
-Copyright (c) 2015, 2019, 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
-Foundation; version 2 of the License.
-
-This program is distributed in the hope that it will be useful, but WITHOUT
-ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
-FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License along with
-this program; if not, write to the Free Software Foundation, Inc.,
-51 Franklin Street, Fifth Floor, Boston, MA 02110-1335 USA
-
-*****************************************************************************/
-
-/**************************************************//**
-@file include/fil0fil.ic
-The low-level file system support functions
-
-Created 31/03/2015 Jan Lindström
-*******************************************************/
-
-#ifndef fil0fil_ic
-#define fil0fil_ic
-
-/*******************************************************************//**
-Return page type name */
-UNIV_INLINE
-const char*
-fil_get_page_type_name(
-/*===================*/
-	ulint	page_type)	/*!< in: FIL_PAGE_TYPE */
-{
-	switch(page_type) {
-	case FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED:
-		return "PAGE_COMPRESSED_ENRYPTED";
-	case FIL_PAGE_PAGE_COMPRESSED:
-		return "PAGE_COMPRESSED";
-	case FIL_PAGE_TYPE_INSTANT:
-	case FIL_PAGE_INDEX:
-		return "INDEX";
-	case FIL_PAGE_RTREE:
-		return "RTREE";
-	case FIL_PAGE_UNDO_LOG:
-		return "UNDO LOG";
-	case FIL_PAGE_INODE:
-		return "INODE";
-	case FIL_PAGE_IBUF_FREE_LIST:
-		return "IBUF_FREE_LIST";
-	case FIL_PAGE_TYPE_ALLOCATED:
-		return "ALLOCATED";
-	case FIL_PAGE_IBUF_BITMAP:
-		return "IBUF_BITMAP";
-	case FIL_PAGE_TYPE_SYS:
-		return "SYS";
-	case FIL_PAGE_TYPE_TRX_SYS:
-		return "TRX_SYS";
-	case FIL_PAGE_TYPE_FSP_HDR:
-		return "FSP_HDR";
-	case FIL_PAGE_TYPE_XDES:
-		return "XDES";
-	case FIL_PAGE_TYPE_BLOB:
-		return "BLOB";
-	case FIL_PAGE_TYPE_ZBLOB:
-		return "ZBLOB";
-	case FIL_PAGE_TYPE_ZBLOB2:
-		return "ZBLOB2";
-	case FIL_PAGE_TYPE_UNKNOWN:
-		return "OLD UNKNOWN PAGE TYPE";
-	default:
-		return "PAGE TYPE CORRUPTED";
-	}
-}
-
-#ifdef UNIV_DEBUG
-/** Validate page type.
-@param[in]	space	Tablespace object
-@param[in]	page	page to validate
-@return true if valid, false if not */
-UNIV_INLINE
-bool
-fil_page_type_validate(
-	fil_space_t*	space,
-	const byte*	page)
-{
-	const uint16_t page_type = fil_page_get_type(page);
-
-	if ((page_type & 1U << FIL_PAGE_COMPRESS_FCRC32_MARKER)
-	    && space->full_crc32()
-	    && space->is_compressed()) {
-		return true;
-	}
-
-	/* Validate page type */
-	if (!((page_type == FIL_PAGE_PAGE_COMPRESSED ||
-		page_type == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED ||
-		page_type == FIL_PAGE_INDEX ||
-		page_type == FIL_PAGE_TYPE_INSTANT ||
-		page_type == FIL_PAGE_RTREE ||
-		page_type == FIL_PAGE_UNDO_LOG ||
-		page_type == FIL_PAGE_INODE ||
-		page_type == FIL_PAGE_IBUF_FREE_LIST ||
-		page_type == FIL_PAGE_TYPE_ALLOCATED ||
-		page_type == FIL_PAGE_IBUF_BITMAP ||
-		page_type == FIL_PAGE_TYPE_SYS ||
-		page_type == FIL_PAGE_TYPE_TRX_SYS ||
-		page_type == FIL_PAGE_TYPE_FSP_HDR ||
-		page_type == FIL_PAGE_TYPE_XDES ||
-		page_type == FIL_PAGE_TYPE_BLOB ||
-		page_type == FIL_PAGE_TYPE_ZBLOB ||
-		page_type == FIL_PAGE_TYPE_ZBLOB2 ||
-		page_type == FIL_PAGE_TYPE_UNKNOWN))) {
-
-		ulint space_id = mach_read_from_4(
-			page + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID);
-
-		ulint offset = mach_read_from_4(page + FIL_PAGE_OFFSET);
-
-		ulint key_version = mach_read_from_4(
-			page + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION);
-
-		if (space && space->full_crc32()) {
-			key_version = mach_read_from_4(
-				page + FIL_PAGE_FCRC32_KEY_VERSION);
-		}
-
-		/* Dump out the page info */
-		ib::fatal() << "Page " << space_id << ":" << offset
-			<< " name " << (space && space->chain.start
-					? space->chain.start->name : "???")
-			<< " page_type " << page_type
-			<< " key_version " << key_version
-			<< " lsn " << mach_read_from_8(page + FIL_PAGE_LSN)
-			<< " compressed_len " << mach_read_from_2(page + FIL_PAGE_DATA);
-		return false;
-	}
-
-	return true;
-}
-#endif /* UNIV_DEBUG */
-
-#endif /* fil0fil_ic */