From 618862c6b5f3b7bc8bbe87bce2d2cee96cdefe26 Mon Sep 17 00:00:00 2001 From: KhaledR57 Date: Tue, 11 Aug 2026 20:31:01 +0300 Subject: [PATCH] MDEV-40493 _ma_apply_redo_index: KEY_OP_MULTI_COPY memcpy uses unchecked offsets The KEY_OP_MULTI_COPY branch passed full_length and every (to,from) pair from the redo record straight to memcpy(), guarded only by DBUG_ASSERTs. Those are compiled out when DBUG_OFF is set, and even in debug builds they check "to" and full_length against max_page_size separately, never their sum. A corrupt record could therefore copy past the page buffer into a neighbouring cached page, while recovery reported success. Add runtime bounds checks. full_length must fit in the page, the (to,from) list must stay inside the record and hold whole pairs, and each pair's larger offset plus full_length must stay in the page. A refused record takes the error path that was already there, which marks the table crashed and fails the redo phase. The test forces an RTREE split, the only producer of this record type, forges the logged "to" offset with corrupt_multi_copy_to debug keyword, then crashes the server so recovery has to replay the record. --- .../maria-recovery-corrupt-multi-copy.result | 23 +++++ .../maria-recovery-corrupt-multi-copy.test | 90 +++++++++++++++++++ storage/maria/ma_key_recover.c | 16 +++- storage/maria/ma_rt_split.c | 5 +- 4 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 mysql-test/suite/maria/maria-recovery-corrupt-multi-copy.result create mode 100644 mysql-test/suite/maria/maria-recovery-corrupt-multi-copy.test diff --git a/mysql-test/suite/maria/maria-recovery-corrupt-multi-copy.result b/mysql-test/suite/maria/maria-recovery-corrupt-multi-copy.result new file mode 100644 index 0000000000000..0b6a44bca9768 --- /dev/null +++ b/mysql-test/suite/maria/maria-recovery-corrupt-multi-copy.result @@ -0,0 +1,23 @@ +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 (line point not null, name varchar(32), spatial key (line)) +transactional=1 row_format=page engine=aria; +set session debug_dbug="+d,corrupt_multi_copy_to"; +select count(*) from t1; +count(*) +450 +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 mdev40493.err +NOT FOUND /Table file is corrupted/ in mdev40493.err +drop database mysqltest; diff --git a/mysql-test/suite/maria/maria-recovery-corrupt-multi-copy.test b/mysql-test/suite/maria/maria-recovery-corrupt-multi-copy.test new file mode 100644 index 0000000000000..bf2ebd622c214 --- /dev/null +++ b/mysql-test/suite/maria/maria-recovery-corrupt-multi-copy.test @@ -0,0 +1,90 @@ +# MDEV-40493: _ma_apply_redo_index() fed the redo record's to/from/full_length +# to memcpy() guarded only by DBUG_ASSERTs, which are compiled out in release +# builds and never checked to+full_length in any build. A corrupted record +# could therefore write past the page buffer while recovery reported success. + +--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 (line point not null, name varchar(32), spatial key (line)) +transactional=1 row_format=page engine=aria; + +# Forge "to" in the KEY_OP_MULTI_COPY records the splits below write. +# An RTREE split is the only producer of that record type. +# +# to+full_length overflows the page while to alone stays in range, the case +# the old asserts missed. Kept small on purpose so an unfixed server +# survives and the two versions differ. +set session debug_dbug="+d,corrupt_multi_copy_to"; + +# ~220 keys fit an 8K index page, so 450 rows force splits. +# Chunked INSERT VALUES on purpose: bulk inserts do not log every change. +--disable_query_log +let $i= 45; +while ($i) +{ + eval insert into t1 (line,name) values (point($i,1),'a'),(point($i,2),'b'),(point($i,3),'c'),(point($i,4),'d'),(point($i,5),'e'),(point($i,6),'f'),(point($i,7),'g'),(point($i,8),'h'),(point($i,9),'i'),(point($i,10),'j'); + 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 as already applied +--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/mdev40493_boot.sql +select 1; +EOF + +--error 1 +--exec $MYSQLD_CMD --bootstrap --log-error=$MYSQLTEST_VARDIR/tmp/mdev40493.err < $MYSQLTEST_VARDIR/tmp/mdev40493_boot.sql > $MYSQLTEST_VARDIR/tmp/mdev40493.out 2>&1 + +--let SEARCH_FILE= $MYSQLTEST_VARDIR/tmp/mdev40493.err +--let SEARCH_PATTERN= Aria engine: Redo phase failed +--source include/search_pattern_in_file.inc + +# Unfixed, the memcpy() 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 split in this log is forged, so it can never be replayed +remove_file $MYSQLD_DATADIR/aria_log.00000001; + +--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/mdev40493.err; +remove_file $MYSQLTEST_VARDIR/tmp/mdev40493.out; +remove_file $MYSQLTEST_VARDIR/tmp/mdev40493_boot.sql; diff --git a/storage/maria/ma_key_recover.c b/storage/maria/ma_key_recover.c index acec592b9227a..2b6f98180b711 100644 --- a/storage/maria/ma_key_recover.c +++ b/storage/maria/ma_key_recover.c @@ -1142,7 +1142,14 @@ uint _ma_apply_redo_index(MARIA_HA *info, log_memcpy_length= uint2korr(header); header+= 2; log_memcpy_end= header + log_memcpy_length; - DBUG_ASSERT(full_length <= max_page_size); + if (unlikely(full_length > max_page_size || + log_memcpy_end > header_end || + (log_memcpy_length & 3))) + { + DBUG_ASSERT(!maria_assert_if_crashed_table); + result= mark_crashed= 1; + goto err; + } while (header < log_memcpy_end) { uint to, from; @@ -1151,7 +1158,12 @@ uint _ma_apply_redo_index(MARIA_HA *info, from= uint2korr(header); header+= 2; /* "from" is a place in the existing page */ - DBUG_ASSERT(MY_MAX(from, to) < max_page_size); + if (unlikely(MY_MAX(from, to) + full_length > max_page_size)) + { + DBUG_ASSERT(!maria_assert_if_crashed_table); + result= mark_crashed= 1; + goto err; + } memcpy(buff + to, buff + from, full_length); } break; diff --git a/storage/maria/ma_rt_split.c b/storage/maria/ma_rt_split.c index a0acb9ce34d51..fc34a329e6e5c 100644 --- a/storage/maria/ma_rt_split.c +++ b/storage/maria/ma_rt_split.c @@ -485,7 +485,10 @@ int maria_rtree_split_page(const MARIA_KEY *key, MARIA_PAGE *page, { /* this memcpy() is internal to the page (source in the page) */ size_t cur_key_with_nod_flag_offs= cur_key_with_nod_flag - page->buff; - int2store(log_internal_copy_ptr, to_with_nod_flag_offs); + /* Only the logged offset is forged; the split is already done */ + int2store(log_internal_copy_ptr, + DBUG_IF("corrupt_multi_copy_to") ? + share->max_index_block_size : to_with_nod_flag_offs); log_internal_copy_ptr+= 2; int2store(log_internal_copy_ptr, cur_key_with_nod_flag_offs); log_internal_copy_ptr+= 2;