From ce46c8a7f53b4a7cd39f2bad6c5695502af04d52 Mon Sep 17 00:00:00 2001 From: KhaledR57 Date: Wed, 12 Aug 2026 19:02:30 +0300 Subject: [PATCH] MDEV-40495 KEY_OP_SHIFT redo moves data past the page buffer The KEY_OP_SHIFT branch of _ma_apply_redo_index() took the shift length straight from the redo record and used it to form a bmove() source and size, and to update page_length. The only guards were DBUG_ASSERTs, which are compiled out when DBUG_OFF is set. A corrupt record with a negative length could therefore move data from outside the page and wrap page_length. Turn both asserts into runtime checks. The page offset must be set and inside the used page, the resulting page length must still fit the page, and for a negative shift the source must stay inside the used page too. The conditions are the ones the asserts already tested, so debug builds keep the same behaviour. This also fixes MDEV-40496, which covers the page_length side of the same branch. The first check bounds it. The test forges the logged shift length with two new debug keywords, corrupt_shift_down and corrupt_shift_up, then crashes the server so recovery has to replay the record. --- .../maria/maria-recovery-corrupt-shift.result | 38 +++++ .../maria/maria-recovery-corrupt-shift.test | 136 ++++++++++++++++++ storage/maria/ma_key_recover.c | 23 ++- 3 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 mysql-test/suite/maria/maria-recovery-corrupt-shift.result create mode 100644 mysql-test/suite/maria/maria-recovery-corrupt-shift.test diff --git a/mysql-test/suite/maria/maria-recovery-corrupt-shift.result b/mysql-test/suite/maria/maria-recovery-corrupt-shift.result new file mode 100644 index 0000000000000..5368f5eedd814 --- /dev/null +++ b/mysql-test/suite/maria/maria-recovery-corrupt-shift.result @@ -0,0 +1,38 @@ +call mtr.add_suppression("File '.*aria_log.000.*' not found"); +drop database if exists mysqltest; +create database mysqltest; +connect admin, localhost, root,,mysqltest,,; +connection default; +use mysqltest; +connection default; +connection admin; +* shut down mysqld, removed logs, restarted it +connection default; +create table t1 (a int, b varchar(60), key(a), key(b)) +transactional=1 row_format=page engine=aria; +set session debug_dbug="+d,corrupt_shift_down"; +select count(*) from t1; +count(*) +200 +set session debug_dbug="+d,maria_flush_whole_log,maria_crash"; +set global aria_checkpoint_interval=1; +ERROR HY000: Lost connection to server during query +# Recovery must refuse the record, not apply it and notice later. +FOUND 1 /Aria engine: Redo phase failed/ in mdev40495.err +NOT FOUND /Table file is corrupted/ in mdev40495.err +use mysqltest; +# +# MDEV-40496: a positive length pushes page_length past the page. +# +create table t2 (a int, b varchar(60), key(a), key(b)) +transactional=1 row_format=page engine=aria; +set session debug_dbug="+d,corrupt_shift_up"; +select count(*) from t2; +count(*) +200 +set session debug_dbug="+d,maria_flush_whole_log,maria_crash"; +set global aria_checkpoint_interval=1; +ERROR HY000: Lost connection to server during query +FOUND 1 /Aria engine: Redo phase failed/ in mdev40496.err +NOT FOUND /Table file is corrupted/ in mdev40496.err +drop database mysqltest; diff --git a/mysql-test/suite/maria/maria-recovery-corrupt-shift.test b/mysql-test/suite/maria/maria-recovery-corrupt-shift.test new file mode 100644 index 0000000000000..8c0ab03449fc1 --- /dev/null +++ b/mysql-test/suite/maria/maria-recovery-corrupt-shift.test @@ -0,0 +1,136 @@ +# MDEV-40495/MDEV-40496: _ma_apply_redo_index() fed the redo record's shift +# length to bmove() and to page_length guarded only by DBUG_ASSERTs, which are +# compiled out in release builds. A negative length could therefore move data +# from outside the page and wrap page_length. + +--source include/not_embedded.inc +--source include/have_debug.inc +--source include/have_maria.inc +# the server is killed on purpose below +--source include/not_valgrind.inc + +call mtr.add_suppression("File '.*aria_log.000.*' not found"); + +let $MYSQLD_DATADIR= `select @@datadir`; +let $MARIA_LOG=.; + +--disable_warnings +drop database if exists mysqltest; +--enable_warnings +create database mysqltest; + +# the log is shared with earlier tests, and recovery below replays all of it +connect (admin, localhost, root,,mysqltest,,); +--enable_reconnect +connection default; +use mysqltest; +--enable_reconnect +--source include/maria_empty_logs.inc + +create table t1 (a int, b varchar(60), key(a), key(b)) +transactional=1 row_format=page engine=aria; + +# Forge the length of every KEY_OP_SHIFT written below. Any key insert that +# lands in the middle of an index page produces one. +# +# -current_size keeps page_length + length at 0, so the record clears the +# first check and reaches the one that bounds the bmove source. +set session debug_dbug="+d,corrupt_shift_down"; + +# Scattered keys on purpose so inserts land mid-page and have to shift. +--disable_query_log +let $i= 40; +while ($i) +{ + eval insert into t1 (a,b) values ($i*7919%10007,concat('v',$i*7)),($i*104729%10007,concat('w',$i*3)),($i*7919%9973,concat('x',$i*11)),($i*104729%9973,concat('y',$i*13)),($i*7919%9967,concat('z',$i*17)); + dec $i; +} +--enable_query_log +select count(*) from t1; + +# Kill without flushing the page cache: the tables stay behind the log, so +# recovery must replay the forged records instead of skipping them +--write_line wait $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +set session debug_dbug="+d,maria_flush_whole_log,maria_crash"; +--error 2013 +set global aria_checkpoint_interval=1; + +--echo # Recovery must refuse the record, not apply it and notice later. +# Failed Aria recovery aborts the server, so --bootstrap drives recovery and +# exits by itself. --log-error keeps the failure out of the shared error log. +--write_file $MYSQLTEST_VARDIR/tmp/mdev40495_boot.sql +select 1; +EOF + +--error 1 +--exec $MYSQLD_CMD --bootstrap --log-error=$MYSQLTEST_VARDIR/tmp/mdev40495.err < $MYSQLTEST_VARDIR/tmp/mdev40495_boot.sql > $MYSQLTEST_VARDIR/tmp/mdev40495.out 2>&1 + +--let SEARCH_FILE= $MYSQLTEST_VARDIR/tmp/mdev40495.err +--let SEARCH_PATTERN= Aria engine: Redo phase failed +--source include/search_pattern_in_file.inc + +# Unfixed, the bmove() happens and the damage surfaces later as error 127. +# Refusing up front means this stays NOT FOUND. +--let SEARCH_PATTERN= Table file is corrupted +--source include/search_pattern_in_file.inc + +# every shift in this log is forged, so it can never be replayed +--error 0,1 +remove_files_wildcard $MYSQLD_DATADIR aria_log.0*; + +--write_line restart $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +--enable_reconnect +--source include/wait_until_connected_again.inc + +use mysqltest; + +--echo # +--echo # MDEV-40496: a positive length pushes page_length past the page. +--echo # + +create table t2 (a int, b varchar(60), key(a), key(b)) +transactional=1 row_format=page engine=aria; + +# max_page_size is caught by the first check instead, the one that bounds +# page_length itself. Nothing is moved, the record never gets that far. +set session debug_dbug="+d,corrupt_shift_up"; + +--disable_query_log +let $i= 40; +while ($i) +{ + eval insert into t2 (a,b) values ($i*7919%10007,concat('v',$i*7)),($i*104729%10007,concat('w',$i*3)),($i*7919%9973,concat('x',$i*11)),($i*104729%9973,concat('y',$i*13)),($i*7919%9967,concat('z',$i*17)); + dec $i; +} +--enable_query_log +select count(*) from t2; + +--write_line wait $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +set session debug_dbug="+d,maria_flush_whole_log,maria_crash"; +--error 2013 +set global aria_checkpoint_interval=1; + +--error 1 +--exec $MYSQLD_CMD --bootstrap --log-error=$MYSQLTEST_VARDIR/tmp/mdev40496.err < $MYSQLTEST_VARDIR/tmp/mdev40495_boot.sql > $MYSQLTEST_VARDIR/tmp/mdev40496.out 2>&1 + +--let SEARCH_FILE= $MYSQLTEST_VARDIR/tmp/mdev40496.err +--let SEARCH_PATTERN= Aria engine: Redo phase failed +--source include/search_pattern_in_file.inc + +--let SEARCH_PATTERN= Table file is corrupted +--source include/search_pattern_in_file.inc + +--error 0,1 +remove_files_wildcard $MYSQLD_DATADIR aria_log.0*; + +--write_line restart $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +--enable_reconnect +--source include/wait_until_connected_again.inc + +drop database mysqltest; + +remove_file $MYSQLTEST_VARDIR/tmp/mdev40495.err; +remove_file $MYSQLTEST_VARDIR/tmp/mdev40495.out; +remove_file $MYSQLTEST_VARDIR/tmp/mdev40496.err; +remove_file $MYSQLTEST_VARDIR/tmp/mdev40496.out; +remove_file $MYSQLTEST_VARDIR/tmp/mdev40495_boot.sql; diff --git a/storage/maria/ma_key_recover.c b/storage/maria/ma_key_recover.c index acec592b9227a..3f62e429aa798 100644 --- a/storage/maria/ma_key_recover.c +++ b/storage/maria/ma_key_recover.c @@ -584,7 +584,11 @@ my_bool _ma_log_add(MARIA_PAGE *ma_page, } } log_pos[0]= KEY_OP_SHIFT; - int2store(log_pos+1, move_length); + /* Only the logged length is forged; the page itself is already moved */ + int2store(log_pos+1, + DBUG_IF("corrupt_shift_down") ? -(int) current_size : + DBUG_IF("corrupt_shift_up") ? (int) max_page_size : + move_length); log_pos+= 3; current_size+= move_length; } @@ -998,12 +1002,23 @@ uint _ma_apply_redo_index(MARIA_HA *info, int length= sint2korr(header); header+= 2; DBUG_PRINT("redo", ("key_op_shift: %d", length)); - DBUG_ASSERT(page_offset != 0 && page_offset <= page_length && - page_length + length <= max_page_size); + if (unlikely(page_offset == 0 || page_offset > page_length || + page_length + length > max_page_size)) + { + DBUG_ASSERT(!maria_assert_if_crashed_table); + result= mark_crashed= 1; + goto err; + } if (length < 0) { - DBUG_ASSERT(page_offset - length <= page_length); + /* Shifting down: the source must still be inside the used page */ + if (unlikely(page_offset - length > page_length)) + { + DBUG_ASSERT(!maria_assert_if_crashed_table); + result= mark_crashed= 1; + goto err; + } bmove(buff + page_offset, buff + page_offset - length, page_length - page_offset + length); }