From de8d57e5220ccfab31db4da86167f441df78869b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 11 Aug 2020 15:49:37 +0300 Subject: [PATCH] MDEV-23447 SIGSEGV in fil_system_t::keyrotate_next() fil_system_t::keyrotate_next(): If space && space->is_in_rotation_list does not hold, iterate from the start of the list. In debug builds, we would typically have hit SIGSEGV because the iterator would have wrapped a null pointer. It might also be that we are dereferencing a stale pointer. There is no test case, because the encryption is very nondeterministic in nature, due to the use of background threads. This scenario can be hit by setting the following: SET GLOBAL innodb_encryption_threads=5; SET GLOBAL innodb_encryption_rotate_key_age=0; --- storage/innobase/fil/fil0crypt.cc | 35 ++++++++++++++++++------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index c687c53ac14..09f2730edec 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -1388,29 +1388,34 @@ the encryption parameters were changed inline fil_space_t *fil_system_t::keyrotate_next(fil_space_t *space, bool recheck, bool encrypt) { - ut_ad(mutex_own(&fil_system->mutex)); + ut_ad(mutex_own(&mutex)); sized_ilist::iterator it= - space ? space : fil_system->rotation_list.begin(); + space && space->is_in_rotation_list ? space : rotation_list.begin(); const sized_ilist::iterator end= - fil_system->rotation_list.end(); + rotation_list.end(); if (space) { - while (++it != end && (!UT_LIST_GET_LEN(it->chain) || it->is_stopping())); + const bool released= !--space->n_pending_ops; - /* If one of the encryption threads already started the encryption - of the table then don't remove the unencrypted spaces from rotation list - - If there is a change in innodb_encrypt_tables variables value then - don't remove the last processed tablespace from the rotation list. */ - if (!--space->n_pending_ops && - (!recheck || space->crypt_data) && !encrypt == !srv_encrypt_tables && - space->is_in_rotation_list) + if (space->is_in_rotation_list) { - ut_a(!fil_system->rotation_list.empty()); - fil_system->rotation_list.remove(*space); - space->is_in_rotation_list= false; + while (++it != end && + (!UT_LIST_GET_LEN(it->chain) || it->is_stopping())); + + /* If one of the encryption threads already started the encryption + of the table then don't remove the unencrypted spaces from rotation list + + If there is a change in innodb_encrypt_tables variables value then + don't remove the last processed tablespace from the rotation list. */ + if (released && (!recheck || space->crypt_data) && + !encrypt == !srv_encrypt_tables) + { + ut_a(!rotation_list.empty()); + rotation_list.remove(*space); + space->is_in_rotation_list= false; + } } }