MDEV-40493 _ma_apply_redo_index: KEY_OP_MULTI_COPY memcpy uses unchecked offsets (page buffer OOB write)

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;

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;

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;

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;