From a083eb941f442e3d9783ef8b849079546e24e590 Mon Sep 17 00:00:00 2001 From: Brandon Nesterenko Date: Thu, 6 Aug 2026 13:39:51 -0600 Subject: [PATCH 1/2] MDEV-40492 (Regression): Oversized Fake Rotate Event can Crash Slave --- .../rpl/t/rpl_rotate_log_event_overflow.test | 182 ++++++++++++++++++ sql/sql_repl.cc | 18 ++ 2 files changed, 200 insertions(+) create mode 100644 mysql-test/suite/rpl/t/rpl_rotate_log_event_overflow.test diff --git a/mysql-test/suite/rpl/t/rpl_rotate_log_event_overflow.test b/mysql-test/suite/rpl/t/rpl_rotate_log_event_overflow.test new file mode 100644 index 0000000000000..3134b475bd2be --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_rotate_log_event_overflow.test @@ -0,0 +1,182 @@ +# +# This test verifies that a replica rejects a fake Rotate event that names +# a binary log file longer than the longest file name a replica can hold. The +# replica rewrites the checksum of that event through a buffer of its own +# whenever the master's binlog_checksum setting differs from the checksum its +# relay log carries. Both directions of that difference are covered, and +# replication must be able to resume after each. The replica connects by +# binlog file position, so each resume also verifies the rejected event left +# those coordinates untouched. +# +# MDEV-40492 reported that the replica sized the copy into that buffer from +# the length the master sent, and overran it. +# +# Note the relay log takes its checksum from the last format description +# event the replica queued, so each case changes binlog_checksum on the master +# while the IO thread is stopped. That is, the fake Rotate of the next +# connection arrives ahead of the format description event that settles the +# difference again. +# +# References: +# * MDEV-40492: queue_event: fake ROTATE checksum adjust copies into fixed +# rot_buf without bounding event_len (stack OOB) +# * MDEV-39689: Slave Overflow on Malformed Table_map_log_event +# + +--source include/have_debug.inc +# Test is format independent +--source include/have_binlog_format_row.inc +--source include/master-slave.inc + +--connection slave +call mtr.add_suppression("Rotate event from master names a binary log file longer"); +call mtr.add_suppression("Relay log write failure: could not queue event from master"); + +--echo # +--echo # Initialize test data +--echo # +--connection master +create table t1 (a int); +insert into t1 values (1); +--source include/save_master_gtid.inc + +--connection slave +--source include/sync_with_master_gtid.inc + +--connection master +set @saved_dbug= @@global.debug_dbug; +set @saved_checksum= @@global.binlog_checksum; + +--echo # +--echo # Test Case 1: the master adds no checksum and the relay log carries one +--echo # + +# Replicating from a master that adds a checksum gives the relay log one. +--connection master +set @@global.binlog_checksum= CRC32; +insert into t1 values (2); +--source include/save_master_gtid.inc + +--connection slave +--source include/sync_with_master_gtid.inc +--source include/stop_slave.inc + +# The dump thread reads the injection when the replica reconnects below. +--connection master +set @@global.binlog_checksum= NONE; +set @@global.debug_dbug= "+d,binlog_sender_oversized_fake_rotate"; + +# The IO thread can reject the event before include/start_slave.inc observes +# the thread running, and the include reports a nonzero Last_IO_Errno as a +# failure of its own. +--connection slave +START SLAVE; + +--echo # Waiting for the IO thread to reject the oversized Rotate event +# queue_event() reports 1593, then handle_slave_io() overwrites it with 1595 +--let $slave_io_errno= 1595 +--let $show_slave_io_error= 1 +# A setup that leaves the master and the relay log agreeing shows up here as +# a timeout. Keep that timeout short rather than the default of five minutes. +--let $slave_timeout= 30 +--source include/wait_for_slave_io_error.inc +--let $slave_timeout= + +# The reported maximum bounds a name of FN_REFLEN bytes behind the same headers +--echo # Ensure the event was rejected before the replica copied it +--let $assert_text= The IO thread reported an oversized Rotate event +--let $assert_file= $MYSQLTEST_VARDIR/log/mysqld.2.err +--let $assert_select= Rotate event from master names a binary log file longer than the maximum file name length;.*the maximum: 539 +--let $assert_count= 1 +--source include/assert_grep.inc + +# Restore the master's checksum before the IO thread reconnects. The relay log +# takes its checksum from the next format description event, so the reconnect +# leaves the master and the relay log agreeing again. +--connection master +set @@global.debug_dbug= @saved_dbug; +set @@global.binlog_checksum= @saved_checksum; + +--connection slave +--source include/stop_slave_sql.inc +--source include/start_slave.inc + +--echo # +--echo # Test Case 2: the master adds a checksum and the relay log carries none +--echo # + +# Replicating from a master that adds no checksum leaves the relay log +# without one. +--connection master +set @@global.binlog_checksum= NONE; +insert into t1 values (3); +--source include/save_master_gtid.inc + +--connection slave +--source include/sync_with_master_gtid.inc +--source include/stop_slave.inc + +# The dump thread reads the injection when the replica reconnects below. +--connection master +set @@global.binlog_checksum= CRC32; +set @@global.debug_dbug= "+d,binlog_sender_oversized_fake_rotate"; + +# The IO thread can reject the event before include/start_slave.inc observes +# the thread running, and the include reports a nonzero Last_IO_Errno as a +# failure of its own. +--connection slave +START SLAVE; + +--echo # Waiting for the IO thread to reject the oversized Rotate event +--let $slave_io_errno= 1595 +--let $show_slave_io_error= 1 +# A setup that leaves the master and the relay log agreeing shows up here as +# a timeout. Keep that timeout short rather than the default of five minutes. +--let $slave_timeout= 30 +--source include/wait_for_slave_io_error.inc +--let $slave_timeout= + +# The reported maximum bounds a name of FN_REFLEN bytes behind the same +# headers, plus that checksum. +--echo # Ensure the event was rejected before the replica copied it +--let $assert_text= The IO thread reported an oversized Rotate event +--let $assert_file= $MYSQLTEST_VARDIR/log/mysqld.2.err +--let $assert_select= Rotate event from master names a binary log file longer than the maximum file name length;.*the maximum: 543 +--let $assert_count= 1 +--source include/assert_grep.inc + +# Restore the master's checksum before the IO thread reconnects. The relay log +# takes its checksum from the next format description event, so the reconnect +# leaves the master and the relay log agreeing again. +--connection master +set @@global.debug_dbug= @saved_dbug; +set @@global.binlog_checksum= @saved_checksum; + +--connection slave +--source include/stop_slave_sql.inc +--source include/start_slave.inc + +--echo # +--echo # Ensure replication works after the oversized Rotate events +--echo # +--connection master +insert into t1 values (4); +--source include/save_master_gtid.inc + +--connection slave +--source include/sync_with_master_gtid.inc +--let $diff_tables=master:t1,slave:t1 +--source include/diff_tables.inc + +--echo # +--echo # Cleanup +--echo # +--connection master +drop table t1; +--source include/save_master_gtid.inc + +--connection slave +--source include/sync_with_master_gtid.inc + +--source include/rpl_end.inc +--echo # End of rpl_rotate_log_event_overflow.test diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc index 72626f162d8f3..20c5dbbe9e885 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -223,6 +223,24 @@ static int fake_rotate_event(binlog_send_info *info, ulonglong position, String *packet= info->packet; ha_checksum crc= 0; +#ifndef DBUG_OFF + /* + Name a binary log file longer than any file name a replica can hold, to + test that a replica bounds the event before copying it into a buffer of + its own. The length is a large multiple of FN_REFLEN rather than one byte + over it, so that the copy of a replica missing the bound runs past its + whole stack frame. An overrun of a few hundred bytes only reaches the + buffers next to the target, which that replica never reads on this path. + The name is replaced here, ahead of the header, so that both the event + length the header carries and the checksum below cover it. + */ + char oversized_ident[32 * FN_REFLEN]; + DBUG_EXECUTE_IF("binlog_sender_oversized_fake_rotate", + memset(oversized_ident, 'a', sizeof(oversized_ident)); + p= oversized_ident; + ident_len= (uint) sizeof(oversized_ident);); +#endif + /* reset transmit packet for the fake rotate event below */ if (reset_transmit_packet(info, info->flags, &ev_offset, &info->errmsg)) DBUG_RETURN(1); From 87e8fcfe746a2ce20e2fe55f6f151f853ce9818f Mon Sep 17 00:00:00 2001 From: Brandon Nesterenko Date: Thu, 6 Aug 2026 13:41:41 -0600 Subject: [PATCH 2/2] MDEV-40492: Oversized Fake Rotate Event can Crash Slave A slave can crash when the master's binlog_checksum setting differs from the checksum the slave's relay log carries. On the first Rotate event of such a connection, the slave IO thread copies the event onto a stack buffer sized for the longest binary log file name a master can send. A master that names a longer file makes the copy run off the end of that buffer, and what it writes past the end is the file name itself. A name that overruns the buffer by a few hundred bytes reaches only the other buffers of the same stack frame, which the slave does not read on this path, so it keeps running. A name that overruns the whole frame takes the return address with it, and the slave crashes. Builds with AddressSanitizer stop at the copy and report a stack-buffer-overflow. The fake Rotate event that opens a connection is rewritten when the master's checksum policy and the relay log's disagree. One case adds a checksum to the event, and the other strips one. Both copy the event into rot_buf using the length the master sent, and neither compared that length against the size of rot_buf. The Rotate_log_event constructed just above bounds its own copy of the file name at FN_REFLEN-1, which leaves the length the master sent untouched. rot_buf was itself one checksum shorter than the event the first case produces from the longest file name. Bound the event before either case copies it. queue_event() now compares the event's length against the longest Rotate a master can send: the two headers, a file name of at most FN_REFLEN bytes, and the checksum the master's own policy puts on the event. rot_buf now reserves that checksum length as well. A Rotate event naming a longer file stops the slave IO thread with an error reporting the length the master sent, instead of running off the end of the buffer. Reviewed-by: TODO Signed-off-by: Brandon Nesterenko --- .../r/rpl_rotate_log_event_overflow.result | 88 +++++++++++++++++++ sql/slave.cc | 51 +++++++++-- 2 files changed, 133 insertions(+), 6 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rpl_rotate_log_event_overflow.result diff --git a/mysql-test/suite/rpl/r/rpl_rotate_log_event_overflow.result b/mysql-test/suite/rpl/r/rpl_rotate_log_event_overflow.result new file mode 100644 index 0000000000000..3ece7ae7db7fb --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_rotate_log_event_overflow.result @@ -0,0 +1,88 @@ +include/master-slave.inc +[connection master] +connection slave; +call mtr.add_suppression("Rotate event from master names a binary log file longer"); +call mtr.add_suppression("Relay log write failure: could not queue event from master"); +# +# Initialize test data +# +connection master; +create table t1 (a int); +insert into t1 values (1); +include/save_master_gtid.inc +connection slave; +include/sync_with_master_gtid.inc +connection master; +set @saved_dbug= @@global.debug_dbug; +set @saved_checksum= @@global.binlog_checksum; +# +# Test Case 1: the master adds no checksum and the relay log carries one +# +connection master; +set @@global.binlog_checksum= CRC32; +insert into t1 values (2); +include/save_master_gtid.inc +connection slave; +include/sync_with_master_gtid.inc +include/stop_slave.inc +connection master; +set @@global.binlog_checksum= NONE; +set @@global.debug_dbug= "+d,binlog_sender_oversized_fake_rotate"; +connection slave; +START SLAVE; +# Waiting for the IO thread to reject the oversized Rotate event +include/wait_for_slave_io_error.inc [errno=1595] +Last_IO_Error = 'Relay log write failure: could not queue event from master' +# Ensure the event was rejected before the replica copied it +include/assert_grep.inc [The IO thread reported an oversized Rotate event] +connection master; +set @@global.debug_dbug= @saved_dbug; +set @@global.binlog_checksum= @saved_checksum; +connection slave; +include/stop_slave_sql.inc +include/start_slave.inc +# +# Test Case 2: the master adds a checksum and the relay log carries none +# +connection master; +set @@global.binlog_checksum= NONE; +insert into t1 values (3); +include/save_master_gtid.inc +connection slave; +include/sync_with_master_gtid.inc +include/stop_slave.inc +connection master; +set @@global.binlog_checksum= CRC32; +set @@global.debug_dbug= "+d,binlog_sender_oversized_fake_rotate"; +connection slave; +START SLAVE; +# Waiting for the IO thread to reject the oversized Rotate event +include/wait_for_slave_io_error.inc [errno=1595] +Last_IO_Error = 'Relay log write failure: could not queue event from master' +# Ensure the event was rejected before the replica copied it +include/assert_grep.inc [The IO thread reported an oversized Rotate event] +connection master; +set @@global.debug_dbug= @saved_dbug; +set @@global.binlog_checksum= @saved_checksum; +connection slave; +include/stop_slave_sql.inc +include/start_slave.inc +# +# Ensure replication works after the oversized Rotate events +# +connection master; +insert into t1 values (4); +include/save_master_gtid.inc +connection slave; +include/sync_with_master_gtid.inc +include/diff_tables.inc [master:t1,slave:t1] +# +# Cleanup +# +connection master; +drop table t1; +include/save_master_gtid.inc +connection slave; +include/sync_with_master_gtid.inc +include/rpl_end.inc +# End of rpl_rotate_log_event_overflow.test diff --git a/sql/slave.cc b/sql/slave.cc index d1deb78749a1f..ebce60f23e434 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -6507,7 +6507,8 @@ static int queue_event(Master_info* mi, const uchar *buf, ulong event_len) mi->checksum_alg_before_fd : mi->rli.relay_log.relay_log_checksum_alg; const uchar *save_buf= NULL; // needed for checksumming the fake Rotate event - uchar rot_buf[LOG_EVENT_HEADER_LEN + ROTATE_HEADER_LEN + FN_REFLEN]; + uchar rot_buf[LOG_EVENT_HEADER_LEN + ROTATE_HEADER_LEN + FN_REFLEN + + BINLOG_CHECKSUM_LEN]; /* The shortest event a master of this binlog version can legally send. */ ulong min_event_len= @@ -6628,7 +6629,47 @@ static int queue_event(Master_info* mi, const uchar *buf, ulong event_len) goto err; case ROTATE_EVENT: { - Rotate_log_event rev(buf, checksum_alg != BINLOG_CHECKSUM_ALG_OFF ? + /* + RSC_1 and RSC_2 below rewrite the fake Rotate (the one that opens a + connection, naming the binary log file the dump starts in; the file + switches later in the connection send more, but by then the first + format description event has left the relay log carrying the master's + algorithm) through rot_buf, and the first two conditions here are the + pair that selects them: a fake Rotate that carries a checksum the + relay log does not, or the reverse. Both cases memcpy() into rot_buf + a length taken from event_len, so event_len has to be bounded before + they run. The Rotate_log_event constructed below does not bound it. + That constructor caps only new_log_ident, the copy of the file name + it keeps for itself, so event_len remains the length the master sent. + This check is placed at the start of the case block so a rejected event + will not change replication state. + */ + bool is_fake_rotate= uint4korr(&buf[0]) == 0; + bool event_has_checksum= checksum_alg != BINLOG_CHECKSUM_ALG_OFF; + bool relay_log_has_checksum= + mi->rli.relay_log.relay_log_checksum_alg != BINLOG_CHECKSUM_ALG_OFF; + + /* The longest Rotate a master can send under its own checksum policy. */ + ulong max_rotate_len= + LOG_EVENT_HEADER_LEN + ROTATE_HEADER_LEN + FN_REFLEN + + (event_has_checksum ? BINLOG_CHECKSUM_LEN : 0); + + if (unlikely(is_fake_rotate && + event_has_checksum != relay_log_has_checksum && + event_len > max_rotate_len)) + { + error= ER_SLAVE_FATAL_ERROR; + error_msg.append(STRING_WITH_LEN("Rotate event from master names a " + "binary log file longer than the " + "maximum file name length; the event's " + "length: ")); + error_msg.append_ulonglong(event_len); + error_msg.append(STRING_WITH_LEN(", the maximum: ")); + error_msg.append_ulonglong(max_rotate_len); + goto err; + } + + Rotate_log_event rev(buf, event_has_checksum ? event_len - BINLOG_CHECKSUM_LEN : event_len, mi->rli.relay_log.description_event_for_queue); @@ -6730,8 +6771,7 @@ static int queue_event(Master_info* mi, const uchar *buf, ulong event_len) to compute checksum for its first FD event for RL the fake Rotate gets checksummed here. */ - if (uint4korr(&buf[0]) == 0 && checksum_alg == BINLOG_CHECKSUM_ALG_OFF && - mi->rli.relay_log.relay_log_checksum_alg != BINLOG_CHECKSUM_ALG_OFF) + if (is_fake_rotate && !event_has_checksum && relay_log_has_checksum) { ha_checksum rot_crc= 0; event_len += BINLOG_CHECKSUM_LEN; @@ -6754,8 +6794,7 @@ static int queue_event(Master_info* mi, const uchar *buf, ulong event_len) RSC_2: If NM \and fake Rotate \and slave does not compute checksum the fake Rotate's checksum is stripped off before relay-logging. */ - if (uint4korr(&buf[0]) == 0 && checksum_alg != BINLOG_CHECKSUM_ALG_OFF && - mi->rli.relay_log.relay_log_checksum_alg == BINLOG_CHECKSUM_ALG_OFF) + if (is_fake_rotate && event_has_checksum && !relay_log_has_checksum) { event_len -= BINLOG_CHECKSUM_LEN; memcpy(rot_buf, buf, event_len);