Skip to content

Commit 3e55ef2

Browse files
author
Jan Lindström
committed
MDEV-8173: InnoDB; Failing assertion: crypt_data->type == 1
Make sure that when we publish the crypt_data we access the memory cache of the tablespace crypt_data. Make sure that crypt_data is stored whenever it is really needed. All this is not yet enough in my opinion because: sql/encryption.cc has DBUG_ASSERT(scheme->type == 1) i.e. crypt_data->type == CRYPT_SCHEME_1 However, for InnoDB point of view we have global crypt_data for every tablespace. When we change variables on crypt_data we take mutex. However, when we use crypt_data for encryption/decryption we use pointer to this global structure and no mutex to protect against changes on crypt_data. Tablespace encryption starts in fil_crypt_start_encrypting_space from crypt_data that has crypt_data->type = CRYPT_SCHEME_UNENCRYPTED and later we write page 0 CRYPT_SCHEME_1 and finally whe publish that to memory cache.
1 parent 44cd6f2 commit 3e55ef2

File tree

11 files changed

+125
-48
lines changed

11 files changed

+125
-48
lines changed

‎mysql-test/suite/encryption/r/encryption_create_or_replace.result

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,15 @@ CREATE TABLE t1 (pk INT PRIMARY KEY, c VARCHAR(256));
33
CREATE TABLE t2 AS SELECT * FROM t1;
44
drop table t1,t2;
55
SET GLOBAL innodb_encryption_threads = 0;
6+
SET GLOBAL innodb_encryption_threads = 4;
7+
CREATE TABLE `table10_int_autoinc` (`col_int_key` int, pk int auto_increment, `col_int` int, key (`col_int_key` ),primary key (pk)) engine=innodb;
8+
INSERT /*! IGNORE */ INTO table10_int_autoinc VALUES (NULL, NULL, -474021888) , (1, NULL, NULL) , (1141047296, NULL, NULL) , (NULL, NULL, NULL) , (NULL, NULL, 1) , (NULL, NULL, 9) , (0, NULL, 1225785344) , (NULL, NULL, 1574174720) , (2, NULL, NULL) , (6, NULL, 3);
9+
CREATE TABLE `table1_int_autoinc` (`col_int_key` int, pk int auto_increment, `col_int` int,key (`col_int_key` ), primary key (pk)) engine=innodb;
10+
CREATE TABLE `table0_int_autoinc` (`col_int_key` int, pk int auto_increment, `col_int` int, key (`col_int_key` ),primary key (pk)) engine=innodb;
11+
INSERT /*! IGNORE */ INTO table1_int_autoinc VALUES (4, NULL, NULL);
12+
INSERT IGNORE INTO `table0_int_autoinc` ( `col_int_key` ) VALUES ( 1 ), ( 3 ), ( 4 ), ( 1 );
13+
INSERT IGNORE INTO `table1_int_autoinc` ( `col_int` ) VALUES ( 1 ), ( 0 ), ( 7 ), ( 9 );
14+
INSERT IGNORE INTO `table10_int_autoinc` ( `col_int` ) VALUES ( 6 ), ( 2 ), ( 3 ), ( 6 );
15+
drop table if exists create_or_replace_t, table1_int_autoinc, table0_int_autoinc, table10_int_autoinc;
16+
SET GLOBAL innodb_encryption_threads = 0;
17+
SET GLOBAL innodb_encrypt_tables = OFF;

‎mysql-test/suite/encryption/t/encryption_create_or_replace.test

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,77 @@ dec $i;
3333
--enable_query_log
3434

