Skip to content

Commit 66a09bd

Browse files
committed
MDEV-13318 Crash recovery failure after the server is killed during innodb_encrypt_log startup
This fixes several InnoDB bugs related to innodb_encrypt_log and two Mariabackup --backup bugs. log_crypt(): Properly derive the initialization vector from the start LSN of each block. Add a debug assertion. log_crypt_init(): Note that the function should only be used when creating redo log files and that the information is persisted in the checkpoint pages. xtrabackup_copy_log(): Validate data_len. xtrabackup_backup_func(): Always use the chosen checkpoint buffer. log_group_write_buf(), log_write_up_to(): Only log_crypt() the redo log payload, not the padding bytes. innobase_start_or_create_for_mysql(): Do not invoke log_crypt_init() or initiate a redo log checkpoint. recv_find_max_checkpoint(): Return the contents of LOG_CHECKPOINT_NO to xtrabackup_backup_func() in log_sys->next_checkpoint_no.
1 parent 8ee4b41 commit 66a09bd

File tree

6 files changed

+46
-40
lines changed

6 files changed

+46
-40
lines changed

extra/mariabackup/xtrabackup.cc

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2359,10 +2359,18 @@ xtrabackup_copy_log(copy_logfile copy, lsn_t start_lsn, lsn_t end_lsn)
23592359

23602360
scanned_checkpoint = checkpoint;
23612361
ulint data_len = log_block_get_data_len(log_block);
2362-
scanned_lsn += data_len;
23632362

2364-
if (data_len != OS_FILE_LOG_BLOCK_SIZE) {
2365-
/* The current end of the log was reached. */
2363+
if (data_len == OS_FILE_LOG_BLOCK_SIZE) {
2364+
/* We got a full log block. */
2365+
scanned_lsn += data_len;
2366+
} else if (data_len
2367+
>= OS_FILE_LOG_BLOCK_SIZE - LOG_BLOCK_TRL_SIZE
2368+
|| data_len <= LOG_BLOCK_HDR_SIZE) {
2369+
/* We got a garbage block (abrupt end of the log). */
2370+
break;
2371+
} else {
2372+
/* We got a partial block (abrupt end of the log). */
2373+
scanned_lsn += data_len;
23662374
break;
23672375
}
23682376
}
@@ -2375,7 +2383,7 @@ xtrabackup_copy_log(copy_logfile copy, lsn_t start_lsn, lsn_t end_lsn)
23752383

