From 01d183ebaa5c0df83f7bdb93491eb13d91b65bec Mon Sep 17 00:00:00 2001 From: Axel Svensson Date: Tue, 10 Oct 2023 17:00:32 +0200 Subject: [PATCH] RONDB-475: Fix crash on GRANT NDB_STORED_USER and rpl filtering - Add test case ndb_rpl_bug_rondb-475 to reproduce a crash on the replica on receiving a GRANT NDB_STORED_USER while using replication filtering. - Fix the bug by introducing a new class variable THD::enum_override_slave_filtering for temporarily disabling replication filtering when executing internal queries. It would also have been possible to temporarily alter THD::slave_thread with no need for a new class variable. The chosen solution was preferred due to simpler verification. --- .../ndb_rpl/r/ndb_rpl_bug_rondb-475.result | 23 ++++++++++++ .../ndb_rpl/t/ndb_rpl_bug_rondb-475-slave.opt | 1 + .../suite/ndb_rpl/t/ndb_rpl_bug_rondb-475.cnf | 1 + .../ndb_rpl/t/ndb_rpl_bug_rondb-475.test | 37 +++++++++++++++++++ sql/event_scheduler.cc | 1 + sql/rpl_replica.cc | 1 + sql/sql_class.cc | 1 + sql/sql_class.h | 12 ++++++ sql/sql_parse.cc | 10 ++++- storage/ndb/plugin/ndb_stored_grants.cc | 21 +++++++++++ storage/ndb/plugin/ndb_thd.cc | 6 +++ 11 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 mysql-test/suite/ndb_rpl/r/ndb_rpl_bug_rondb-475.result create mode 100644 mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475-slave.opt create mode 100644 mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475.cnf create mode 100644 mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475.test diff --git a/mysql-test/suite/ndb_rpl/r/ndb_rpl_bug_rondb-475.result b/mysql-test/suite/ndb_rpl/r/ndb_rpl_bug_rondb-475.result new file mode 100644 index 000000000000..0246555e523a --- /dev/null +++ b/mysql-test/suite/ndb_rpl/r/ndb_rpl_bug_rondb-475.result @@ -0,0 +1,23 @@ +include/master-slave.inc +Warnings: +Note #### Sending passwords in plain text without SSL/TLS is extremely insecure. +Note #### Storing MySQL user name or password information in the connection metadata repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START REPLICA; see the 'START REPLICA Syntax' in the MySQL Manual for more information. +[connection master] +CREATE TABLE t1 (a INT) ENGINE=NDB; +CREATE TABLE t2 (a INT) ENGINE=NDB; +CREATE USER 'ndb_u1'@'localhost' IDENTIFIED by 'pass1'; +GRANT NDB_STORED_USER ON *.* TO 'ndb_u1'@'localhost'; +SHOW TABLES; +Tables_in_test +t2 +SELECT user, host FROM mysql.user WHERE user LIKE 'ndb_%'; +user host +ndb_u1 localhost +SELECT grantee FROM information_schema.user_privileges +WHERE privilege_type='NDB_STORED_USER'; +grantee +'ndb_u1'@'localhost' +DROP TABLE t1; +DROP TABLE t2; +DROP USER ndb_u1@localhost; +include/rpl_end.inc diff --git a/mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475-slave.opt b/mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475-slave.opt new file mode 100644 index 000000000000..0d3485f9e257 --- /dev/null +++ b/mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475-slave.opt @@ -0,0 +1 @@ +--replicate-ignore-table=test.t1 diff --git a/mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475.cnf b/mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475.cnf new file mode 100644 index 000000000000..4a36f32ff8c8 --- /dev/null +++ b/mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475.cnf @@ -0,0 +1 @@ +!include suite/ndb_rpl/my.cnf diff --git a/mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475.test b/mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475.test new file mode 100644 index 000000000000..fa2972cf8348 --- /dev/null +++ b/mysql-test/suite/ndb_rpl/t/ndb_rpl_bug_rondb-475.test @@ -0,0 +1,37 @@ +# This is a regression test for bug: +# RONDB-475 Mysqld Segfault when reversing replication role + +# This bug causes mysqld on the replica to crash when replicating a GRANT +# NDB_STORED_USER statement while replication filtering is active. + +# For this test case, replication filtering is activated in file +# suite/ndb_rpl/t/ndb_rpl_bug_rondb-475-slave.opt + +--source include/have_ndb.inc +--source suite/ndb_rpl/ndb_master-slave.inc + +# Create tables and user on master then execute GRANT NDB_STORED_USER +connection master; +CREATE TABLE t1 (a INT) ENGINE=NDB; +CREATE TABLE t2 (a INT) ENGINE=NDB; +CREATE USER 'ndb_u1'@'localhost' IDENTIFIED by 'pass1'; +GRANT NDB_STORED_USER ON *.* TO 'ndb_u1'@'localhost'; +--source include/wait_for_ndb_committed_to_binlog.inc +sync_slave_with_master; + +# Check that t2 and ndb_u1 replicates but not t1 +connection slave; +SHOW TABLES; +SELECT user, host FROM mysql.user WHERE user LIKE 'ndb_%'; +SELECT grantee FROM information_schema.user_privileges + WHERE privilege_type='NDB_STORED_USER'; + +# Cleanup +connection master; +DROP TABLE t1; +DROP TABLE t2; +DROP USER ndb_u1@localhost; +--source include/wait_for_ndb_committed_to_binlog.inc +sync_slave_with_master; + +--source include/rpl_end.inc diff --git a/sql/event_scheduler.cc b/sql/event_scheduler.cc index a9d604fe595c..bef170bd0049 100644 --- a/sql/event_scheduler.cc +++ b/sql/event_scheduler.cc @@ -219,6 +219,7 @@ void pre_init_event_thread(THD *thd) { thd->security_context()->set_user_ptr(STRING_WITH_LEN("event_scheduler")); thd->get_protocol_classic()->get_net()->read_timeout = replica_net_timeout; thd->slave_thread = false; + thd->override_slave_filtering = THD::NO_OVERRIDE_SLAVE_FILTERING; thd->variables.option_bits |= OPTION_AUTO_IS_NULL; thd->get_protocol_classic()->set_client_capabilities(CLIENT_MULTI_RESULTS); diff --git a/sql/rpl_replica.cc b/sql/rpl_replica.cc index e62c5284dc37..d16574b1f641 100644 --- a/sql/rpl_replica.cc +++ b/sql/rpl_replica.cc @@ -4061,6 +4061,7 @@ int init_replica_thread(THD *thd, SLAVE_THD_TYPE thd_type) { : SYSTEM_THREAD_SLAVE_IO; thd->get_protocol_classic()->init_net(nullptr); thd->slave_thread = true; + thd->override_slave_filtering = THD::NO_OVERRIDE_SLAVE_FILTERING; thd->enable_slow_log = opt_log_slow_replica_statements; set_slave_thread_options(thd); diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 1444bea0495e..5a359695b09e 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -747,6 +747,7 @@ THD::THD(bool enable_plugins) lex->set_current_query_block(nullptr); m_lock_usec = 0L; slave_thread = false; + override_slave_filtering = THD::NO_OVERRIDE_SLAVE_FILTERING; memset(&variables, 0, sizeof(variables)); m_thread_id = Global_THD_manager::reserved_thread_id; file_id = 0; diff --git a/sql/sql_class.h b/sql/sql_class.h index b3ed3dfbf5b7..a648d196ad0d 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -2697,6 +2697,18 @@ class THD : public MDL_context_owner, /// instead /sven bool slave_thread; + // override_slave_filtering is used to indicate that we want to execute a + // query that normally would be suppressed in a slave thread. + enum enum_override_slave_filtering { + // Use a couple of semi-random constants to ensure initialization. They are + // chosen to encode "OSFY" and "OSFN" (meaning "override slave filtering + // yes/no") to aid debugging. + // Debug search terms: OSFY, OSFN, YFSO, NFSO + OVERRIDE_SLAVE_FILTERING = 0x4f534659, + NO_OVERRIDE_SLAVE_FILTERING = 0x4f53464e, + }; + enum enum_override_slave_filtering override_slave_filtering; + uchar password; private: diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 2bc816f74bf6..3ec44b93120b 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -3011,6 +3011,12 @@ int mysql_execute_command(THD *thd, bool first_level) { CONDITIONAL_SYNC_POINT_FOR_TIMESTAMP("before_execute_command"); + DBUG_ASSERT(thd->override_slave_filtering == + THD::NO_OVERRIDE_SLAVE_FILTERING || + (thd->override_slave_filtering == + THD::OVERRIDE_SLAVE_FILTERING && + thd->slave_thread)); + /* If there is a CREATE TABLE...START TRANSACTION command which is not yet committed or rollbacked, then we should allow only @@ -3207,12 +3213,14 @@ int mysql_execute_command(THD *thd, bool first_level) { in 5.0 there are no SET statements in the binary log) - DROP TEMPORARY TABLE IF EXISTS: we always execute it (otherwise we have stale files on slave caused by exclusion of one tmp table). + - Slave filtering is overridden, e.g. since the query is internal. */ if (!(lex->sql_command == SQLCOM_UPDATE_MULTI) && !(lex->sql_command == SQLCOM_SET_OPTION) && !(lex->sql_command == SQLCOM_DROP_TABLE && lex->drop_temporary && lex->drop_if_exists) && - all_tables_not_ok(thd, all_tables)) { + all_tables_not_ok(thd, all_tables) && + thd->override_slave_filtering == THD::NO_OVERRIDE_SLAVE_FILTERING) { /* we warn the slave SQL thread */ my_error(ER_REPLICA_IGNORED_TABLE, MYF(0)); binlog_gtid_end_transaction(thd); diff --git a/storage/ndb/plugin/ndb_stored_grants.cc b/storage/ndb/plugin/ndb_stored_grants.cc index a82b1eba63c7..f3876fd50bd3 100644 --- a/storage/ndb/plugin/ndb_stored_grants.cc +++ b/storage/ndb/plugin/ndb_stored_grants.cc @@ -289,8 +289,29 @@ void ThreadContext::deserialize_users(std::string &str) { /* returns false on success */ bool ThreadContext::exec_sql(const std::string &statement) { assert(m_closed); + + /* Many queries can be ignored if we are a slave thread. Since this query is + executed for internal purposes, we always want it to execute. We therefore + set a flag to override suppression of query execution in slave thread. */ + DBUG_ASSERT(m_thd->override_slave_filtering == + THD::NO_OVERRIDE_SLAVE_FILTERING); + if(m_thd->slave_thread) { + m_thd->override_slave_filtering = THD::OVERRIDE_SLAVE_FILTERING; + } + /* execute_query_iso() returns false on success */ m_closed = execute_query_iso(statement, nullptr); + + /* Restore the flag for overriding suppression of query execution */ + if(m_thd->slave_thread) { + DBUG_ASSERT(m_thd->override_slave_filtering == + THD::OVERRIDE_SLAVE_FILTERING); + m_thd->override_slave_filtering = THD::NO_OVERRIDE_SLAVE_FILTERING; + } else { + DBUG_ASSERT(m_thd->override_slave_filtering == + THD::NO_OVERRIDE_SLAVE_FILTERING); + } + return m_closed; } diff --git a/storage/ndb/plugin/ndb_thd.cc b/storage/ndb/plugin/ndb_thd.cc index ea3e17c9a1f8..69dbfd1c2c06 100644 --- a/storage/ndb/plugin/ndb_thd.cc +++ b/storage/ndb/plugin/ndb_thd.cc @@ -48,6 +48,12 @@ Ndb *check_ndb_in_thd(THD *thd, bool validate_ndb) { if (!thd_ndb->recycle_ndb()) return nullptr; } + DBUG_ASSERT(thd->override_slave_filtering == + THD::NO_OVERRIDE_SLAVE_FILTERING || + (thd->override_slave_filtering == + THD::OVERRIDE_SLAVE_FILTERING && + thd->slave_thread)); + return thd_ndb->ndb; }