3535
drop table t1,t2;
36-
SET GLOBAL innodb_encryption_threads = 0;
36+
SET GLOBAL innodb_encryption_threads = 0;
37+
38+
#
39+
# MDEV-8173: InnoDB; Failing assertion: crypt_data->type == 1
40+
#
41+
42+
SET GLOBAL innodb_encryption_threads = 4;
43+
44+
CREATE TABLE `table10_int_autoinc` (`col_int_key` int, pk int auto_increment, `col_int` int, key (`col_int_key` ),primary key (pk)) engine=innodb;
45+
INSERT /*! IGNORE */ INTO table10_int_autoinc VALUES (NULL, NULL, -474021888) , (1, NULL, NULL) , (1141047296, NULL, NULL) , (NULL, NULL, NULL) , (NULL, NULL, 1) , (NULL, NULL, 9) , (0, NULL, 1225785344) , (NULL, NULL, 1574174720) , (2, NULL, NULL) , (6, NULL, 3);
46+
47+
CREATE TABLE `table1_int_autoinc` (`col_int_key` int, pk int auto_increment, `col_int` int,key (`col_int_key` ), primary key (pk)) engine=innodb;
48+
49+
CREATE TABLE `table0_int_autoinc` (`col_int_key` int, pk int auto_increment, `col_int` int, key (`col_int_key` ),primary key (pk)) engine=innodb;
50+
51+
INSERT /*! IGNORE */ INTO table1_int_autoinc VALUES (4, NULL, NULL);
52+
INSERT IGNORE INTO `table0_int_autoinc` ( `col_int_key` ) VALUES ( 1 ), ( 3 ), ( 4 ), ( 1 );
53+
INSERT IGNORE INTO `table1_int_autoinc` ( `col_int` ) VALUES ( 1 ), ( 0 ), ( 7 ), ( 9 );
54+
INSERT IGNORE INTO `table10_int_autoinc` ( `col_int` ) VALUES ( 6 ), ( 2 ), ( 3 ), ( 6 );
55+
56+
--connect (con1,localhost,root,,test)
57+
--connect (con2,localhost,root,,test)
58+
59+
--disable_abort_on_error
60+
--disable_warnings
61+
--disable_query_log
62+
63+
let $i = 500;
64+
while ($i)
65+
{
66+
connection con1;
67+
send SET GLOBAL innodb_encrypt_tables = ON;
68+
connection default;
69+
CREATE OR REPLACE TABLE `create_or_replace_t` AS SELECT * FROM `table1_int_autoinc`;
70+
connection con2;
71+
send CREATE OR REPLACE TABLE `create_or_replace_t` AS SELECT * FROM `table10_int_autoinc`;
72+
connection default;
73+
send CREATE OR REPLACE TABLE `create_or_replace_t` AS SELECT * FROM `table0_int_autoinc`;
74+
connection con1;
75+
--reap;
76+
send SET GLOBAL innodb_encrypt_tables = OFF;
77+
connection con2;
78+
--reap;
79+
connection default;
80+
--reap;
81+
send CREATE OR REPLACE TABLE `create_or_replace_t` AS SELECT * FROM `table1_int_autoinc`;
82+
connection con2;
83+
send CREATE OR REPLACE TABLE `create_or_replace_t` AS SELECT * FROM `table10_int_autoinc`;
84+
connection con1;
85+
--reap;
86+
send SET GLOBAL innodb_encrypt_tables = ON;
87+
connection default;
88+
--reap;
89+
send CREATE OR REPLACE TABLE `create_or_replace_t` AS SELECT * FROM `table1_int_autoinc`;
90+
connection con2;
91+
--reap;
92+
CREATE OR REPLACE TABLE `create_or_replace_t` AS SELECT * FROM `table10_int_autoinc`;
93+
CREATE OR REPLACE TABLE `create_or_replace_t` AS SELECT * FROM `table0_int_autoinc`;
94+
connection con1;
95+
--reap;
96+
connection default;
97+
--reap;
98+
dec $i;
99+
}
100+
101+
--enable_query_log
102+
connection default;
103+
drop table if exists create_or_replace_t, table1_int_autoinc, table0_int_autoinc, table10_int_autoinc;
104+
--disconnect con1
105+
--disconnect con2
106+
--enable_abort_on_error
107+
--enable_warnings
108+
SET GLOBAL innodb_encryption_threads = 0;
109+
SET GLOBAL innodb_encrypt_tables = OFF;

