From ba835f89ba4a07344322765c27c424a2ea96f6ee Mon Sep 17 00:00:00 2001 From: Sven Sandberg Date: Mon, 29 Dec 2008 17:04:10 +0100 Subject: [PATCH] BUG#40482: server/mysqlbinlog crashes when reading invalid Incident_log_event Problem: When an Incident_log_event contains a bad incident number on disk, the server crashes with an assertion. Fix: Don't validate input with assertions. Use errors. mysql-test/include/cleanup_fake_relay_log.inc: Added auxiliary file to restore things that setup_fake_relay_log.inc did. mysql-test/include/setup_fake_relay_log.inc: Added auxiliary file to setup replication from an existing relay log. mysql-test/std_data/bug40482-bin.000001: Binlog file for rpl.rpl_binlog_corruption mysql-test/suite/rpl/t/rpl_binlog_corruption.test: New test file. sql/log_event.cc: Check that the incident number is correct at the time the event is constructed. Do not assert it at the time it is printed. sql/log_event.h: Incident_log_event::is_valid() should verify that the incident number is valid. sql/rpl_constants.h: Incident numbers should be hard-coded, since they may appear in files. --- mysql-test/include/cleanup_fake_relay_log.inc | 16 ++++ mysql-test/include/setup_fake_relay_log.inc | 78 ++++++++++++++++++ mysql-test/std_data/bug40482-bin.000001 | Bin 0 -> 172 bytes .../suite/rpl/r/rpl_binlog_corruption.result | 8 ++ .../rpl/t/rpl_binlog_corruption-master.opt | 1 + .../suite/rpl/t/rpl_binlog_corruption.test | 42 ++++++++++ sql/log_event.cc | 15 +++- sql/log_event.h | 5 +- sql/rpl_constants.h | 4 +- 9 files changed, 162 insertions(+), 7 deletions(-) create mode 100644 mysql-test/include/cleanup_fake_relay_log.inc create mode 100644 mysql-test/include/setup_fake_relay_log.inc create mode 100644 mysql-test/std_data/bug40482-bin.000001 create mode 100644 mysql-test/suite/rpl/r/rpl_binlog_corruption.result create mode 100644 mysql-test/suite/rpl/t/rpl_binlog_corruption-master.opt create mode 100644 mysql-test/suite/rpl/t/rpl_binlog_corruption.test diff --git a/mysql-test/include/cleanup_fake_relay_log.inc b/mysql-test/include/cleanup_fake_relay_log.inc new file mode 100644 index 00000000000..43aa46cb657 --- /dev/null +++ b/mysql-test/include/cleanup_fake_relay_log.inc @@ -0,0 +1,16 @@ +# ==== Purpose ==== +# +# Clean up files create by setup_fake_relay_log.inc. +# +# ==== Usage ==== +# +# See setup_fake_relay_log.inc + +--echo Cleaning up after setup_fake_relay_log.inc + +# Remove files. +remove_file $_fake_relay_log; +remove_file $_fake_relay_index; +--disable_query_log +eval SET @@global.relay_log_purge= $_fake_relay_log_purge; +--enable_query_log diff --git a/mysql-test/include/setup_fake_relay_log.inc b/mysql-test/include/setup_fake_relay_log.inc new file mode 100644 index 00000000000..8113ed6dc10 --- /dev/null +++ b/mysql-test/include/setup_fake_relay_log.inc @@ -0,0 +1,78 @@ +# ==== Purpose ==== +# +# Setup replication from an existing relay log in the current +# connection. +# +# ==== Usage ==== +# +# Make sure the slave is not running and issue: +# +# let $fake_relay_log= /path/to/fake-relay-log-file.000001 +# source include/setup_fake_relay_log.inc; +# START SLAVE SQL_THREAD; # setup_fake_relay_log doesn't start slave +# ... +# source include/cleanup_fake_relay_log.inc +# +# You must run the server with --relay-log=FILE. You probably want to +# run with --replicate-same-server-id too. +# +# ==== Implementation ==== +# +# First makes a sanity check, ensuring that the slave threads are not +# running. Then copies the $fake_relay_log to RELAY_BIN-fake.000001, +# where RELAY_BIN is the basename of the relay log, and updates +# RELAY_BIN.index. Finally issues CHANGE MASTER so that it uses the +# given files. +# +# ==== Side effects ==== +# +# Creates a binlog file and a binlog index file, and sets +# @@global.relay_log_purge=1. All this is restored when you call +# cleanup_fake_relay_log.inc. +# +# Enables the query log. + + +--disable_query_log + +# Print message. +let $_fake_relay_log_printable= `SELECT REPLACE('$fake_relay_log', '$MYSQL_TEST_DIR', 'MYSQL_TEST_DIR')`; +--echo Setting up fake replication from $_fake_relay_log_printable + +# Sanity check. +let $_sql_running= query_get_value(SHOW SLAVE STATUS, Slave_SQL_Running, 1); +let $_io_running= query_get_value(SHOW SLAVE STATUS, Slave_IO_Running, 1); +if (`SELECT "$_sql_running" = "Yes" OR "$_io_running" = "Yes"`) { + --echo Error: Slave was running when test case sourced + --echo include/setup_fake_replication.inc + --echo Slave_IO_Running = $_io_running; Slave_SQL_Running = $_sql_running + --echo Printing some debug info: + SHOW SLAVE STATUS; + SHOW MASTER STATUS; + SHOW BINLOG EVENTS; + SHOW PROCESSLIST; +} + +# Read server variables. +let $MYSQLD_DATADIR= `SELECT @@datadir`; +let $_fake_filename= query_get_value(SHOW VARIABLES LIKE 'relay_log', Value, 1); +if (`SELECT '$_fake_filename' = ''`) { + --echo Badly written test case: relay_log variable is empty. Please use the + --echo server option --relay-log=FILE. +} +let $_fake_relay_log= $MYSQLD_DATADIR/$_fake_filename-fake.000001; +let $_fake_relay_index= $MYSQLD_DATADIR/$_fake_filename.index; +# Need to restore relay_log_purge in cleanup_fake_relay_log.inc, since +# CHANGE MASTER modifies it (see the manual for CHANGE MASTER). +let $_fake_relay_log_purge= `SELECT @@global.relay_log_purge`; + +# Create relay log file. +copy_file $fake_relay_log $_fake_relay_log; + +# Create relay log index. +eval SELECT '$_fake_relay_log' INTO OUTFILE '$_fake_relay_index'; + +# Setup replication from existing relay log. +eval CHANGE MASTER TO MASTER_HOST='dummy.localdomain', RELAY_LOG_FILE='$_fake_relay_log', RELAY_LOG_POS=4; + +--enable_query_log diff --git a/mysql-test/std_data/bug40482-bin.000001 b/mysql-test/std_data/bug40482-bin.000001 new file mode 100644 index 0000000000000000000000000000000000000000..d001bf664c7096f862e741ef6d93f58b6bdd6796 GIT binary patch literal 172 zcmeyDl$mF&%J0d~$iTpm2E(uint2korr(buf + common_header_len)); + int incident_number= uint2korr(buf + common_header_len); + if (incident_number >= INCIDENT_COUNT || + incident_number <= INCIDENT_NONE) + { + // If the incident is not recognized, this binlog event is + // invalid. If we set incident_number to INCIDENT_NONE, the + // invalidity will be detected by is_valid(). + incident_number= INCIDENT_NONE; + DBUG_VOID_RETURN; + } + m_incident= static_cast(incident_number); char const *ptr= buf + common_header_len + post_header_len; char const *const str_end= buf + event_len; uint8 len= 0; // Assignment to keep compiler happy @@ -9085,9 +9095,6 @@ Incident_log_event::description() const DBUG_PRINT("info", ("m_incident: %d", m_incident)); - DBUG_ASSERT(0 <= m_incident); - DBUG_ASSERT((size_t) m_incident <= sizeof(description)/sizeof(*description)); - return description[m_incident]; } diff --git a/sql/log_event.h b/sql/log_event.h index 3c109b798d3..db14341b51d 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -3870,7 +3870,10 @@ public: virtual Log_event_type get_type_code() { return INCIDENT_EVENT; } - virtual bool is_valid() const { return 1; } + virtual bool is_valid() const + { + return m_incident > INCIDENT_NONE && m_incident < INCIDENT_COUNT; + } virtual int get_data_size() { return INCIDENT_HEADER_LEN + 1 + m_message.length; } diff --git a/sql/rpl_constants.h b/sql/rpl_constants.h index 426e80a328d..32fb4b8a7f2 100644 --- a/sql/rpl_constants.h +++ b/sql/rpl_constants.h @@ -6,10 +6,10 @@ */ enum Incident { /** No incident */ - INCIDENT_NONE, + INCIDENT_NONE = 0, /** There are possibly lost events in the replication stream */ - INCIDENT_LOST_EVENTS, + INCIDENT_LOST_EVENTS = 1, /** Shall be last event of the enumeration */ INCIDENT_COUNT