Skip to content

Commit 36a9717

Browse files
committed
MDEV-13167 InnoDB key rotation is not skipping unused pages
In key rotation, we must initialize unallocated but previously initialized pages, so that if encryption is enabled on a table, all clear-text data for the page will eventually be overwritten. But we should not rotate keys on pages that were never allocated after the data file was created. According to the latching order rules, after acquiring the tablespace latch, no page latches of previously allocated user pages may be acquired. So, key rotation should check the page allocation status after acquiring the page latch, not before. But, the latching order rules also prohibit accessing pages that were not allocated first, and then acquiring the tablespace latch. Such behaviour would indeed result in a deadlock when running the following tests: encryption.innodb_encryption-page-compression encryption.innodb-checksum-algorithm Because the key rotation is accessing potentially unallocated pages, it cannot reliably check if these pages were allocated. It can only check the page header. If the page number is zero, we can assume that the page is unallocated. fil_crypt_rotate_pages(): Skip pages that are known to be uninitialized. fil_crypt_rotate_page(): Detect uninitialized pages by FIL_PAGE_OFFSET. Page 0 is never encrypted, and on other pages that are initialized, FIL_PAGE_OFFSET must contain the page number. fil_crypt_is_page_uninitialized(): Remove. It suffices to check the page number field in fil_crypt_rotate_page().
1 parent e52dd13 commit 36a9717

File tree

1 file changed

+41
-43
lines changed

1 file changed

+41
-43
lines changed

storage/innobase/fil/fil0crypt.cc

Lines changed: 41 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,20 +1596,6 @@ fil_crypt_find_page_to_rotate(
15961596
return found;
15971597
}
15981598

1599-
/***********************************************************************
1600-
Check if a page is uninitialized (doesn't need to be rotated)
1601-
@param[in] frame Page to check
1602-
@param[in] page_size Page size
1603-
@return true if page is uninitialized, false if not. */
1604-
static inline
1605-
bool
1606-
fil_crypt_is_page_uninitialized(
1607-
const byte *frame,
1608-
const page_size_t& page_size)
1609-
{
1610-
return (buf_page_is_zeroes(frame, page_size));
1611-
}
1612-
16131599
#define fil_crypt_get_page_throttle(state,offset,mtr,sleeptime_ms) \
16141600
fil_crypt_get_page_throttle_func(state, offset, mtr, \
16151601
sleeptime_ms, __FILE__, __LINE__)
@@ -1770,9 +1756,9 @@ fil_crypt_rotate_page(
17701756
ulint offset = state->offset;
17711757
ulint sleeptime_ms = 0;
17721758
fil_space_crypt_t *crypt_data = space->crypt_data;
1773-
const page_size_t page_size = page_size_t(space->flags);
17741759

17751760
ut_ad(space->n_pending_ops > 0);
1761+
ut_ad(offset > 0);
17761762

17771763
/* In fil_crypt_thread where key rotation is done we have
17781764
acquired space and checked that this space is not yet
@@ -1787,44 +1773,55 @@ fil_crypt_rotate_page(
17871773
return;
17881774
}
17891775

1776+
ut_d(const bool was_free = fseg_page_is_free(space, offset));
1777+
17901778
mtr_t mtr;
17911779
mtr.start();
17921780
if (buf_block_t* block = fil_crypt_get_page_throttle(state,
17931781
offset, &mtr,
17941782
&sleeptime_ms)) {
1795-
mtr.set_named_space(space);
1796-
17971783
bool modified = false;
17981784
int needs_scrubbing = BTR_SCRUB_SKIP_PAGE;
17991785
lsn_t block_lsn = block->page.newest_modification;
18001786
byte* frame = buf_block_get_frame(block);
18011787
uint kv = mach_read_from_4(frame+FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION);
18021788

1803-
/* check if tablespace is closing after reading page */
1804-
if (!space->is_stopping()) {
1805-
1806-
if (kv == 0 &&
1807-
fil_crypt_is_page_uninitialized(frame, page_size)) {
1808-
;
1809-
} else if (fil_crypt_needs_rotation(
1810-
crypt_data->encryption,
1811-
kv, key_state->key_version,
1812-
key_state->rotate_key_age)) {
1813-
1814-
modified = true;
1815-
1816-
/* force rotation by dummy updating page */
1817-
mlog_write_ulint(frame +
1818-
FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID,
1819-
space_id, MLOG_4BYTES, &mtr);
1820-
1821-
/* statistics */
1822-
state->crypt_stat.pages_modified++;
1823-
} else {
1824-
if (crypt_data->is_encrypted()) {
1825-
if (kv < state->min_key_version_found) {
1826-
state->min_key_version_found = kv;
1827-
}
1789+
if (space->is_stopping()) {
1790+
/* The tablespace is closing (in DROP TABLE or
1791+
TRUNCATE TABLE or similar): avoid further access */
1792+
} else if (!*reinterpret_cast<uint32_t*>(FIL_PAGE_OFFSET
1793+
+ frame)) {
1794+
/* It looks like this page was never
1795+
allocated. Because key rotation is accessing
1796+
pages in a pattern that is unlike the normal
1797+
B-tree and undo log access pattern, we cannot
1798+
invoke fseg_page_is_free() here, because that
1799+
could result in a deadlock. If we invoked
1800+
fseg_page_is_free() and released the
1801+
tablespace latch before acquiring block->lock,
1802+
then the fseg_page_is_free() information
1803+
could be stale already. */
1804+
ut_ad(was_free);
1805+
ut_ad(kv == 0);
1806+
ut_ad(page_get_space_id(frame) == 0);
1807+
} else if (fil_crypt_needs_rotation(
1808+
crypt_data->encryption,
1809+
kv, key_state->key_version,
1810+
key_state->rotate_key_age)) {
1811+
1812+
mtr.set_named_space(space);
1813+
modified = true;
1814+
1815+
/* force rotation by dummy updating page */
1816+
mlog_write_ulint(frame + FIL_PAGE_SPACE_ID,
1817+
space_id, MLOG_4BYTES, &mtr);
1818+
1819+
/* statistics */
1820+
state->crypt_stat.pages_modified++;
1821+
} else {
1822+
if (crypt_data->is_encrypted()) {
1823+
if (kv < state->min_key_version_found) {
1824+
state->min_key_version_found = kv;
18281825
}
18291826
}
18301827

@@ -1934,7 +1931,8 @@ fil_crypt_rotate_pages(
19341931
rotate_thread_t* state)
19351932
{
19361933
ulint space = state->space->id;
1937-
ulint end = state->offset + state->batch;
1934+
ulint end = std::min(state->offset + state->batch,
1935+
state->space->free_limit);
19381936

19391937
ut_ad(state->space->n_pending_ops > 0);
19401938

0 commit comments

Comments
 (0)