‎sql/encryption.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,13 @@ int do_crypt(const unsigned char* src, unsigned int slen,
169169
compile_time_assert(ENCRYPTION_SCHEME_KEY_INVALID ==
170170
(int)ENCRYPTION_KEY_VERSION_INVALID);
171171

172-
DBUG_ASSERT(scheme->type == 1);
172+
// Maybe temporal solution for MDEV-8173
173+
// Rationale: scheme->type is currently global/object
174+
// and when used here might not represent actual state
175+
// of smaller granularity objects e.g. InnoDB page state
176+
// as type is stored to tablespace (FIL) and could represent
177+
// state where key rotation is trying to reach
178+
//DBUG_ASSERT(scheme->type == 1);
173179

174180
if (key_version == ENCRYPTION_KEY_VERSION_INVALID ||
175181
key_version == ENCRYPTION_KEY_NOT_ENCRYPTED)

‎storage/innobase/buf/buf0buf.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4334,7 +4334,7 @@ buf_page_io_complete(
43344334
"InnoDB: space %lu file %s read of page %lu.\n"
43354335
"InnoDB: You may have to recover"
43364336
" from a backup.\n",
4337-
bpage->space,
4337+
(ulint)bpage->space,
43384338
space ? space->name : "NULL",
43394339
(ulong) bpage->offset);
43404340
buf_page_print(frame, buf_page_get_zip_size(bpage),

‎storage/innobase/fil/fil0crypt.cc

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -266,13 +266,8 @@ fil_space_read_crypt_data(
266266
ulint offset) /*!< in: offset */
267267
{
268268
if (memcmp(page + offset, EMPTY_PATTERN, MAGIC_SZ) == 0) {
269-
/* crypt is not stored but create memory cache for
270-
not system tablespace */
271-
if (space != 0) {
272-
return fil_space_create_crypt_data(FIL_SPACE_ENCRYPTION_DEFAULT, FIL_DEFAULT_ENCRYPTION_KEY);
273-
} else {
274-
return NULL;
275-
}
269+
/* Crypt data is not stored. */
270+
return NULL;
276271
}
277272

278273
if (memcmp(page + offset, CRYPT_MAGIC, MAGIC_SZ) != 0) {
@@ -288,12 +283,8 @@ fil_space_read_crypt_data(
288283
page[offset + 3],
289284
page[offset + 4],
290285
page[offset + 5]);
291-
/* Create memory cache for not system tablespace */
292-
if (space != 0) {
293-
return fil_space_create_crypt_data(FIL_SPACE_ENCRYPTION_DEFAULT, FIL_DEFAULT_ENCRYPTION_KEY);
294-
} else {
295-
return NULL;
296-
}
286+
/* Create data is not stored. */
287+
return NULL;
297288
}
298289

299290
ulint type = mach_read_from_1(page + offset + MAGIC_SZ + 0);
@@ -461,12 +452,6 @@ fil_space_write_crypt_data(
461452
return;
462453
}
463454

464-
/* If tablespace encryption is disabled and encryption mode is
465-
DEFAULT, then do not continue writing crypt data to page 0. */
466-
if (!srv_encrypt_tables && crypt_data->encryption == FIL_SPACE_ENCRYPTION_DEFAULT) {
467-
return;
468-
}
469-
470455
fil_space_write_crypt_data_low(crypt_data, crypt_data->type,
471456
page, offset, maxsize, mtr);
472457
}
@@ -1073,7 +1058,7 @@ fil_crypt_start_encrypting_space(
10731058
crypt_data->rotate_state.active_threads = 1;
10741059

10751060
mutex_enter(&crypt_data->mutex);
1076-
fil_space_set_crypt_data(space, crypt_data);
1061+
crypt_data = fil_space_set_crypt_data(space, crypt_data);
10771062
mutex_exit(&crypt_data->mutex);
10781063

10791064
fil_crypt_start_converting = true;
@@ -1108,6 +1093,7 @@ fil_crypt_start_encrypting_space(
11081093
/* 3 - compute location to store crypt data */
11091094
byte* frame = buf_block_get_frame(block);
11101095
ulint maxsize;
1096+
ut_ad(crypt_data);
11111097
crypt_data->page0_offset =
11121098
fsp_header_get_crypt_offset(zip_size, &maxsize);
11131099

@@ -1160,6 +1146,7 @@ fil_crypt_start_encrypting_space(
11601146

11611147
/* 5 - publish crypt data */
11621148
mutex_enter(&fil_crypt_threads_mutex);
1149+
ut_ad(crypt_data);
11631150
mutex_enter(&crypt_data->mutex);
11641151
crypt_data->type = CRYPT_SCHEME_1;
11651152
ut_a(crypt_data->rotate_state.active_threads == 1);
@@ -1173,6 +1160,7 @@ fil_crypt_start_encrypting_space(
11731160
return pending_op;
11741161
} while (0);
11751162

1163+
ut_ad(crypt_data);
11761164
mutex_enter(&crypt_data->mutex);
11771165
ut_a(crypt_data->rotate_state.active_threads == 1);
11781166
crypt_data->rotate_state.active_threads = 0;

‎storage/innobase/fil/fil0fil.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6855,14 +6855,15 @@ fil_space_get_crypt_data(
68556855
/******************************************************************
68566856
Get crypt data for a tablespace */
68576857
UNIV_INTERN
6858-
void
6858+
fil_space_crypt_t*
68596859
fil_space_set_crypt_data(
68606860
/*=====================*/
68616861
ulint id, /*!< in: space id */
68626862
fil_space_crypt_t* crypt_data) /*!< in: crypt data */
68636863
{
68646864
fil_space_t* space;
68656865
fil_space_crypt_t* free_crypt_data = NULL;
6866+
fil_space_crypt_t* ret_crypt_data = NULL;
68666867

68676868
ut_ad(fil_system);
68686869

@@ -6881,9 +6882,11 @@ fil_space_set_crypt_data(
68816882
mutex_exit(&fil_system->mutex);
68826883
fil_space_merge_crypt_data(space->crypt_data,
68836884
crypt_data);
6885+
ret_crypt_data = space->crypt_data;
68846886
free_crypt_data = crypt_data;
68856887
} else {
68866888
space->crypt_data = crypt_data;
6889+
ret_crypt_data = space->crypt_data;
68876890
mutex_exit(&fil_system->mutex);
68886891
}
68896892
} else {
@@ -6899,4 +6902,6 @@ fil_space_set_crypt_data(
68996902
*/
69006903
fil_space_destroy_crypt_data(&free_crypt_data);
69016904
}
6905+
6906+
return ret_crypt_data;
69026907
}

‎storage/innobase/include/fil0crypt.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ fil_space_get_crypt_data(
134134
/*********************************************************************
135135
Set crypt data for a space*/
136136
UNIV_INTERN
137-
void
137+
fil_space_crypt_t*
138138
fil_space_set_crypt_data(
139139
/*=====================*/
140140
ulint space, /*!< in: tablespace id */

‎storage/xtradb/buf/buf0buf.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4414,7 +4414,7 @@ buf_page_io_complete(
44144414
"InnoDB: space %lu file %s read of page %lu.\n"
44154415
"InnoDB: You may have to recover"
44164416
" from a backup.\n",
4417-
bpage->space,
4417+
(ulint)bpage->space,
44184418
space ? space->name : "NULL",
44194419
(ulong) bpage->offset);
44204420

‎storage/xtradb/fil/fil0crypt.cc

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -266,13 +266,8 @@ fil_space_read_crypt_data(
266266
ulint offset) /*!< in: offset */
267267
{
268268
if (memcmp(page + offset, EMPTY_PATTERN, MAGIC_SZ) == 0) {
269-
/* crypt is not stored but create memory cache for
270-
not system tablespace */
271-
if (space != 0) {
272-
return fil_space_create_crypt_data(FIL_SPACE_ENCRYPTION_DEFAULT, FIL_DEFAULT_ENCRYPTION_KEY);
273-
} else {
274-
return NULL;
275-
}
269+
/* Crypt data is not stored. */
270+
return NULL;
276271
}
277272

278273
if (memcmp(page + offset, CRYPT_MAGIC, MAGIC_SZ) != 0) {
@@ -288,12 +283,8 @@ fil_space_read_crypt_data(
288283
page[offset + 3],
289284
page[offset + 4],
290285
page[offset + 5]);
291-
/* Create memory cache for not system tablespace */
292-
if (space != 0) {
293-
return fil_space_create_crypt_data(FIL_SPACE_ENCRYPTION_DEFAULT, FIL_DEFAULT_ENCRYPTION_KEY);
294-
} else {
295-
return NULL;
296-
}
286+
/* Crypt data is not stored. */
287+
return NULL;
297288
}
298289

299290
ulint type = mach_read_from_1(page + offset + MAGIC_SZ + 0);
@@ -461,12 +452,6 @@ fil_space_write_crypt_data(
461452
return;
462453
}
463454

464-
/* If tablespace encryption is disabled and encryption mode is
465-
DEFAULT, then do not continue writing crypt data to page 0. */
466-
if (!srv_encrypt_tables && crypt_data->encryption == FIL_SPACE_ENCRYPTION_DEFAULT) {
467-
return;
468-
}
469-
470455
fil_space_write_crypt_data_low(crypt_data, crypt_data->type,
471456
page, offset, maxsize, mtr);
472457
}
@@ -1073,7 +1058,7 @@ fil_crypt_start_encrypting_space(
10731058
crypt_data->rotate_state.active_threads = 1;
10741059

10751060
mutex_enter(&crypt_data->mutex);
1076-
fil_space_set_crypt_data(space, crypt_data);
1061+
crypt_data = fil_space_set_crypt_data(space, crypt_data);
10771062
mutex_exit(&crypt_data->mutex);
10781063

10791064
fil_crypt_start_converting = true;
@@ -1108,6 +1093,7 @@ fil_crypt_start_encrypting_space(
11081093
/* 3 - compute location to store crypt data */
11091094
byte* frame = buf_block_get_frame(block);
11101095
ulint maxsize;
1096+
ut_ad(crypt_data);
11111097
crypt_data->page0_offset =
11121098
fsp_header_get_crypt_offset(zip_size, &maxsize);
11131099

@@ -1160,6 +1146,7 @@ fil_crypt_start_encrypting_space(
11601146

11611147
/* 5 - publish crypt data */
11621148
mutex_enter(&fil_crypt_threads_mutex);
1149+
ut_ad(crypt_data);
11631150
mutex_enter(&crypt_data->mutex);
11641151
crypt_data->type = CRYPT_SCHEME_1;
11651152
ut_a(crypt_data->rotate_state.active_threads == 1);
@@ -1173,6 +1160,7 @@ fil_crypt_start_encrypting_space(
11731160
return pending_op;
11741161
} while (0);
11751162

1163+
ut_ad(crypt_data);
11761164
mutex_enter(&crypt_data->mutex);
11771165
ut_a(crypt_data->rotate_state.active_threads == 1);
11781166
crypt_data->rotate_state.active_threads = 0;

‎storage/xtradb/fil/fil0fil.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6982,14 +6982,15 @@ fil_space_get_crypt_data(
69826982
/******************************************************************
69836983
Get crypt data for a tablespace */
69846984
UNIV_INTERN
6985-
void
6985+
fil_space_crypt_t*
69866986
fil_space_set_crypt_data(
69876987
/*==================*/
69886988
ulint id, /*!< in: space id */
69896989
fil_space_crypt_t* crypt_data) /*!< in: crypt data */
69906990
{
69916991
fil_space_t* space;
69926992
fil_space_crypt_t* free_crypt_data = NULL;
6993+
fil_space_crypt_t* ret_crypt_data = NULL;
69936994

69946995
ut_ad(fil_system);
69956996

@@ -7008,9 +7009,11 @@ fil_space_set_crypt_data(
70087009
mutex_exit(&fil_system->mutex);
70097010
fil_space_merge_crypt_data(space->crypt_data,
70107011
crypt_data);
7012+
ret_crypt_data = space->crypt_data;
70117013
free_crypt_data = crypt_data;
70127014
} else {
70137015
space->crypt_data = crypt_data;
7016+
ret_crypt_data = space->crypt_data;
70147017
mutex_exit(&fil_system->mutex);
70157018
}
70167019
} else {
@@ -7026,4 +7029,6 @@ fil_space_set_crypt_data(
70267029
*/
70277030
fil_space_destroy_crypt_data(&free_crypt_data);
70287031
}
7032+
7033+
return ret_crypt_data;
70297034
}

0 commit comments

Comments
 (0)