Skip to content

Commit

Permalink
Bug#35635853 shutdown plugin core when init_events_waits_history_long…
Browse files Browse the repository at this point in the history
… failed

Problem
=======

In some edge cases,
when an out of memory condition happens during the performance schema
initialization, the server later crashes on cleanup.

Root cause
==========

The problem is with the internal representation of an array,
in the performance schema data buffers.

Conceptually, arrays are like a std::span<T> in C++,
but the code pre dates C++20 by more than 10 years.

For various data types T,
the performance schema represents internally an array of T with:
- a xxx_array_ptr variable, of type T*
- a xxx_array_size variable

When an array is not allocated:
- xxx_array_ptr == nullptr
- xxx_array_size == 0

When an array is allocated:
- xxx_array_ptr != nullptr
- xxx_array_size != 0

The problem is that on some error path,
typically when an allocation fails,
the code can leave an inconsistent state,
such as:
- xxx_array_ptr == nullptr
- xxx_array_size != 0

which later confuses code and causes a crash.

The same inconsistent state can also exist
after cleanup, when the array size is not always reset.

Fix
===

Revised every use of:
- PFS_MALLOC_ARRAY,
- PFS_FREE_ARRAY,
to make sure that the array pointer and the array size
are coherent (either both set, or both nullptr / 0).

Change-Id: I581a385fc1dc3ee4c2ad516664de6d256649954c
  • Loading branch information
