From d47af174be567a584f8cedc2de62e8cab772c85e Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Thu, 11 Nov 2010 12:34:46 +0100 Subject: Bug#58003 Segfault on CHECKSUM TABLE performance_schema.EVENTS_WAITS_HISTORY_LONG EXTENDED This fix is a follow up on the fix for similar issue 56761. When sanitizing data read from the events_waits_history_long table, the code needs also to sanitize the schema_name / object_name / file_name pointers, because such pointers could also hold invalid values. Checking the string length alone was required but not sufficient. This fix verifies that: - the table schema and table name used in table io events - the file name used in file io events are valid pointers before dereferencing these pointers. --- storage/perfschema/pfs_global.h | 16 ++++++++ storage/perfschema/pfs_instr.cc | 23 +++++++++-- storage/perfschema/pfs_instr.h | 1 + storage/perfschema/pfs_instr_class.cc | 70 ++++++++++++++++++++++++++------ storage/perfschema/pfs_instr_class.h | 2 + storage/perfschema/table_events_waits.cc | 21 +++++++--- 6 files changed, 112 insertions(+), 21 deletions(-) (limited to 'storage/perfschema') diff --git a/storage/perfschema/pfs_global.h b/storage/perfschema/pfs_global.h index 6050612e24c..c0c0490a380 100644 --- a/storage/perfschema/pfs_global.h +++ b/storage/perfschema/pfs_global.h @@ -79,5 +79,21 @@ inline uint randomized_index(const void *ptr, uint max_size) void pfs_print_error(const char *format, ...); +/** + Given an array defined as T ARRAY[MAX], + check that an UNSAFE pointer actually points to an element + within the array. +*/ +#define SANITIZE_ARRAY_BODY(T, ARRAY, MAX, UNSAFE) \ + intptr offset; \ + if ((&ARRAY[0] <= UNSAFE) && \ + (UNSAFE < &ARRAY[MAX])) \ + { \ + offset= ((intptr) UNSAFE - (intptr) ARRAY) % sizeof(T); \ + if (offset == 0) \ + return UNSAFE; \ + } \ + return NULL + #endif diff --git a/storage/perfschema/pfs_instr.cc b/storage/perfschema/pfs_instr.cc index 0c7b25a03de..904efc2aff4 100644 --- a/storage/perfschema/pfs_instr.cc +++ b/storage/perfschema/pfs_instr.cc @@ -758,9 +758,26 @@ PFS_thread* create_thread(PFS_thread_class *klass, const void *identity, */ PFS_thread *sanitize_thread(PFS_thread *unsafe) { - if ((&thread_array[0] <= unsafe) && - (unsafe < &thread_array[thread_max])) - return unsafe; + SANITIZE_ARRAY_BODY(PFS_thread, thread_array, thread_max, unsafe); +} + +const char *sanitize_file_name(const char *unsafe) +{ + intptr ptr= (intptr) unsafe; + intptr first= (intptr) &file_array[0]; + intptr last= (intptr) &file_array[file_max]; + + /* Check if unsafe points inside file_array[] */ + if (likely((first <= ptr) && (ptr < last))) + { + /* Check if unsafe points to PFS_file::m_filename */ + intptr offset= (ptr - first) % sizeof(PFS_file); + intptr valid_offset= my_offsetof(PFS_file, m_filename[0]); + if (likely(offset == valid_offset)) + { + return unsafe; + } + } return NULL; } diff --git a/storage/perfschema/pfs_instr.h b/storage/perfschema/pfs_instr.h index 791e2cd1f8d..2f6b729628e 100644 --- a/storage/perfschema/pfs_instr.h +++ b/storage/perfschema/pfs_instr.h @@ -227,6 +227,7 @@ struct PFS_thread }; PFS_thread *sanitize_thread(PFS_thread *unsafe); +const char *sanitize_file_name(const char *unsafe); PFS_single_stat_chain* find_per_thread_mutex_class_wait_stat(PFS_thread *thread, diff --git a/storage/perfschema/pfs_instr_class.cc b/storage/perfschema/pfs_instr_class.cc index 48547f73628..d99ca4d513c 100644 --- a/storage/perfschema/pfs_instr_class.cc +++ b/storage/perfschema/pfs_instr_class.cc @@ -543,15 +543,9 @@ PFS_mutex_class *find_mutex_class(PFS_sync_key key) FIND_CLASS_BODY(key, mutex_class_allocated_count, mutex_class_array); } -#define SANITIZE_ARRAY_BODY(ARRAY, MAX, UNSAFE) \ - if ((&ARRAY[0] <= UNSAFE) && \ - (UNSAFE < &ARRAY[MAX])) \ - return UNSAFE; \ - return NULL - PFS_mutex_class *sanitize_mutex_class(PFS_mutex_class *unsafe) { - SANITIZE_ARRAY_BODY(mutex_class_array, mutex_class_max, unsafe); + SANITIZE_ARRAY_BODY(PFS_mutex_class, mutex_class_array, mutex_class_max, unsafe); } /** @@ -566,7 +560,7 @@ PFS_rwlock_class *find_rwlock_class(PFS_sync_key key) PFS_rwlock_class *sanitize_rwlock_class(PFS_rwlock_class *unsafe) { - SANITIZE_ARRAY_BODY(rwlock_class_array, rwlock_class_max, unsafe); + SANITIZE_ARRAY_BODY(PFS_rwlock_class, rwlock_class_array, rwlock_class_max, unsafe); } /** @@ -581,7 +575,7 @@ PFS_cond_class *find_cond_class(PFS_sync_key key) PFS_cond_class *sanitize_cond_class(PFS_cond_class *unsafe) { - SANITIZE_ARRAY_BODY(cond_class_array, cond_class_max, unsafe); + SANITIZE_ARRAY_BODY(PFS_cond_class, cond_class_array, cond_class_max, unsafe); } /** @@ -636,7 +630,7 @@ PFS_thread_class *find_thread_class(PFS_sync_key key) PFS_thread_class *sanitize_thread_class(PFS_thread_class *unsafe) { - SANITIZE_ARRAY_BODY(thread_class_array, thread_class_max, unsafe); + SANITIZE_ARRAY_BODY(PFS_thread_class, thread_class_array, thread_class_max, unsafe); } /** @@ -687,7 +681,7 @@ PFS_file_class *find_file_class(PFS_file_key key) PFS_file_class *sanitize_file_class(PFS_file_class *unsafe) { - SANITIZE_ARRAY_BODY(file_class_array, file_class_max, unsafe); + SANITIZE_ARRAY_BODY(PFS_file_class, file_class_array, file_class_max, unsafe); } /** @@ -820,7 +814,59 @@ search: PFS_table_share *sanitize_table_share(PFS_table_share *unsafe) { - SANITIZE_ARRAY_BODY(table_share_array, table_share_max, unsafe); + SANITIZE_ARRAY_BODY(PFS_table_share, table_share_array, table_share_max, unsafe); +} + +const char *sanitize_table_schema_name(const char *unsafe) +{ + intptr ptr= (intptr) unsafe; + intptr first= (intptr) &table_share_array[0]; + intptr last= (intptr) &table_share_array[table_share_max]; + + PFS_table_share dummy; + + /* Check if unsafe points inside table_share_array[] */ + if (likely((first <= ptr) && (ptr < last))) + { + intptr offset= (ptr - first) % sizeof(PFS_table_share); + intptr from= my_offsetof(PFS_table_share, m_key.m_hash_key); + intptr len= sizeof(dummy.m_key.m_hash_key); + /* Check if unsafe points inside PFS_table_share::m_key::m_hash_key */ + if (likely((from <= offset) && (offset < from + len))) + { + PFS_table_share *base= (PFS_table_share*) (ptr - offset); + /* Check if unsafe really is the schema name */ + if (likely(base->m_schema_name == unsafe)) + return unsafe; + } + } + return NULL; +} + +const char *sanitize_table_object_name(const char *unsafe) +{ + intptr ptr= (intptr) unsafe; + intptr first= (intptr) &table_share_array[0]; + intptr last= (intptr) &table_share_array[table_share_max]; + + PFS_table_share dummy; + + /* Check if unsafe points inside table_share_array[] */ + if (likely((first <= ptr) && (ptr < last))) + { + intptr offset= (ptr - first) % sizeof(PFS_table_share); + intptr from= my_offsetof(PFS_table_share, m_key.m_hash_key); + intptr len= sizeof(dummy.m_key.m_hash_key); + /* Check if unsafe points inside PFS_table_share::m_key::m_hash_key */ + if (likely((from <= offset) && (offset < from + len))) + { + PFS_table_share *base= (PFS_table_share*) (ptr - offset); + /* Check if unsafe really is the table name */ + if (likely(base->m_table_name == unsafe)) + return unsafe; + } + } + return NULL; } static void reset_mutex_class_waits(void) diff --git a/storage/perfschema/pfs_instr_class.h b/storage/perfschema/pfs_instr_class.h index bd6560dbb55..107db628226 100644 --- a/storage/perfschema/pfs_instr_class.h +++ b/storage/perfschema/pfs_instr_class.h @@ -222,6 +222,8 @@ PFS_thread_class *find_thread_class(PSI_thread_key key); PFS_thread_class *sanitize_thread_class(PFS_thread_class *unsafe); PFS_file_class *find_file_class(PSI_file_key key); PFS_file_class *sanitize_file_class(PFS_file_class *unsafe); +const char *sanitize_table_schema_name(const char *unsafe); +const char *sanitize_table_object_name(const char *unsafe); PFS_table_share *find_or_create_table_share(PFS_thread *thread, const char *schema_name, diff --git a/storage/perfschema/table_events_waits.cc b/storage/perfschema/table_events_waits.cc index d55f0fc2dc8..8408cc55975 100644 --- a/storage/perfschema/table_events_waits.cc +++ b/storage/perfschema/table_events_waits.cc @@ -194,6 +194,9 @@ void table_events_waits_common::make_row(bool thread_own_wait, PFS_instr_class *safe_class; const char *base; const char *safe_source_file; + const char *safe_table_schema_name; + const char *safe_table_object_name; + const char *safe_file_name; m_row_exists= false; safe_thread= sanitize_thread(pfs_thread); @@ -252,15 +255,19 @@ void table_events_waits_common::make_row(bool thread_own_wait, m_row.m_object_type= "TABLE"; m_row.m_object_type_length= 5; m_row.m_object_schema_length= wait->m_schema_name_length; + safe_table_schema_name= sanitize_table_schema_name(wait->m_schema_name); if (unlikely((m_row.m_object_schema_length == 0) || - (m_row.m_object_schema_length > sizeof(m_row.m_object_schema)))) + (m_row.m_object_schema_length > sizeof(m_row.m_object_schema)) || + (safe_table_schema_name == NULL))) return; - memcpy(m_row.m_object_schema, wait->m_schema_name, m_row.m_object_schema_length); + memcpy(m_row.m_object_schema, safe_table_schema_name, m_row.m_object_schema_length); m_row.m_object_name_length= wait->m_object_name_length; + safe_table_object_name= sanitize_table_object_name(wait->m_object_name); if (unlikely((m_row.m_object_name_length == 0) || - (m_row.m_object_name_length > sizeof(m_row.m_object_name)))) + (m_row.m_object_name_length > sizeof(m_row.m_object_name)) || + (safe_table_object_name == NULL))) return; - memcpy(m_row.m_object_name, wait->m_object_name, m_row.m_object_name_length); + memcpy(m_row.m_object_name, safe_table_object_name, m_row.m_object_name_length); safe_class= &global_table_class; break; case WAIT_CLASS_FILE: @@ -268,10 +275,12 @@ void table_events_waits_common::make_row(bool thread_own_wait, m_row.m_object_type_length= 4; m_row.m_object_schema_length= 0; m_row.m_object_name_length= wait->m_object_name_length; + safe_file_name= sanitize_file_name(wait->m_object_name); if (unlikely((m_row.m_object_name_length == 0) || - (m_row.m_object_name_length > sizeof(m_row.m_object_name)))) + (m_row.m_object_name_length > sizeof(m_row.m_object_name)) || + (safe_file_name == NULL))) return; - memcpy(m_row.m_object_name, wait->m_object_name, m_row.m_object_name_length); + memcpy(m_row.m_object_name, safe_file_name, m_row.m_object_name_length); safe_class= sanitize_file_class((PFS_file_class*) wait->m_class); break; case NO_WAIT_CLASS: -- cgit v1.2.1