From 811614d412427ab7eaffd8cd5144a3d802b46f8e Mon Sep 17 00:00:00 2001 From: Hugo Wen Date: Fri, 19 Jul 2024 22:57:51 +0000 Subject: [PATCH] MDEV-34625 Fix undefined behavior of using uninitialized member variables Commit a8a75ba2d causes the MariaDB server to crash, usually with signal 11, at random code locations due to invalid pointer values during any table operation. This issue occurs when the server is built with -O3 and other customized compiler flags. For example, the command `use db1;` causes server to crash in the `check_table_access` function at line sql_parse.cc:7080 because `tables->correspondent_table` is an invalid pointer value of 0x1. The crashes are due to undefined behavior from using uninitialized variables. The problematic commit a8a75ba2d introduces code that allocates memory and sets it to 0 using thd->calloc before initializing it with a placement new operation. This process depends on setting memory to 0 to initialize member variables not explicitly set in the constructor. However, the compiler can optimize out the memset/bfill, leading to uninitialized values and unpredictable issues. Once a constructor function initializes an object, any uninitialized variables within that object are subject to undefined behavior. The state of memory before the constructor runs, whether it involves memset or was used for other purposes, is irrelevant after the placement new operation. This behavior can be demonstrated with this [test](https://gcc.godbolt.org/z/5n87z1raG) I wrote to examine the assembly code. The code in MariaDB can be abstracted to the following, though it has many layers wrapped around it and more complex logic, causing slight differences in optimization in the MariaDB build. To summarize, on x86, the memset in the following code is optimized out with both -O2 and -O3 in GCC 13, and is only preserved in the much older GCC 4.9. struct S { int i; // uninitialized in consturctor S() {}; }; int bar() { void *buf = malloc(sizeof(S)); memset(buf, 0, sizeof(S)); // optimized out S* s = new(buf) S; return s->i; } With GCC13 -O3: bar(): sub rsp, 8 mov edi, 4 call malloc mov eax, DWORD PTR [rax] add rsp, 8 ret With GCC4.9 -O3 bar(): sub rsp, 8 mov edi, 4 call malloc mov DWORD PTR [rax], 0 xor eax, eax add rsp, 8 ret Now we ensure the constructor initializes variables correctly by running the reset() function in the constructor to perform the memset/bfill(0) operation. After applying the fix, the crash is gone. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services. --- sql/sql_parse.cc | 2 +- sql/table.cc | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 77adc6db59a..ea7b8325af2 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -8375,7 +8375,7 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd, } bool has_alias_ptr= alias != nullptr; - void *memregion= thd->calloc(sizeof(TABLE_LIST)); + void *memregion= thd->alloc(sizeof(TABLE_LIST)); TABLE_LIST *ptr= new (memregion) TABLE_LIST(thd, db, fqtn, alias_str, has_alias_ptr, table, lock_type, mdl_type, table_options, diff --git a/sql/table.cc b/sql/table.cc index 1906acd14e2..373c70c5983 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -5865,6 +5865,7 @@ TABLE_LIST::TABLE_LIST(THD *thd, List *index_hints_ptr, LEX_STRING *option_ptr) { + reset(); db= db_str; is_fqtn= fqtn; alias= alias_str;