Skip to content

Commit

Permalink
MDEV-33216 stack-use-after-return in Wsrep_schema_impl::open_table()
Browse files Browse the repository at this point in the history 
Fix a case of stack-use-after-return reported by ASAN in
Wsrep_schema_impl::open_table(). This function has a stack allocated
TABLE_LIST object and return TABLE_LIST::table to the caller.
Changed the function to take a TABLE_LIST pointer as argument.

Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>
  • Loading branch information
@sciascid @sysprg
sciascid authored and sysprg committed Mar 28, 2024
1 parent c811393 commit a618ff2
Showing 1 changed file with 89 additions and 54 deletions.
143 changes: 89 additions & 54 deletions sql/wsrep_schema.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,58 +245,51 @@ static void finish_stmt(THD* thd) {
close_thread_tables(thd);
}

static int open_table(THD* thd,
const LEX_CSTRING *schema_name,
const LEX_CSTRING *table_name,
enum thr_lock_type const lock_type,
TABLE** table) {
assert(table);
*table= NULL;

static int open_table(THD *thd, const LEX_CSTRING *schema_name,
const LEX_CSTRING *table_name,
enum thr_lock_type const lock_type,
TABLE_LIST *table_list)
{
assert(table_list);
DBUG_ENTER("Wsrep_schema::open_table()");

TABLE_LIST tables;
uint flags= (MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK |
MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY |
MYSQL_OPEN_IGNORE_FLUSH |
MYSQL_LOCK_IGNORE_TIMEOUT);

tables.init_one_table(schema_name,
table_name,
NULL, lock_type);
const uint flags= (MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK |
MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY |
MYSQL_OPEN_IGNORE_FLUSH | MYSQL_LOCK_IGNORE_TIMEOUT);
table_list->init_one_table(schema_name, table_name, NULL, lock_type);
thd->lex->query_tables_own_last= 0;

if (!open_n_lock_single_table(thd, &tables, tables.lock_type, flags)) {
if (!open_n_lock_single_table(thd, table_list, table_list->lock_type, flags))
{
close_thread_tables(thd);
DBUG_RETURN(1);
}

*table= tables.table;
(*table)->use_all_columns();
table_list->table->use_all_columns();

DBUG_RETURN(0);
}


static int open_for_write(THD* thd, const char* table_name, TABLE** table) {
static int open_for_write(THD* thd, const char* table_name, TABLE_LIST* table_list)
{
LEX_CSTRING schema_str= { wsrep_schema_str.c_str(), wsrep_schema_str.length() };
LEX_CSTRING table_str= { table_name, strlen(table_name) };
if (Wsrep_schema_impl::open_table(thd, &schema_str, &table_str, TL_WRITE,
table)) {
table_list))
{
// No need to log an error if the query was bf aborted,
// thd client will get ER_LOCK_DEADLOCK in the end.
const bool interrupted= thd->killed ||
(thd->is_error() &&
(thd->get_stmt_da()->sql_errno() == ER_QUERY_INTERRUPTED));
if (!interrupted) {
if (!interrupted)
{
WSREP_ERROR("Failed to open table %s.%s for writing",
schema_str.str, table_name);
}
return 1;
}
empty_record(*table);
(*table)->use_all_columns();
restore_record(*table, s->default_values);
empty_record(table_list->table);
table_list->table->use_all_columns();
restore_record(table_list->table, s->default_values);
return 0;
}

Expand Down Expand Up @@ -438,19 +431,21 @@ static int delete_row(TABLE* table) {
return 0;
}

static int open_for_read(THD* thd, const char* table_name, TABLE** table) {

static int open_for_read(THD *thd, const char *table_name,
TABLE_LIST *table_list)
{
LEX_CSTRING schema_str= { wsrep_schema_str.c_str(), wsrep_schema_str.length() };
LEX_CSTRING table_str= { table_name, strlen(table_name) };
if (Wsrep_schema_impl::open_table(thd, &schema_str, &table_str, TL_READ,
table)) {
table_list))
{
WSREP_ERROR("Failed to open table %s.%s for reading",
schema_str.str, table_name);
return 1;
}
empty_record(*table);
(*table)->use_all_columns();
restore_record(*table, s->default_values);
empty_record(table_list->table);
table_list->table->use_all_columns();
restore_record(table_list->table, s->default_values);
return 0;
}

Expand Down Expand Up @@ -706,8 +701,10 @@ int Wsrep_schema::store_view(THD* thd, const Wsrep_view& view)
assert(view.status() == Wsrep_view::primary);
int ret= 1;
int error;
TABLE_LIST cluster_table_l;
TABLE* cluster_table= 0;
TABLE* members_table= 0;
TABLE_LIST members_table_l;
TABLE* members_table = 0;
#ifdef WSREP_SCHEMA_MEMBERS_HISTORY
TABLE* members_history_table= 0;
#endif /* WSREP_SCHEMA_MEMBERS_HISTORY */
Expand All @@ -732,11 +729,13 @@ int Wsrep_schema::store_view(THD* thd, const Wsrep_view& view)
Store cluster view info
*/
Wsrep_schema_impl::init_stmt(thd);
if (Wsrep_schema_impl::open_for_write(thd, cluster_table_str.c_str(), &cluster_table))
if (Wsrep_schema_impl::open_for_write(thd, cluster_table_str.c_str(), &cluster_table_l))
{
goto out;
}

cluster_table= cluster_table_l.table;

Wsrep_schema_impl::store(cluster_table, 0, view.state_id().id());
Wsrep_schema_impl::store(cluster_table, 1, view.view_seqno().get());
Wsrep_schema_impl::store(cluster_table, 2, view.state_id().seqno().get());
Expand All @@ -756,12 +755,14 @@ int Wsrep_schema::store_view(THD* thd, const Wsrep_view& view)
*/
Wsrep_schema_impl::init_stmt(thd);
if (Wsrep_schema_impl::open_for_write(thd, members_table_str.c_str(),
&members_table))
&members_table_l))
{
WSREP_ERROR("failed to open wsrep.members table");
goto out;
}

members_table= members_table_l.table;

for (size_t i= 0; i < view.members().size(); ++i)
{
Wsrep_schema_impl::store(members_table, 0, view.members()[i].id());
Expand Down Expand Up @@ -815,8 +816,10 @@ Wsrep_view Wsrep_schema::restore_view(THD* thd, const Wsrep_id& own_id) const {
int ret= 1;
int error;

TABLE_LIST cluster_table_l;
TABLE* cluster_table= 0;
bool end_cluster_scan= false;
TABLE_LIST members_table_l;
TABLE* members_table= 0;
bool end_members_scan= false;

Expand All @@ -842,8 +845,12 @@ Wsrep_view Wsrep_schema::restore_view(THD* thd, const Wsrep_id& own_id) const {
Read cluster info from cluster table
*/
Wsrep_schema_impl::init_stmt(thd);
if (Wsrep_schema_impl::open_for_read(thd, cluster_table_str.c_str(), &cluster_table) ||
Wsrep_schema_impl::init_for_scan(cluster_table)) {
if (Wsrep_schema_impl::open_for_read(thd, cluster_table_str.c_str(), &cluster_table_l)) {
goto out;
}
cluster_table = cluster_table_l.table;

if (Wsrep_schema_impl::init_for_scan(cluster_table)) {
goto out;
}

Expand All @@ -867,8 +874,14 @@ Wsrep_view Wsrep_schema::restore_view(THD* thd, const Wsrep_id& own_id) const {
Read members from members table
*/
Wsrep_schema_impl::init_stmt(thd);
if (Wsrep_schema_impl::open_for_read(thd, members_table_str.c_str(), &members_table) ||
Wsrep_schema_impl::init_for_scan(members_table)) {
if (Wsrep_schema_impl::open_for_read(thd, members_table_str.c_str(),
&members_table_l))
{
goto out;
}

members_table= members_table_l.table;
if (Wsrep_schema_impl::init_for_scan(members_table)) {
goto out;
}
end_members_scan= true;
Expand Down Expand Up @@ -972,14 +985,15 @@ int Wsrep_schema::append_fragment(THD* thd,
Wsrep_schema_impl::sql_safe_updates sql_safe_updates(thd);
Wsrep_schema_impl::init_stmt(thd);

TABLE* frag_table= 0;
if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table))
TABLE_LIST frag_table_l;
if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table_l))
{
trans_rollback_stmt(thd);
thd->lex->restore_backup_query_tables_list(&query_tables_list_backup);
DBUG_RETURN(1);
}

TABLE* frag_table= frag_table_l.table;
Wsrep_schema_impl::store(frag_table, 0, server_id);
Wsrep_schema_impl::store(frag_table, 1, transaction_id.get());
Wsrep_schema_impl::store(frag_table, 2, seqno.get());
Expand Down Expand Up @@ -1023,13 +1037,15 @@ int Wsrep_schema::update_fragment_meta(THD* thd,
uchar *key=NULL;
key_part_map key_map= 0;
TABLE* frag_table= 0;
TABLE_LIST frag_table_l;

Wsrep_schema_impl::init_stmt(thd);
if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table))
if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table_l))
{
thd->lex->restore_backup_query_tables_list(&query_tables_list_backup);
DBUG_RETURN(1);
}
frag_table= frag_table_l.table;

/* Find record with the given uuid, trx id, and seqno -1 */
Wsrep_schema_impl::store(frag_table, 0, ws_meta.server_id());
Expand Down Expand Up @@ -1152,12 +1168,14 @@ int Wsrep_schema::remove_fragments(THD* thd,
thd->reset_n_backup_open_tables_state(&open_tables_backup);

TABLE* frag_table= 0;
if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table))
TABLE_LIST frag_table_l;
if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table_l))
{
ret= 1;
}
else
{
frag_table= frag_table_l.table;
for (std::vector<wsrep::seqno>::const_iterator i= fragments.begin();
i != fragments.end(); ++i)
{
Expand Down Expand Up @@ -1218,19 +1236,21 @@ int Wsrep_schema::replay_transaction(THD* orig_thd,
int ret= 1;
int error;
TABLE* frag_table= 0;
TABLE_LIST frag_table_l;
uchar *key=NULL;
key_part_map key_map= 0;

for (std::vector<wsrep::seqno>::const_iterator i= fragments.begin();
i != fragments.end(); ++i)
{
Wsrep_schema_impl::init_stmt(&thd);
if ((error= Wsrep_schema_impl::open_for_read(&thd, sr_table_str.c_str(), &frag_table)))
if ((error= Wsrep_schema_impl::open_for_read(&thd, sr_table_str.c_str(), &frag_table_l)))
{
WSREP_WARN("Could not open SR table for read: %d", error);
Wsrep_schema_impl::finish_stmt(&thd);
DBUG_RETURN(1);
}
frag_table= frag_table_l.table;

Wsrep_schema_impl::store(frag_table, 0, ws_meta.server_id());
Wsrep_schema_impl::store(frag_table, 1, ws_meta.transaction_id().get());
Expand Down Expand Up @@ -1276,12 +1296,13 @@ int Wsrep_schema::replay_transaction(THD* orig_thd,

if ((error= Wsrep_schema_impl::open_for_write(&thd,
sr_table_str.c_str(),
&frag_table)))
&frag_table_l)))
{
WSREP_WARN("Could not open SR table for write: %d", error);
Wsrep_schema_impl::finish_stmt(&thd);
DBUG_RETURN(1);
}
frag_table= frag_table_l.table;

error= Wsrep_schema_impl::init_for_index_scan(frag_table,
key,
Expand Down Expand Up @@ -1323,7 +1344,9 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd)
(char*) &storage_thd);
wsrep_assign_from_threadvars(&storage_thd);
TABLE* frag_table= 0;
TABLE_LIST frag_table_l;
TABLE* cluster_table= 0;
TABLE_LIST cluster_table_l;
Wsrep_storage_service storage_service(&storage_thd);
Wsrep_schema_impl::binlog_off binlog_off(&storage_thd);
Wsrep_schema_impl::wsrep_off wsrep_off(&storage_thd);
Expand All @@ -1338,10 +1361,15 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd)