23762384
if (ulint write_size = ulint(end_lsn - start_lsn)) {
23772385
if (srv_encrypt_log) {
2378-
log_crypt(log_sys->buf, write_size);
2386+
log_crypt(log_sys->buf, start_lsn, write_size);
23792387
}
23802388

23812389
if (ds_write(dst_log_file, log_sys->buf, write_size)) {
@@ -3757,10 +3765,10 @@ xtrabackup_backup_func()
37573765

37583766
const byte* buf = log_sys->checkpoint_buf;
37593767

3760-
checkpoint_lsn_start = mach_read_from_8(buf + LOG_CHECKPOINT_LSN);
3761-
checkpoint_no_start = mach_read_from_8(buf + LOG_CHECKPOINT_NO);
3762-
37633768
reread_log_header:
3769+
checkpoint_lsn_start = log_sys->log.lsn;
3770+
checkpoint_no_start = log_sys->next_checkpoint_no;
3771+
37643772
err = recv_find_max_checkpoint(&max_cp_field);
37653773

37663774
if (err != DB_SUCCESS) {
@@ -3774,10 +3782,9 @@ xtrabackup_backup_func()
37743782
ut_ad(!((log_sys->log.format ^ LOG_HEADER_FORMAT_CURRENT)
37753783
& ~LOG_HEADER_FORMAT_ENCRYPTED));
37763784

3777-
if (checkpoint_no_start != mach_read_from_8(buf + LOG_CHECKPOINT_NO)) {
3785+
log_group_header_read(&log_sys->log, max_cp_field);
37783786

3779-
checkpoint_lsn_start = mach_read_from_8(buf + LOG_CHECKPOINT_LSN);
3780-
checkpoint_no_start = mach_read_from_8(buf + LOG_CHECKPOINT_NO);
3787+
if (checkpoint_no_start != mach_read_from_8(buf + LOG_CHECKPOINT_NO)) {
37813788
goto reread_log_header;
37823789
}
37833790

storage/innobase/include/log0crypt.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ MDEV-11782: Rewritten for MariaDB 10.2 by Marko Mäkelä, MariaDB Corporation.
3232
/** innodb_encrypt_log: whether to encrypt the redo log */
3333
extern my_bool srv_encrypt_log;
3434

35-
/** Initialize the redo log encryption key.
35+
/** Initialize the redo log encryption key and random parameters
36+
when creating a new redo log.
37+
The random parameters will be persisted in the log checkpoint pages.
38+
@see log_crypt_write_checkpoint_buf()
39+
@see log_crypt_read_checkpoint_buf()
3640
@return whether the operation succeeded */
3741
UNIV_INTERN
3842
bool
@@ -71,10 +75,11 @@ log_crypt_read_checkpoint_buf(const byte* buf);
7175

7276
/** Encrypt or decrypt log blocks.
7377
@param[in,out] buf log blocks to encrypt or decrypt
78+
@param[in] lsn log sequence number of the start of the buffer
7479
@param[in] size size of the buffer, in bytes
7580
@param[in] decrypt whether to decrypt instead of encrypting */
7681
UNIV_INTERN
7782
void
78-
log_crypt(byte* buf, ulint size, bool decrypt = false);
83+
log_crypt(byte* buf, lsn_t lsn, ulint size, bool decrypt = false);
7984

8085
#endif // log0crypt.h

storage/innobase/log/log0crypt.cc

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,12 @@ get_crypt_info(ulint checkpoint_no)
103103

104104
/** Encrypt or decrypt log blocks.
105105
@param[in,out] buf log blocks to encrypt or decrypt
106+
@param[in] lsn log sequence number of the start of the buffer
106107
@param[in] size size of the buffer, in bytes
107108
@param[in] decrypt whether to decrypt instead of encrypting */
108109
UNIV_INTERN
109110
void
110-
log_crypt(byte* buf, ulint size, bool decrypt)
111+
log_crypt(byte* buf, lsn_t lsn, ulint size, bool decrypt)
111112
{
112113
ut_ad(size % OS_FILE_LOG_BLOCK_SIZE == 0);
113114
ut_a(info.key_version);
@@ -117,12 +118,12 @@ log_crypt(byte* buf, ulint size, bool decrypt)
117118
compile_time_assert(sizeof(uint32_t) == 4);
118119

119120
#define LOG_CRYPT_HDR_SIZE 4
121+
lsn &= ~lsn_t(OS_FILE_LOG_BLOCK_SIZE - 1);
120122

121123
for (const byte* const end = buf + size; buf != end;
122-
buf += OS_FILE_LOG_BLOCK_SIZE) {
124+
buf += OS_FILE_LOG_BLOCK_SIZE, lsn += OS_FILE_LOG_BLOCK_SIZE) {
123125
uint32_t dst[(OS_FILE_LOG_BLOCK_SIZE - LOG_CRYPT_HDR_SIZE)
124126
/ sizeof(uint32_t)];
125-
const ulint log_block_no = log_block_get_hdr_no(buf);
126127

127128
/* The log block number is not encrypted. */
128129
*aes_ctr_iv =
@@ -137,10 +138,10 @@ log_crypt(byte* buf, ulint size, bool decrypt)
137138
# error "LOG_BLOCK_HDR_NO has been moved; redo log format affected!"
138139
#endif
139140
aes_ctr_iv[1] = info.crypt_nonce.word;
140-
mach_write_to_8(reinterpret_cast<byte*>(aes_ctr_iv + 2),
141-
log_block_get_start_lsn(
142-
decrypt ? srv_start_lsn : log_sys->lsn,
143-
log_block_no));
141+
mach_write_to_8(reinterpret_cast<byte*>(aes_ctr_iv + 2), lsn);
142+
ut_ad(log_block_get_start_lsn(lsn,
143+
log_block_get_hdr_no(buf))
144+
== lsn);
144145

145146
int rc = encryption_crypt(
146147
buf + LOG_CRYPT_HDR_SIZE, sizeof dst,
@@ -206,7 +207,11 @@ init_crypt_key(crypt_info_t* info, bool upgrade = false)
206207
return true;
207208
}
208209

209-
/** Initialize the redo log encryption key.
210+
/** Initialize the redo log encryption key and random parameters
211+
when creating a new redo log.
212+
The random parameters will be persisted in the log checkpoint pages.
213+
@see log_crypt_write_checkpoint_buf()
214+
@see log_crypt_read_checkpoint_buf()
210215
@return whether the operation succeeded */
211216
UNIV_INTERN
212217
bool

storage/innobase/log/log0log.cc

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -997,10 +997,6 @@ log_group_write_buf(
997997
|| log_block_get_hdr_no(buf)
998998
== log_block_convert_lsn_to_no(start_lsn));
999999

1000-
if (log_sys->is_encrypted()) {
1001-
log_crypt(buf, write_len);
1002-
}
1003-
10041000
/* Calculate the checksums for each log block and write them to
10051001
the trailer fields of the log blocks */
10061002

@@ -1264,6 +1260,12 @@ log_write_up_to(
12641260
::memset(write_buf + area_end, 0, pad_size);
12651261
}
12661262
}
1263+
1264+
if (log_sys->is_encrypted()) {
1265+
log_crypt(write_buf + area_start, log_sys->write_lsn,
1266+
area_end - area_start);
1267+
}
1268+
12671269
/* Do the write to the log files */
12681270
log_group_write_buf(
12691271
&log_sys->log, write_buf + area_start,

storage/innobase/log/log0recv.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,8 @@ log_group_read_log_seg(
715715
}
716716

717717
if (group->is_encrypted()) {
718-
log_crypt(buf, OS_FILE_LOG_BLOCK_SIZE, true);
718+
log_crypt(buf, start_lsn,
719+
OS_FILE_LOG_BLOCK_SIZE, true);
719720
}
720721
}
721722
}
@@ -1016,6 +1017,7 @@ recv_find_max_checkpoint(ulint* max_field)
10161017
buf + LOG_CHECKPOINT_LSN);
10171018
group->lsn_offset = mach_read_from_8(
10181019
buf + LOG_CHECKPOINT_OFFSET);
1020+
log_sys->next_checkpoint_no = checkpoint_no;
10191021
}
10201022
}
10211023

storage/innobase/srv/srv0start.cc

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2223,14 +2223,6 @@ innobase_start_or_create_for_mysql()
22232223

22242224
recv_sys->dblwr.pages.clear();
22252225

2226-
if (err == DB_SUCCESS && !srv_read_only_mode) {
2227-
log_mutex_enter();
2228-
if (log_sys->is_encrypted() && !log_crypt_init()) {
2229-
err = DB_ERROR;
2230-
}
2231-
log_mutex_exit();
2232-
}
2233-
22342226
if (err == DB_SUCCESS) {
22352227
/* Initialize the change buffer. */
22362228
err = dict_boot();
@@ -2733,13 +2725,6 @@ innobase_start_or_create_for_mysql()
27332725
fil_crypt_threads_init();
27342726
fil_system_exit();
27352727

2736-
/*
2737-
Create a checkpoint before logging anything new, so that
2738-
the current encryption key in use is definitely logged
2739-
before any log blocks encrypted with that key.
2740-
*/
2741-
log_make_checkpoint_at(LSN_MAX, TRUE);
2742-
27432728
/* Initialize online defragmentation. */
27442729
btr_defragment_init();
27452730
btr_defragment_thread_active = true;

0 commit comments

Comments
 (0)