revision-id: 43f27ae4778b3e7be86897e49c182dc05080fd88 (mariadb-10.1.43-66-g43f27ae4778) parent(s): 982294ac1680938ac9223fb64a64e21f0cbc322a author: Sujatha committer: Sujatha timestamp: 2020-01-23 11:57:46 +0530 message: MDEV-21490: binlog tests fail with valgrind: Conditional jump or move depends on uninitialised value in sql_ex_info::init Problem: ======= P1) Conditional jump or move depends on uninitialised value(s) sql_ex_info::init(char const*, char const*, bool) (log_event.cc:3083) code: All the following variables are not initialized. ---- return ((cached_new_format != -1) ? cached_new_format : (cached_new_format=(field_term_len > 1 || enclosed_len > 1 || line_term_len > 1 || line_start_len > 1 || escaped_len > 1))); P2) Conditional jump or move depends on uninitialised value(s) Rows_log_event::Rows_log_event(char const*, unsigned int, Format_description_log_event const*) (log_event.cc:9571) Code: Uninitialized values is reported for 'var_header_len' variable. ---- if (var_header_len < 2 || event_len < static_cast<unsigned int>(var_header_len + (post_start - buf))) P3) Conditional jump or move depends on uninitialised value(s) Table_map_log_event::pack_info(Protocol*) (log_event.cc:11553) code:'m_table_id' is uninitialized. ---- void Table_map_log_event::pack_info(Protocol *protocol) ... size_t bytes= my_snprintf(buf, sizeof(buf), "table_id: %lu (%s.%s)", m_table_id, m_dbnam, m_tblnam); Fix: === P1 - Fix) Initialize cached_new_format,field_term_len, enclosed_len, line_term_len, line_start_len, escaped_len members in default constructor. P2 - Fix) "var_header_len" is initialized by reading the event buffer. In case of an invalid event the buffer will contain invalid data. Hence added a check to validate the event data. If event_len is smaller than valid header length return immediately. P3 - Fix) 'm_table_id' within Table_map_log_event is initialized by reading data from the event buffer. Use 'VALIDATE_BYTES_READ' macro to validate the current state of the buffer. If it is invalid return immediately. --- sql/log_event.cc | 7 +++++++ sql/log_event.h | 9 ++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index e8881c77f2b..9924b9a0493 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -9535,6 +9535,12 @@ Rows_log_event::Rows_log_event(const char *buf, uint event_len, uint8 const post_header_len= description_event->post_header_len[event_type-1]; + if (event_len < (uint)(common_header_len + post_header_len)) + { + m_cols.bitmap= 0; + DBUG_VOID_RETURN; + } + DBUG_PRINT("enter",("event_len: %u common_header_len: %d " "post_header_len: %d", event_len, common_header_len, @@ -11043,6 +11049,7 @@ Table_map_log_event::Table_map_log_event(const char *buf, uint event_len, const char *post_start= buf + common_header_len; post_start+= TM_MAPID_OFFSET; + VALIDATE_BYTES_READ(post_start, buf, event_len); if (post_header_len == 6) { /* Master is of an intermediate source tree before 5.1.4. Id is 4 bytes */ diff --git a/sql/log_event.h b/sql/log_event.h index 2c8dc3d7353..04fce65faeb 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -2057,7 +2057,14 @@ class Query_log_event: public Log_event ****************************************************************************/ struct sql_ex_info { - sql_ex_info() {} /* Remove gcc warning */ + sql_ex_info(): + cached_new_format(-1), + field_term_len(0), + enclosed_len(0), + line_term_len(0), + line_start_len(0), + escaped_len(0) + {} /* Remove gcc warning */ const char* field_term; const char* enclosed; const char* line_term;