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().

Author: dr-m

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

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 @@ static bool buf_page_decrypt_after_read(buf_page_t *bpage,
 		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,16 +457,13 @@ static bool buf_page_decrypt_after_read(buf_page_t *bpage,
 		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)) {
 			slot->release();
 			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) {

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 @@ static byte *buf_page_encrypt(fil_space_t* space, buf_page_t* bpage, byte* s,
 
     /* 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 @@ static byte *buf_page_encrypt(fil_space_t* space, buf_page_t* bpage, byte* s,
     d= tmp;
   }
 
-  ut_d(fil_page_type_validate(space, d));
   (*slot)->out_buf= d;
   return d;
 }

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));
 }
 
 /***********************************************************************/

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,

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 */