marcalff committed Aug 23, 2023
1 parent b11b0e0 commit 67d9af1
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 3 deletions.
5 changes: 5 additions & 0 deletions storage/perfschema/pfs_digest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ int init_digest(const PFS_global_param *param) {
digest_monotonic_index.m_u32.store(1);
digest_full = false;

statements_digest_stat_array = nullptr;
statements_digest_token_array = nullptr;
statements_digest_query_sample_text_array = nullptr;

if (digest_max == 0) {
return 0;
}
Expand Down Expand Up @@ -152,6 +156,7 @@ void cleanup_digest() {
statements_digest_stat_array = nullptr;
statements_digest_token_array = nullptr;
statements_digest_query_sample_text_array = nullptr;
digest_max = 0;
}

static const uchar *digest_hash_get_key(const uchar *entry, size_t *length) {
Expand Down
9 changes: 8 additions & 1 deletion storage/perfschema/pfs_events_stages.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,20 @@ int init_events_stages_history_long(uint events_stages_history_long_sizing) {
events_stages_history_long_index.m_u32.store(0);

if (events_stages_history_long_size == 0) {
events_stages_history_long_array = nullptr;
return 0;
}

events_stages_history_long_array = PFS_MALLOC_ARRAY(
&builtin_memory_stages_history_long, events_stages_history_long_size,
sizeof(PFS_events_stages), PFS_events_stages, MYF(MY_ZEROFILL));

return (events_stages_history_long_array ? 0 : 1);
if (events_stages_history_long_array == nullptr) {
events_stages_history_long_size = 0;
return 1;
}

return 0;
}

/** Cleanup table EVENTS_STAGES_HISTORY_LONG. */
Expand All @@ -85,6 +91,7 @@ void cleanup_events_stages_history_long() {
events_stages_history_long_size, sizeof(PFS_events_stages),
events_stages_history_long_array);
events_stages_history_long_array = nullptr;
events_stages_history_long_size = 0;
}

static inline void copy_events_stages(PFS_events_stages *dest,
Expand Down
5 changes: 5 additions & 0 deletions storage/perfschema/pfs_events_statements.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ int init_events_statements_history_long(
events_statements_history_long_full = false;
events_statements_history_long_index.m_u32.store(0);

events_statements_history_long_array = nullptr;
h_long_stmts_digest_token_array = nullptr;
h_long_stmts_text_array = nullptr;

if (events_statements_history_long_size == 0) {
return 0;
}
Expand Down Expand Up @@ -149,6 +153,7 @@ void cleanup_events_statements_history_long() {
events_statements_history_long_array = nullptr;
h_long_stmts_digest_token_array = nullptr;
h_long_stmts_text_array = nullptr;
events_statements_history_long_size = 0;
}

static inline void copy_events_statements(PFS_events_statements *dest,
Expand Down
9 changes: 8 additions & 1 deletion storage/perfschema/pfs_events_transactions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ int init_events_transactions_history_long(
events_transactions_history_long_index.m_u32.store(0);

if (events_transactions_history_long_size == 0) {
events_transactions_history_long_array = nullptr;
return 0;
}

Expand All @@ -83,7 +84,12 @@ int init_events_transactions_history_long(
events_transactions_history_long_size, sizeof(PFS_events_transactions),
PFS_events_transactions, MYF(MY_ZEROFILL));

return (events_transactions_history_long_array ? 0 : 1);
if (events_transactions_history_long_array == nullptr) {
events_transactions_history_long_size = 0;
return 1;
}

return 0;
}

/** Cleanup table EVENTS_TRANSACTIONS_HISTORY_LONG. */
Expand All @@ -93,6 +99,7 @@ void cleanup_events_transactions_history_long() {
sizeof(PFS_events_transactions),
events_transactions_history_long_array);
events_transactions_history_long_array = nullptr;
events_transactions_history_long_size = 0;
}

static inline void copy_events_transactions(
Expand Down
9 changes: 8 additions & 1 deletion storage/perfschema/pfs_events_waits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,20 @@ int init_events_waits_history_long(uint events_waits_history_long_sizing) {
events_waits_history_long_index.m_u32.store(0);

if (events_waits_history_long_size == 0) {
events_waits_history_long_array = nullptr;
return 0;
}

events_waits_history_long_array = PFS_MALLOC_ARRAY(
&builtin_memory_waits_history_long, events_waits_history_long_size,
sizeof(PFS_events_waits), PFS_events_waits, MYF(MY_ZEROFILL));

return (events_waits_history_long_array ? 0 : 1);
if (events_waits_history_long_array == nullptr) {
events_waits_history_long_size = 0;
return 1;
}

return 0;
}

/** Cleanup table EVENTS_WAITS_HISTORY_LONG. */
Expand All @@ -86,6 +92,7 @@ void cleanup_events_waits_history_long() {
events_waits_history_long_size, sizeof(PFS_events_waits),
events_waits_history_long_array);
events_waits_history_long_array = nullptr;
events_waits_history_long_size = 0;
}

static inline void copy_events_waits(PFS_events_waits *dest,
Expand Down
1 change: 1 addition & 0 deletions storage/perfschema/pfs_instr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ int init_instruments(const PFS_global_param *param) {
PFS_MALLOC_ARRAY(&builtin_memory_file_handle, file_handle_max,
sizeof(PFS_file *), PFS_file *, MYF(MY_ZEROFILL));
if (unlikely(file_handle_array == nullptr)) {
file_handle_max = 0;
return 1;
}
}
Expand Down
11 changes: 11 additions & 0 deletions storage/perfschema/pfs_instr_class.cc
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ int init_sync_class(uint mutex_class_sizing, uint rwlock_class_sizing,
&builtin_memory_mutex_class, mutex_class_max, sizeof(PFS_mutex_class),
PFS_mutex_class, MYF(MY_ZEROFILL));
if (unlikely(mutex_class_array == nullptr)) {
mutex_class_max = 0;
return 1;
}
}
Expand All @@ -384,6 +385,7 @@ int init_sync_class(uint mutex_class_sizing, uint rwlock_class_sizing,
&builtin_memory_rwlock_class, rwlock_class_max,
sizeof(PFS_rwlock_class), PFS_rwlock_class, MYF(MY_ZEROFILL));
if (unlikely(rwlock_class_array == nullptr)) {
rwlock_class_max = 0;
return 1;
}
}
Expand All @@ -393,6 +395,7 @@ int init_sync_class(uint mutex_class_sizing, uint rwlock_class_sizing,
cond_class_max, sizeof(PFS_cond_class),
PFS_cond_class, MYF(MY_ZEROFILL));
if (unlikely(cond_class_array == nullptr)) {
cond_class_max = 0;
return 1;
}
}
Expand Down Expand Up @@ -455,6 +458,7 @@ int init_thread_class(uint thread_class_sizing) {
&builtin_memory_thread_class, thread_class_max,
sizeof(PFS_thread_class), PFS_thread_class, MYF(MY_ZEROFILL));
if (unlikely(thread_class_array == nullptr)) {
thread_class_max = 0;
result = 1;
}
} else {
Expand Down Expand Up @@ -510,6 +514,7 @@ int init_meter_class(uint meter_class_sizing) {
&builtin_memory_meter_class, meter_class_max, sizeof(PFS_meter_class),
PFS_meter_class, MYF(MY_ZEROFILL));
if (unlikely(meter_class_array == nullptr)) {
meter_class_max = 0;
result = 1;
}
} else {
Expand Down Expand Up @@ -556,6 +561,7 @@ int init_metric_class(uint metric_class_sizing) {
&builtin_memory_metric_class, metric_class_max,
sizeof(PFS_metric_class), PFS_metric_class, MYF(MY_ZEROFILL));
if (unlikely(metric_class_array == nullptr)) {
metric_class_max = 0;
result = 1;
}
} else {
Expand Down Expand Up @@ -939,6 +945,7 @@ int init_file_class(uint file_class_sizing) {
file_class_max, sizeof(PFS_file_class),
PFS_file_class, MYF(MY_ZEROFILL));
if (unlikely(file_class_array == nullptr)) {
file_class_max = 0;
return 1;
}
} else {
Expand Down Expand Up @@ -980,6 +987,7 @@ int init_stage_class(uint stage_class_sizing) {
&builtin_memory_stage_class, stage_class_max, sizeof(PFS_stage_class),
PFS_stage_class, MYF(MY_ZEROFILL));
if (unlikely(stage_class_array == nullptr)) {
stage_class_max = 0;
return 1;
}
} else {
Expand Down Expand Up @@ -1021,6 +1029,7 @@ int init_statement_class(uint statement_class_sizing) {
&builtin_memory_statement_class, statement_class_max,
sizeof(PFS_statement_class), PFS_statement_class, MYF(MY_ZEROFILL));
if (unlikely(statement_class_array == nullptr)) {
statement_class_max = 0;
return 1;
}
} else {
Expand Down Expand Up @@ -1062,6 +1071,7 @@ int init_socket_class(uint socket_class_sizing) {
&builtin_memory_socket_class, socket_class_max,
sizeof(PFS_socket_class), PFS_socket_class, MYF(MY_ZEROFILL));
if (unlikely(socket_class_array == nullptr)) {
socket_class_max = 0;
return 1;
}
} else {
Expand Down Expand Up @@ -1103,6 +1113,7 @@ int init_memory_class(uint memory_class_sizing) {
&builtin_memory_memory_class, memory_class_max,
sizeof(PFS_memory_class), PFS_memory_class, MYF(MY_ZEROFILL));
if (unlikely(memory_class_array.load() == nullptr)) {
memory_class_max = 0;
return 1;
}
} else {
Expand Down

0 comments on commit 67d9af1

Please sign in to comment.