Wsrep_schema_impl::init_stmt(&storage_thd);
storage_thd.wsrep_skip_locking= FALSE;
if (Wsrep_schema_impl::open_for_read(&storage_thd,
cluster_table_str.c_str(),
&cluster_table) ||
Wsrep_schema_impl::init_for_scan(cluster_table))
if (Wsrep_schema_impl::open_for_read(&storage_thd, cluster_table_str.c_str(),
&cluster_table_l))
{
Wsrep_schema_impl::finish_stmt(&storage_thd);
DBUG_RETURN(1);
}
cluster_table= cluster_table_l.table;

if (Wsrep_schema_impl::init_for_scan(cluster_table))
{
Wsrep_schema_impl::finish_stmt(&storage_thd);
DBUG_RETURN(1);
Expand Down Expand Up @@ -1379,12 +1407,19 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd)
Open the table for reading and writing so that fragments without
valid seqno can be deleted.
*/
if (Wsrep_schema_impl::open_for_write(&storage_thd, sr_table_str.c_str(), &frag_table) ||
Wsrep_schema_impl::init_for_scan(frag_table))
if (Wsrep_schema_impl::open_for_write(&storage_thd, sr_table_str.c_str(),
&frag_table_l))
{
WSREP_ERROR("Failed to open SR table for write");
goto out;
}
frag_table= frag_table_l.table;

if (Wsrep_schema_impl::init_for_scan(frag_table))
{
WSREP_ERROR("Failed to init for index scan");
goto out;
}

while (true)
{
Expand Down

0 comments on commit a618ff2

Please sign in to comment.