From 0b4ae6724f2707841c50f88d8c8598f2a0cdef4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 7 Jan 2020 14:38:21 +0200 Subject: [PATCH 01/11] Fixup MDEV-21429: Correct a definition INNOBASE_ALTER_NOVALIDATE: Remove the set of operations INNOBASE_ONLINE_CREATE that was accidentally included in the definition. In the merge of 82187a1221467c7d193fca60a11a020ab4228e4a to 10.3 (in commit eda719793acd90f6157bcb825722dab674376bf4) the flags were defined correctly. This bug was caught by the test innodb_zip.index_large_prefix. --- storage/innobase/handler/handler0alter.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 64b1a806e44..eee0e43d73f 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -103,8 +103,7 @@ static const Alter_inplace_info::HA_ALTER_FLAGS INNOBASE_FOREIGN_OPERATIONS /** Operations that InnoDB cares about and can perform without validation */ static const Alter_inplace_info::HA_ALTER_FLAGS INNOBASE_ALTER_NOVALIDATE - = INNOBASE_ONLINE_CREATE - | INNOBASE_FOREIGN_OPERATIONS + = INNOBASE_FOREIGN_OPERATIONS | Alter_inplace_info::DROP_INDEX | Alter_inplace_info::DROP_UNIQUE_INDEX | Alter_inplace_info::ALTER_COLUMN_NAME @@ -115,6 +114,7 @@ static const Alter_inplace_info::HA_ALTER_FLAGS INNOBASE_ALTER_NOVALIDATE /** Operations that InnoDB cares about and can perform without rebuild */ static const Alter_inplace_info::HA_ALTER_FLAGS INNOBASE_ALTER_NOREBUILD = INNOBASE_ALTER_NOVALIDATE + | INNOBASE_ONLINE_CREATE | Alter_inplace_info::ALTER_COLUMN_EQUAL_PACK_LENGTH | Alter_inplace_info::ADD_VIRTUAL_COLUMN; From a6dd827a4d987a5c779ac0ce2658214f8678832f Mon Sep 17 00:00:00 2001 From: Sujatha Date: Wed, 18 Dec 2019 14:59:44 +0530 Subject: [PATCH 02/11] MDEV-18046: Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events Problem: ======== SHOW BINLOG EVENTS FROM causes a variety of failures, some of which are listed below. It is not a race condition issue, but there is some non-determinism in it. Analysis: ======== "show binlog events from " code considers the user given position as a valid event start position. The code starts reading data from this event start position onwards and tries to map it to a set of known events. Each event has a specific event structure and asserts have been added to ensure that read event data satisfies the event specific requirements. When a random position is supplied to "show binlog events command" the event structure specific checks will fail and they result in assert. Fix: ==== The fix is split into different parts. Each part addresses either an ASAN issue or an assert/crash. **Part1: Checksum based position validation when checksum is enabled** Using checksum validate the very first event read at the user specified position. If there is a checksum mismatch report an appropriate error for the invalid event. --- .../r/binlog_invalid_read_in_rotate.result | 18 +++++++ ...binlog_show_binlog_event_random_pos.result | 12 +++++ ...binlog_invalid_read_in_rotate.combinations | 5 ++ .../t/binlog_invalid_read_in_rotate.test | 48 +++++++++++++++++++ ..._show_binlog_event_random_pos.combinations | 5 ++ .../binlog_show_binlog_event_random_pos.test | 37 ++++++++++++++ sql/sql_repl.cc | 23 ++++++--- 7 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 mysql-test/suite/binlog/r/binlog_invalid_read_in_rotate.result create mode 100644 mysql-test/suite/binlog/r/binlog_show_binlog_event_random_pos.result create mode 100644 mysql-test/suite/binlog/t/binlog_invalid_read_in_rotate.combinations create mode 100644 mysql-test/suite/binlog/t/binlog_invalid_read_in_rotate.test create mode 100644 mysql-test/suite/binlog/t/binlog_show_binlog_event_random_pos.combinations create mode 100644 mysql-test/suite/binlog/t/binlog_show_binlog_event_random_pos.test diff --git a/mysql-test/suite/binlog/r/binlog_invalid_read_in_rotate.result b/mysql-test/suite/binlog/r/binlog_invalid_read_in_rotate.result new file mode 100644 index 00000000000..442fc951289 --- /dev/null +++ b/mysql-test/suite/binlog/r/binlog_invalid_read_in_rotate.result @@ -0,0 +1,18 @@ +RESET MASTER; +call mtr.add_suppression("Error in Log_event::read_log_event*"); +call mtr.add_suppression("Replication event checksum verification failed while reading from a log file*"); +DROP TABLE /*! IF EXISTS*/ t1; +Warnings: +Note 1051 Unknown table 'test.t1' +CREATE TABLE `t1` ( +`col_int` int, +pk integer auto_increment, +`col_char_12_key` char(12), +`col_int_key` int, +`col_char_12` char(12), +primary key (pk), +key (`col_char_12_key` ), +key (`col_int_key` )) ENGINE=InnoDB; +INSERT /*! IGNORE */ INTO t1 VALUES (183173120, NULL, 'abcd', 1, 'efghijk'), (1, NULL, 'lmno', 2, 'p'); +ALTER TABLE `t1` ENABLE KEYS; +DROP TABLE t1; diff --git a/mysql-test/suite/binlog/r/binlog_show_binlog_event_random_pos.result b/mysql-test/suite/binlog/r/binlog_show_binlog_event_random_pos.result new file mode 100644 index 00000000000..358422c5842 --- /dev/null +++ b/mysql-test/suite/binlog/r/binlog_show_binlog_event_random_pos.result @@ -0,0 +1,12 @@ +RESET MASTER; +call mtr.add_suppression("Error in Log_event::read_log_event*"); +call mtr.add_suppression("Replication event checksum verification failed while reading from a log file*"); +DROP TABLE IF EXISTS t1; +Warnings: +Note 1051 Unknown table 'test.t1' +CREATE TABLE t1 (c1 CHAR(255), c2 CHAR(255), c3 CHAR(255), c4 CHAR(255), c5 CHAR(255)); +INSERT INTO t1 VALUES (repeat('a', 255), repeat('a', 255),repeat('a', 255),repeat('a', 255),repeat('a', 255)); +INSERT INTO t1 VALUES (repeat('a', 255), repeat('a', 255),repeat('a', 255),repeat('a', 255),repeat('a', 255)); +UPDATE t1 SET c1=repeat('b',255); +INSERT INTO t1 VALUES (repeat('a', 255), repeat('a', 255),repeat('a', 255),repeat('a', 255),repeat('a', 255)); +DROP TABLE t1; diff --git a/mysql-test/suite/binlog/t/binlog_invalid_read_in_rotate.combinations b/mysql-test/suite/binlog/t/binlog_invalid_read_in_rotate.combinations new file mode 100644 index 00000000000..72076e12d48 --- /dev/null +++ b/mysql-test/suite/binlog/t/binlog_invalid_read_in_rotate.combinations @@ -0,0 +1,5 @@ +[enable_checksum] +binlog_checksum=1 + +[disable_checksum] +binlog_checksum=0 diff --git a/mysql-test/suite/binlog/t/binlog_invalid_read_in_rotate.test b/mysql-test/suite/binlog/t/binlog_invalid_read_in_rotate.test new file mode 100644 index 00000000000..36bb56c8342 --- /dev/null +++ b/mysql-test/suite/binlog/t/binlog_invalid_read_in_rotate.test @@ -0,0 +1,48 @@ +# ==== Purpose ==== +# +# Test verifies that reading binary log by using "SHOW BINLOG EVENTS" command +# from various random positions doesn't lead to crash. It should exit +# gracefully with appropriate error. +# +# ==== References ==== +# +# MDEV-18046:Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events + +--source include/have_log_bin.inc +--source include/have_innodb.inc + +RESET MASTER; + +call mtr.add_suppression("Error in Log_event::read_log_event*"); +call mtr.add_suppression("Replication event checksum verification failed while reading from a log file*"); + +DROP TABLE /*! IF EXISTS*/ t1; +CREATE TABLE `t1` ( +`col_int` int, +pk integer auto_increment, +`col_char_12_key` char(12), +`col_int_key` int, +`col_char_12` char(12), +primary key (pk), +key (`col_char_12_key` ), +key (`col_int_key` )) ENGINE=InnoDB; + +INSERT /*! IGNORE */ INTO t1 VALUES (183173120, NULL, 'abcd', 1, 'efghijk'), (1, NULL, 'lmno', 2, 'p'); + +--disable_warnings +ALTER TABLE `t1` ENABLE KEYS; +--enable_warnings + +--let $max_pos= query_get_value(SHOW MASTER STATUS,Position,1) +--let $pos= 1 +while ($pos <= $max_pos) +{ + --disable_query_log + --disable_result_log + --error 0,1220 + eval SHOW BINLOG EVENTS FROM $pos; + --inc $pos + --enable_result_log + --enable_query_log +} +DROP TABLE t1; diff --git a/mysql-test/suite/binlog/t/binlog_show_binlog_event_random_pos.combinations b/mysql-test/suite/binlog/t/binlog_show_binlog_event_random_pos.combinations new file mode 100644 index 00000000000..72076e12d48 --- /dev/null +++ b/mysql-test/suite/binlog/t/binlog_show_binlog_event_random_pos.combinations @@ -0,0 +1,5 @@ +[enable_checksum] +binlog_checksum=1 + +[disable_checksum] +binlog_checksum=0 diff --git a/mysql-test/suite/binlog/t/binlog_show_binlog_event_random_pos.test b/mysql-test/suite/binlog/t/binlog_show_binlog_event_random_pos.test new file mode 100644 index 00000000000..e6a9e1cb2c1 --- /dev/null +++ b/mysql-test/suite/binlog/t/binlog_show_binlog_event_random_pos.test @@ -0,0 +1,37 @@ +# ==== Purpose ==== +# +# Test verifies that reading binary log by using "SHOW BINLOG EVENTS" command +# from various random positions doesn't lead to crash. It should exit +# gracefully with appropriate error. +# +# ==== References ==== +# +# MDEV-18046:Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events + +--source include/have_log_bin.inc +--source include/have_innodb.inc + +RESET MASTER; +call mtr.add_suppression("Error in Log_event::read_log_event*"); +call mtr.add_suppression("Replication event checksum verification failed while reading from a log file*"); + +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (c1 CHAR(255), c2 CHAR(255), c3 CHAR(255), c4 CHAR(255), c5 CHAR(255)); +INSERT INTO t1 VALUES (repeat('a', 255), repeat('a', 255),repeat('a', 255),repeat('a', 255),repeat('a', 255)); +INSERT INTO t1 VALUES (repeat('a', 255), repeat('a', 255),repeat('a', 255),repeat('a', 255),repeat('a', 255)); +UPDATE t1 SET c1=repeat('b',255); +INSERT INTO t1 VALUES (repeat('a', 255), repeat('a', 255),repeat('a', 255),repeat('a', 255),repeat('a', 255)); +--let $max_pos= query_get_value(SHOW MASTER STATUS,Position,1) +--let $pos= 1 +while ($pos <= $max_pos) +{ + --disable_query_log + --disable_result_log + --error 0,1220 + eval SHOW BINLOG EVENTS FROM $pos LIMIT 100; + --inc $pos + --enable_result_log + --enable_query_log +} + +DROP TABLE t1; diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc index 48d2781cd46..851bff5fd24 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -3840,6 +3840,11 @@ bool mysql_show_binlog_events(THD* thd) List field_list; const char *errmsg = 0; bool ret = TRUE; + /* + Using checksum validate the correctness of event pos specified in show + binlog events command. + */ + bool verify_checksum_once= false; IO_CACHE log; File file = -1; MYSQL_BIN_LOG *binary_log= NULL; @@ -3895,6 +3900,10 @@ bool mysql_show_binlog_events(THD* thd) mi= 0; } + /* Validate user given position using checksum */ + if (lex_mi->pos == pos && !opt_master_verify_checksum) + verify_checksum_once= true; + unit->set_limit(thd->lex->current_select); limit_start= unit->offset_limit_cnt; limit_end= unit->select_limit_cnt; @@ -3977,15 +3986,16 @@ bool mysql_show_binlog_events(THD* thd) for (event_count = 0; (ev = Log_event::read_log_event(&log, (mysql_mutex_t*) 0, description_event, - opt_master_verify_checksum)); ) + (opt_master_verify_checksum || + verify_checksum_once))); ) { if (event_count >= limit_start && - ev->net_send(protocol, linfo.log_file_name, pos)) + ev->net_send(protocol, linfo.log_file_name, pos)) { - errmsg = "Net error"; - delete ev; + errmsg = "Net error"; + delete ev; mysql_mutex_unlock(log_lock); - goto err; + goto err; } if (ev->get_type_code() == FORMAT_DESCRIPTION_EVENT) @@ -4011,10 +4021,11 @@ bool mysql_show_binlog_events(THD* thd) delete ev; } + verify_checksum_once= false; pos = my_b_tell(&log); if (++event_count >= limit_end) - break; + break; } if (event_count < limit_end && log.error) From 913f405d95d757d229892a0a5e3bd1d72efb2838 Mon Sep 17 00:00:00 2001 From: Sujatha Date: Wed, 18 Dec 2019 15:00:20 +0530 Subject: [PATCH 03/11] MDEV-18046: Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events Problem: ======== SHOW BINLOG EVENTS FROM reports following assert when ASAN is enabled. Rows_log_event::Rows_log_event(const char*, uint, const Format_description_log_event*): Assertion `var_header_len >= 2' Implemented part of upstream patch. commit: mysql/mysql-server@a3a497ccf7ecacc900551fb1e47ea4078b45c351 Fix: === **Part2: Avoid reading out of buffer limits** --- sql/log_event.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index 65f29441e1a..588647fc390 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -9546,7 +9546,14 @@ Rows_log_event::Rows_log_event(const char *buf, uint event_len, which includes length bytes */ var_header_len= uint2korr(post_start); - assert(var_header_len >= 2); + /* Check length and also avoid out of buffer read */ + if (var_header_len < 2 || + event_len < static_cast(var_header_len + + (post_start - buf))) + { + m_cols.bitmap= 0; + DBUG_VOID_RETURN; + } var_header_len-= 2; /* Iterate over var-len header, extracting 'chunks' */ From 5a54e84e5d60602a97392eb284ba3b3b69a61bda Mon Sep 17 00:00:00 2001 From: Sujatha Date: Wed, 18 Dec 2019 15:00:47 +0530 Subject: [PATCH 04/11] MDEV-18046: Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events Problem: ======== SHOW BINLOG EVENTS FROM reports following crash when ASAN is enabled. SEGV on unknown address in inline_mysql_mutex_destroy in my_bitmap_free in Update_rows_log_event::~Update_rows_log_event() Fix: === **Part3: Initialize m_cols_ai.bitmap to NULL** --- sql/log_event.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index 588647fc390..c22743501aa 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -9513,7 +9513,8 @@ Rows_log_event::Rows_log_event(const char *buf, uint event_len, uint8 const common_header_len= description_event->common_header_len; Log_event_type event_type= (Log_event_type) buf[EVENT_TYPE_OFFSET]; m_type= event_type; - + m_cols_ai.bitmap= 0; + uint8 const post_header_len= description_event->post_header_len[event_type-1]; DBUG_PRINT("enter",("event_len: %u common_header_len: %d " @@ -12746,9 +12747,12 @@ void Update_rows_log_event::init(MY_BITMAP const *cols) Update_rows_log_event::~Update_rows_log_event() { - if (m_cols_ai.bitmap == m_bitbuf_ai) // no my_malloc happened - m_cols_ai.bitmap= 0; // so no my_free in my_bitmap_free - my_bitmap_free(&m_cols_ai); // To pair with my_bitmap_init(). + if (m_cols_ai.bitmap) + { + if (m_cols_ai.bitmap == m_bitbuf_ai) // no my_malloc happened + m_cols_ai.bitmap= 0; // so no my_free in my_bitmap_free + my_bitmap_free(&m_cols_ai); // To pair with my_bitmap_init(). + } } From a42ef108157e3791c57c5b0a5bce6b360b477e3d Mon Sep 17 00:00:00 2001 From: Sujatha Date: Wed, 18 Dec 2019 15:01:19 +0530 Subject: [PATCH 05/11] MDEV-18046: Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events Problem: ======== SHOW BINLOG EVENTS FROM reports following ASAN error heap-buffer-overflow within "my_strndup" in Rotate_log_event my_strndup /mysys/my_malloc.c:254 Rotate_log_event::Rotate_log_event(char const*, unsigned int, Format_description_log_event const*) Fix: === **Part4: Improved the check for event_len validation** --- sql/log_event.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index c22743501aa..aa5ae9e2eb9 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -6449,7 +6449,7 @@ Rotate_log_event::Rotate_log_event(const char* buf, uint event_len, // The caller will ensure that event_len is what we have at EVENT_LEN_OFFSET uint8 post_header_len= description_event->post_header_len[ROTATE_EVENT-1]; uint ident_offset; - if (event_len < LOG_EVENT_MINIMAL_HEADER_LEN) + if (event_len < (uint)(LOG_EVENT_MINIMAL_HEADER_LEN + post_header_len)) DBUG_VOID_RETURN; buf+= LOG_EVENT_MINIMAL_HEADER_LEN; pos= post_header_len ? uint8korr(buf + R_POS_OFFSET) : 4; From 15781283eb4ec0eaf814565b9a4edd581eec6d3b Mon Sep 17 00:00:00 2001 From: Sujatha Date: Wed, 18 Dec 2019 15:01:48 +0530 Subject: [PATCH 06/11] MDEV-18046: Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events Problem: ======== SHOW BINLOG EVENTS FROM reports following ASAN error AddressSanitizer: heap-buffer-overflow on address String::append(char const*, unsigned int) Query_log_event::pack_info(Protocol*) Fix: === **Part5: Added check to catch buffer overflow** --- sql/log_event.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index aa5ae9e2eb9..ebc14e6571d 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -3815,7 +3815,9 @@ Query_log_event::Query_log_event(const char* buf, uint event_len, uint32 max_length= uint32(event_len - ((const char*)(end + db_len + 1) - (buf - common_header_len))); - if (q_len != max_length) + if (q_len != max_length || + (event_len < uint((const char*)(end + db_len + 1) - + (buf - common_header_len)))) { q_len= 0; query= NULL; From d6fa69e4be945da174ae3445dc8203ece689f048 Mon Sep 17 00:00:00 2001 From: Sujatha Date: Wed, 18 Dec 2019 15:02:23 +0530 Subject: [PATCH 07/11] MDEV-18046: Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events Problem: ======== SHOW BINLOG EVENTS FROM reports following ASAN error AddressSanitizer: heap-buffer-overflow on address 0x60400002acb8 Load_log_event::copy_log_event(char const*, unsigned long, int, Format_description_log_event const*) Fix: === **Part6: Moved the event_len validation to the begin of copy_log_event function** --- sql/log_event.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index ebc14e6571d..65ce94f2695 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -5891,6 +5891,8 @@ int Load_log_event::copy_log_event(const char *buf, ulong event_len, { DBUG_ENTER("Load_log_event::copy_log_event"); uint data_len; + if ((int) event_len < body_offset) + DBUG_RETURN(1); char* buf_end = (char*)buf + event_len; /* this is the beginning of the post-header */ const char* data_head = buf + description_event->common_header_len; @@ -5900,9 +5902,7 @@ int Load_log_event::copy_log_event(const char *buf, ulong event_len, table_name_len = (uint)data_head[L_TBL_LEN_OFFSET]; db_len = (uint)data_head[L_DB_LEN_OFFSET]; num_fields = uint4korr(data_head + L_NUM_FIELDS_OFFSET); - - if ((int) event_len < body_offset) - DBUG_RETURN(1); + /* Sql_ex.init() on success returns the pointer to the first byte after the sql_ex structure, which is the start of field lengths array. @@ -5911,7 +5911,7 @@ int Load_log_event::copy_log_event(const char *buf, ulong event_len, buf_end, (uchar)buf[EVENT_TYPE_OFFSET] != LOAD_EVENT))) DBUG_RETURN(1); - + data_len = event_len - body_offset; if (num_fields > data_len) // simple sanity check against corruption DBUG_RETURN(1); From 2187f1c2caacd5d6dcb93789473dbaffc9613776 Mon Sep 17 00:00:00 2001 From: Sujatha Date: Wed, 18 Dec 2019 15:02:56 +0530 Subject: [PATCH 08/11] MDEV-18046: Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events Problem: ======== SHOW BINLOG EVENTS FROM reports following ASAN error "heap-buffer-overflow on address" and some times it asserts. Table_map_log_event::Table_map_log_event(const char*, uint, const Format_description_log_event*) Assertion `m_field_metadata_size <= (m_colcnt * 2)' failed. Fix: === **Part7: Avoid reading out of buffer** Converted debug assert to error handler code. --- sql/log_event.cc | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index 65ce94f2695..577b9f1a149 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -11013,7 +11013,6 @@ Table_map_log_event::Table_map_log_event(const char *buf, uint event_len, uint8 post_header_len= description_event->post_header_len[TABLE_MAP_EVENT-1]; DBUG_PRINT("info",("event_len: %u common_header_len: %d post_header_len: %d", event_len, common_header_len, post_header_len)); - /* Don't print debug messages when running valgrind since they can trigger false warnings. @@ -11022,6 +11021,9 @@ Table_map_log_event::Table_map_log_event(const char *buf, uint event_len, DBUG_DUMP("event buffer", (uchar*) buf, event_len); #endif + if (event_len < (uint)(common_header_len + post_header_len)) + DBUG_VOID_RETURN; + /* Read the post-header */ const char *post_start= buf + common_header_len; @@ -11084,15 +11086,24 @@ Table_map_log_event::Table_map_log_event(const char *buf, uint event_len, if (bytes_read < event_len) { m_field_metadata_size= net_field_length(&ptr_after_colcnt); - DBUG_ASSERT(m_field_metadata_size <= (m_colcnt * 2)); - uint num_null_bytes= (m_colcnt + 7) / 8; - m_meta_memory= (uchar *)my_multi_malloc(MYF(MY_WME), - &m_null_bits, num_null_bytes, - &m_field_metadata, m_field_metadata_size, - NULL); - memcpy(m_field_metadata, ptr_after_colcnt, m_field_metadata_size); - ptr_after_colcnt= (uchar*)ptr_after_colcnt + m_field_metadata_size; - memcpy(m_null_bits, ptr_after_colcnt, num_null_bytes); + if(m_field_metadata_size <= (m_colcnt * 2)) + { + uint num_null_bytes= (m_colcnt + 7) / 8; + m_meta_memory= (uchar *)my_multi_malloc(MYF(MY_WME), + &m_null_bits, num_null_bytes, + &m_field_metadata, m_field_metadata_size, + NULL); + memcpy(m_field_metadata, ptr_after_colcnt, m_field_metadata_size); + ptr_after_colcnt= (uchar*)ptr_after_colcnt + m_field_metadata_size; + memcpy(m_null_bits, ptr_after_colcnt, num_null_bytes); + } + else + { + m_coltype= NULL; + my_free(m_memory); + m_memory= NULL; + DBUG_VOID_RETURN; + } } } From bac33533617c6d77a2ec09250bb9b053c7216771 Mon Sep 17 00:00:00 2001 From: Sujatha Date: Wed, 18 Dec 2019 15:03:32 +0530 Subject: [PATCH 09/11] MDEV-18046: Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events Problem: ======== SHOW BINLOG EVENTS FROM reports following ASAN error AddressSanitizer: SEGV on unknown address The signal is caused by a READ memory access. User_var_log_event::User_var_log_event(char const*, unsigned int, Format_description_log_event const*) Implemented part of upstream patch. commit: mysql/mysql-server@a3a497ccf7ecacc900551fb1e47ea4078b45c351 Fix: === **Part8: added checks to avoid reading out of buffer limits** --- sql/log_event.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sql/log_event.cc b/sql/log_event.cc index 577b9f1a149..5e26ba8e1bf 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -7957,6 +7957,11 @@ User_var_log_event(const char* buf, uint event_len, we keep the flags set to UNDEF_F. */ uint bytes_read= ((val + val_len) - buf_start); + if (bytes_read > event_len) + { + error= true; + goto err; + } if ((data_written - bytes_read) > 0) { flags= (uint) *(buf + UV_VAL_IS_NULL + UV_VAL_TYPE_SIZE + From d05c511d340c34f82af686fde2f1d741e60764a5 Mon Sep 17 00:00:00 2001 From: Sujatha Date: Wed, 18 Dec 2019 15:04:06 +0530 Subject: [PATCH 10/11] MDEV-18046: Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events Problem: ======== SHOW BINLOG EVENTS FROM reports following assert when ASAN is enabled. Query_log_event::Query_log_event(const char*, uint, const Format_description_log_event*, Log_event_type): Assertion `(pos) + (6) <= (end)' failed Fix: === **Part9: Removed additional DBUG_ASSERT** --- sql/log_event.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index 5e26ba8e1bf..eb16f519da4 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -3502,7 +3502,6 @@ code_name(int code) #define CHECK_SPACE(PTR,END,CNT) \ do { \ DBUG_PRINT("info", ("Read %s", code_name(pos[-1]))); \ - DBUG_ASSERT((PTR) + (CNT) <= (END)); \ if ((PTR) + (CNT) > (END)) { \ DBUG_PRINT("info", ("query= 0")); \ query= 0; \ From cb204e11eaf4c473ce5d5a10a21de147430057dc Mon Sep 17 00:00:00 2001 From: Sujatha Date: Tue, 7 Jan 2020 18:26:53 +0530 Subject: [PATCH 11/11] MDEV-18046: Assortment of crashes, assertion failures and ASAN errors in mysql_show_binlog_events Problem: ======== SHOW BINLOG EVENTS FROM reports following ASAN error. AddressSanitizer: heap-buffer-overflow on address READ of size 1 at 0x60e00009cf71 thread T28 #0 0x55e37e034ae2 in net_field_length Fix: === **Part10: Avoid reading out of buffer** --- sql/log_event.cc | 57 ++++++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index eb16f519da4..e8881c77f2b 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -3492,6 +3492,18 @@ code_name(int code) } #endif +#define VALIDATE_BYTES_READ(CUR_POS, START, EVENT_LEN) \ + do { \ + uchar *cur_pos= (uchar *)CUR_POS; \ + uchar *start= (uchar *)START; \ + uint len= EVENT_LEN; \ + uint bytes_read= (uint)(cur_pos - start); \ + DBUG_PRINT("info", ("Bytes read: %u event_len:%u.\n",\ + bytes_read, len)); \ + if (bytes_read >= len) \ + DBUG_VOID_RETURN; \ + } while (0) + /** Macro to check that there is enough space to read from memory. @@ -11010,7 +11022,6 @@ Table_map_log_event::Table_map_log_event(const char *buf, uint event_len, m_data_size(0), m_field_metadata(0), m_field_metadata_size(0), m_null_bits(0), m_meta_memory(NULL) { - unsigned int bytes_read= 0; DBUG_ENTER("Table_map_log_event::Table_map_log_event(const char*,uint,...)"); uint8 common_header_len= description_event->common_header_len; @@ -11054,15 +11065,18 @@ Table_map_log_event::Table_map_log_event(const char *buf, uint event_len, /* Extract the length of the various parts from the buffer */ uchar const *const ptr_dblen= (uchar const*)vpart + 0; + VALIDATE_BYTES_READ(ptr_dblen, buf, event_len); m_dblen= *(uchar*) ptr_dblen; /* Length of database name + counter + terminating null */ uchar const *const ptr_tbllen= ptr_dblen + m_dblen + 2; + VALIDATE_BYTES_READ(ptr_tbllen, buf, event_len); m_tbllen= *(uchar*) ptr_tbllen; /* Length of table name + counter + terminating null */ uchar const *const ptr_colcnt= ptr_tbllen + m_tbllen + 2; uchar *ptr_after_colcnt= (uchar*) ptr_colcnt; + VALIDATE_BYTES_READ(ptr_after_colcnt, buf, event_len); m_colcnt= net_field_length(&ptr_after_colcnt); DBUG_PRINT("info",("m_dblen: %lu off: %ld m_tbllen: %lu off: %ld m_colcnt: %lu off: %ld", @@ -11085,32 +11099,27 @@ Table_map_log_event::Table_map_log_event(const char *buf, uint event_len, memcpy(m_coltype, ptr_after_colcnt, m_colcnt); ptr_after_colcnt= ptr_after_colcnt + m_colcnt; - bytes_read= (uint) (ptr_after_colcnt - (uchar *)buf); - DBUG_PRINT("info", ("Bytes read: %d.\n", bytes_read)); - if (bytes_read < event_len) + VALIDATE_BYTES_READ(ptr_after_colcnt, buf, event_len); + m_field_metadata_size= net_field_length(&ptr_after_colcnt); + if(m_field_metadata_size <= (m_colcnt * 2)) { - m_field_metadata_size= net_field_length(&ptr_after_colcnt); - if(m_field_metadata_size <= (m_colcnt * 2)) - { - uint num_null_bytes= (m_colcnt + 7) / 8; - m_meta_memory= (uchar *)my_multi_malloc(MYF(MY_WME), - &m_null_bits, num_null_bytes, - &m_field_metadata, m_field_metadata_size, - NULL); - memcpy(m_field_metadata, ptr_after_colcnt, m_field_metadata_size); - ptr_after_colcnt= (uchar*)ptr_after_colcnt + m_field_metadata_size; - memcpy(m_null_bits, ptr_after_colcnt, num_null_bytes); - } - else - { - m_coltype= NULL; - my_free(m_memory); - m_memory= NULL; - DBUG_VOID_RETURN; - } + uint num_null_bytes= (m_colcnt + 7) / 8; + m_meta_memory= (uchar *)my_multi_malloc(MYF(MY_WME), + &m_null_bits, num_null_bytes, + &m_field_metadata, m_field_metadata_size, + NULL); + memcpy(m_field_metadata, ptr_after_colcnt, m_field_metadata_size); + ptr_after_colcnt= (uchar*)ptr_after_colcnt + m_field_metadata_size; + memcpy(m_null_bits, ptr_after_colcnt, num_null_bytes); + } + else + { + m_coltype= NULL; + my_free(m_memory); + m_memory= NULL; + DBUG_VOID_RETURN; } } - DBUG_VOID_RETURN; } #endif