From 760b57814f010d66f53ee5e7f885035180ff609b Mon Sep 17 00:00:00 2001 From: Daniel Bartholomew Date: Wed, 27 May 2026 14:26:47 -0400 Subject: [PATCH 01/23] bump the VERSION --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index d0ca98664ba..1f9c29e61cf 100644 --- a/VERSION +++ b/VERSION @@ -1,4 +1,4 @@ MYSQL_VERSION_MAJOR=10 MYSQL_VERSION_MINOR=6 -MYSQL_VERSION_PATCH=27 +MYSQL_VERSION_PATCH=28 SERVER_MATURITY=stable From aefe48e6ef955947d139ff04488d6890034ed034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 28 May 2026 09:23:51 +0300 Subject: [PATCH 02/23] MDEV-39344: trx_disconnect_prepared() uses wrong mutex trx_t::disconnect_prepared(): Replaces trx_disconnect_prepared(). Protect is_recovered, mysql_thd with mutex, to be consistent with protecting fetch_data_into_cache_low(). This fixes up commit c0817dac99c28698dfc2b548d89acf1fb41dc32e (MDEV-29575). Suggested by: Sergey Vojtovich --- storage/innobase/handler/ha_innodb.cc | 2 +- storage/innobase/include/trx0sys.h | 3 --- storage/innobase/include/trx0trx.h | 8 +++----- storage/innobase/trx/trx0trx.cc | 26 +++++++++++--------------- 4 files changed, 15 insertions(+), 24 deletions(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index d1bcde83b80..02649112b8c 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -2953,7 +2953,7 @@ static int innobase_close_connection(handlerton *, THD *thd) noexcept case TRX_STATE_PREPARED: if (trx->has_logged_persistent()) { - trx_disconnect_prepared(trx); + trx->disconnect_prepared(); return 0; } /* fall through */ diff --git a/storage/innobase/include/trx0sys.h b/storage/innobase/include/trx0sys.h index c2dbcc604d1..6e514331d7b 100644 --- a/storage/innobase/include/trx0sys.h +++ b/storage/innobase/include/trx0sys.h @@ -825,9 +825,6 @@ public: mysql_mutex_unlock(&mutex); } - void freeze() const { mysql_mutex_lock(&mutex); } - void unfreeze() const { mysql_mutex_unlock(&mutex); } - private: alignas(CPU_LEVEL1_DCACHE_LINESIZE) mutable mysql_mutex_t mutex; alignas(CPU_LEVEL1_DCACHE_LINESIZE) ilist trx_list; diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h index 61ecae6efc8..784ae192747 100644 --- a/storage/innobase/include/trx0trx.h +++ b/storage/innobase/include/trx0trx.h @@ -73,10 +73,6 @@ trx_t *trx_create(); /** At shutdown, frees a transaction object. */ void trx_free_at_shutdown(trx_t *trx); -/** Disconnect a prepared transaction from MySQL. -@param[in,out] trx transaction */ -void trx_disconnect_prepared(trx_t *trx); - /** Initialize (resurrect) transactions at startup. */ dberr_t trx_lists_init_at_db_start(); @@ -730,7 +726,7 @@ public: This field is accessed by the thread that owns the transaction, without holding any mutex. There is only one foreign-thread access in trx_print_low() - and a possible race condition with trx_disconnect_prepared(). */ + and a possible race condition with disconnect_prepared(). */ bool is_recovered; const char* op_info; /*!< English text describing the current operation, or an empty @@ -968,6 +964,8 @@ private: public: /** Commit the transaction. */ void commit() noexcept; + /** Disconnect a prepared transaction */ + void disconnect_prepared() noexcept; /** Try to drop a persistent table. @param table persistent table diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index 2d2492600b5..abfa22810ed 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -532,23 +532,19 @@ TRANSACTIONAL_TARGET void trx_free_at_shutdown(trx_t *trx) } -/** - Disconnect a prepared transaction from MySQL - @param[in,out] trx transaction -*/ -void trx_disconnect_prepared(trx_t *trx) +void trx_t::disconnect_prepared() noexcept { - ut_ad(trx_state_eq(trx, TRX_STATE_PREPARED)); - ut_ad(trx->mysql_thd); - ut_ad(!trx->mysql_log_file_name); - trx->read_view.close(); - trx_sys.trx_list.freeze(); - trx->is_recovered= true; - trx->mysql_thd= NULL; - trx_sys.trx_list.unfreeze(); + ut_ad(trx_state_eq(this, TRX_STATE_PREPARED)); + ut_ad(mysql_thd); + ut_ad(!mysql_log_file_name); + read_view.close(); + mutex_lock(); + is_recovered= true; + mysql_thd= nullptr; + mutex_unlock(); /* todo/fixme: suggest to do it at innodb prepare */ - trx->will_lock= false; - trx_sys.rw_trx_hash.put_pins(trx); + will_lock= false; + trx_sys.rw_trx_hash.put_pins(this); } MY_ATTRIBUTE((nonnull, warn_unused_result)) From 34fd0fedc5f3b612958b4a88bcc083a4bfd2ebd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 28 May 2026 09:24:06 +0300 Subject: [PATCH 03/23] MDEV-39344: fetch_data_into_cache() clears "is_truncated" There is a memory limit implemented for the INFORMATION_SCHEMA tables innodb_trx, innodb_locks, innodb_lock_waits. The predicate trx_i_s_cache_is_truncated() would never hold in trx_i_s_common_fill_table() due to a logic bug that had been introduced in a refactoring. fetch_data_into_cache(): Shrink the critical section of lock_sys.latch and do not unconditionally reset cache->is_truncated when leaving the function. This fixes a correctness regression that had been introduced back in 2007 by commit 5cbf4e3840491c2d80928641bc0fbb8135e9c201 for the InnoDB Plugin for MySQL 5.1, which later became the built-in InnoDB in MySQL 5.5 and MariaDB Server 5.5. Reviewed by: Sergey Vojtovich --- storage/innobase/trx/trx0i_s.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/storage/innobase/trx/trx0i_s.cc b/storage/innobase/trx/trx0i_s.cc index c1784b94bb1..fdea942e797 100644 --- a/storage/innobase/trx/trx0i_s.cc +++ b/storage/innobase/trx/trx0i_s.cc @@ -1045,8 +1045,11 @@ static void fetch_data_into_cache_low(trx_i_s_cache_t *cache, const trx_t *trx) static void fetch_data_into_cache(trx_i_s_cache_t *cache) { - LockMutexGuard g{SRW_LOCK_CALL}; + /* these are protected by cache->rw_lock.wr_lock() */ trx_i_s_cache_clear(cache); + /* this flag may be set by fetch_data_into_cache_low() below */ + cache->is_truncated= false; + LockMutexGuard g{SRW_LOCK_CALL}; /* Capture the state of transactions */ trx_sys.trx_list.for_each([cache](trx_t &trx) { @@ -1059,7 +1062,6 @@ static void fetch_data_into_cache(trx_i_s_cache_t *cache) trx.mutex_unlock(); } }); - cache->is_truncated= false; } From 160695101c171378b5a35972cc042be076e23776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 28 May 2026 09:24:11 +0300 Subject: [PATCH 04/23] MDEV-39344: Fix a TOCTOU race trx_i_s_possibly_fetch_data_into_cache(): Fetch into the cache and return whether the cache was truncated. Previously, we checked this flag without holding any latch. trx_i_s_cache_start_write(), trx_i_s_cache_end_write(), trx_i_s_cache_is_truncated(): Remove. These are now part of trx_i_s_possibly_fetch_data_into_cache(). trx_i_s_cache: Remove. The singleton cache object can be a static object with no pointer indirection. --- storage/innobase/handler/i_s.cc | 36 +++--- storage/innobase/include/trx0i_s.h | 75 ++----------- storage/innobase/srv/srv0srv.cc | 4 +- storage/innobase/trx/trx0i_s.cc | 172 ++++++++--------------------- 4 files changed, 72 insertions(+), 215 deletions(-) diff --git a/storage/innobase/handler/i_s.cc b/storage/innobase/handler/i_s.cc index 36105ad1c2b..2882bbee008 100644 --- a/storage/innobase/handler/i_s.cc +++ b/storage/innobase/handler/i_s.cc @@ -187,11 +187,7 @@ static bool trx_i_s_common_fill_table(THD *thd, TABLE_LIST *tables) RETURN_IF_INNODB_NOT_STARTED(tables->schema_table_name.str); /* update the cache */ - trx_i_s_cache_start_write(trx_i_s_cache); - trx_i_s_possibly_fetch_data_into_cache(trx_i_s_cache); - trx_i_s_cache_end_write(trx_i_s_cache); - - if (trx_i_s_cache_is_truncated(trx_i_s_cache)) + if (trx_i_s_possibly_fetch_data_into_cache()) sql_print_warning("InnoDB: Data in %.*s truncated due to memory limit" " of %u bytes", int(tables->schema_table_name.length), @@ -380,22 +376,20 @@ static int fill_innodb_trx_from_cache(THD *thd, TABLE_LIST *tables, Item*) struct cache { - cache() { trx_i_s_cache_start_read(trx_i_s_cache); } - ~cache() { trx_i_s_cache_end_read(trx_i_s_cache); } + cache() { trx_i_s_cache_start_read(); } + ~cache() { trx_i_s_cache_end_read(); } } c; Field** fields = tables->table->field; - rows_num = trx_i_s_cache_get_rows_used(trx_i_s_cache, - I_S_INNODB_TRX); + rows_num = trx_i_s_cache_get_rows_used(I_S_INNODB_TRX); for (i = 0; i < rows_num; i++) { i_s_trx_row_t* row; row = (i_s_trx_row_t*) - trx_i_s_cache_get_nth_row( - trx_i_s_cache, I_S_INNODB_TRX, i); + trx_i_s_cache_get_nth_row(I_S_INNODB_TRX, i); /* trx_id */ OK(fields[IDX_TRX_ID]->store(row->trx_id, true)); @@ -669,14 +663,13 @@ fill_innodb_locks_from_cache( struct cache { - cache() { trx_i_s_cache_start_read(trx_i_s_cache); } - ~cache() { trx_i_s_cache_end_read(trx_i_s_cache); } + cache() { trx_i_s_cache_start_read(); } + ~cache() { trx_i_s_cache_end_read(); } } c; Field** fields = tables->table->field; - rows_num = trx_i_s_cache_get_rows_used(trx_i_s_cache, - I_S_INNODB_LOCKS); + rows_num = trx_i_s_cache_get_rows_used(I_S_INNODB_LOCKS); for (i = 0; i < rows_num; i++) { @@ -685,8 +678,7 @@ fill_innodb_locks_from_cache( const char* bufend; row = (i_s_locks_row_t*) - trx_i_s_cache_get_nth_row( - trx_i_s_cache, I_S_INNODB_LOCKS, i); + trx_i_s_cache_get_nth_row(I_S_INNODB_LOCKS, i); /* lock_id */ trx_i_s_create_lock_id(row, lock_id, sizeof(lock_id)); @@ -855,22 +847,20 @@ fill_innodb_lock_waits_from_cache( struct cache { - cache() { trx_i_s_cache_start_read(trx_i_s_cache); } - ~cache() { trx_i_s_cache_end_read(trx_i_s_cache); } + cache() { trx_i_s_cache_start_read(); } + ~cache() { trx_i_s_cache_end_read(); } } c; Field** fields = tables->table->field; - rows_num = trx_i_s_cache_get_rows_used(trx_i_s_cache, - I_S_INNODB_LOCK_WAITS); + rows_num = trx_i_s_cache_get_rows_used(I_S_INNODB_LOCK_WAITS); for (i = 0; i < rows_num; i++) { i_s_lock_waits_row_t* row; row = (i_s_lock_waits_row_t*) - trx_i_s_cache_get_nth_row( - trx_i_s_cache, I_S_INNODB_LOCK_WAITS, i); + trx_i_s_cache_get_nth_row(I_S_INNODB_LOCK_WAITS, i); /* requesting_trx_id */ OK(fields[IDX_REQUESTING_TRX_ID]->store( diff --git a/storage/innobase/include/trx0i_s.h b/storage/innobase/include/trx0i_s.h index f549745baa2..7fc37ec40bd 100644 --- a/storage/innobase/include/trx0i_s.h +++ b/storage/innobase/include/trx0i_s.h @@ -146,9 +146,6 @@ struct i_s_lock_waits_row_t { const i_s_locks_row_t* blocking_lock_row; /*!< blocking lock */ }; -/** Cache of INFORMATION_SCHEMA table data */ -struct trx_i_s_cache_t; - /** Auxiliary enum used by functions that need to select one of the INFORMATION_SCHEMA tables */ enum i_s_table { @@ -157,90 +154,42 @@ enum i_s_table { I_S_INNODB_LOCK_WAITS /*!< INFORMATION_SCHEMA.innodb_lock_waits */ }; -/** This is the intermediate buffer where data needed to fill the -INFORMATION SCHEMA tables is fetched and later retrieved by the C++ -code in handler/i_s.cc. */ -extern trx_i_s_cache_t* trx_i_s_cache; - /*******************************************************************//** Initialize INFORMATION SCHEMA trx related cache. */ -void -trx_i_s_cache_init( -/*===============*/ - trx_i_s_cache_t* cache); /*!< out: cache to init */ +void trx_i_s_cache_init(); /*******************************************************************//** Free the INFORMATION SCHEMA trx related cache. */ -void -trx_i_s_cache_free( -/*===============*/ - trx_i_s_cache_t* cache); /*!< in/out: cache to free */ +void trx_i_s_cache_free(); /*******************************************************************//** Issue a shared/read lock on the tables cache. */ -void -trx_i_s_cache_start_read( -/*=====================*/ - trx_i_s_cache_t* cache); /*!< in: cache */ +void trx_i_s_cache_start_read() noexcept; /*******************************************************************//** Release a shared/read lock on the tables cache. */ -void -trx_i_s_cache_end_read( -/*===================*/ - trx_i_s_cache_t* cache); /*!< in: cache */ - -/*******************************************************************//** -Issue an exclusive/write lock on the tables cache. */ -void -trx_i_s_cache_start_write( -/*======================*/ - trx_i_s_cache_t* cache); /*!< in: cache */ - -/*******************************************************************//** -Release an exclusive/write lock on the tables cache. */ -void -trx_i_s_cache_end_write( -/*====================*/ - trx_i_s_cache_t* cache); /*!< in: cache */ - +void trx_i_s_cache_end_read() noexcept; /*******************************************************************//** Retrieves the number of used rows in the cache for a given INFORMATION SCHEMA table. +@param table table to be read @return number of rows */ -ulint -trx_i_s_cache_get_rows_used( -/*========================*/ - trx_i_s_cache_t* cache, /*!< in: cache */ - enum i_s_table table); /*!< in: which table */ +ulint trx_i_s_cache_get_rows_used(i_s_table table); /*******************************************************************//** Retrieves the nth row in the cache for a given INFORMATION SCHEMA table. +@param table table to read +@param n row number @return row */ -void* -trx_i_s_cache_get_nth_row( -/*======================*/ - trx_i_s_cache_t* cache, /*!< in: cache */ - enum i_s_table table, /*!< in: which table */ - ulint n); /*!< in: row number */ +void *trx_i_s_cache_get_nth_row(i_s_table table, ulint n); /*******************************************************************//** Update the transactions cache if it has not been read for some time. -@return 0 - fetched, 1 - not */ -int -trx_i_s_possibly_fetch_data_into_cache( -/*===================================*/ - trx_i_s_cache_t* cache); /*!< in/out: cache */ +@retval false when fetched or cached +@retval true if fetched but the cache was truncated */ +bool trx_i_s_possibly_fetch_data_into_cache(); -/*******************************************************************//** -Returns true, if the data in the cache is truncated due to the memory -limit posed by TRX_I_S_MEM_LIMIT. -@return TRUE if truncated */ -bool -trx_i_s_cache_is_truncated( -/*=======================*/ - trx_i_s_cache_t* cache); /*!< in: cache */ /** The maximum length of a resulting lock_id_size in trx_i_s_create_lock_id(), not including the terminating NUL. ":%lu:%lu:%lu" -> 63 chars */ diff --git a/storage/innobase/srv/srv0srv.cc b/storage/innobase/srv/srv0srv.cc index def31335d34..a674af4f2c9 100644 --- a/storage/innobase/srv/srv0srv.cc +++ b/storage/innobase/srv/srv0srv.cc @@ -621,7 +621,7 @@ static void srv_init() &page_zip_stat_per_index_mutex, nullptr); /* Initialize some INFORMATION SCHEMA internal structures */ - trx_i_s_cache_init(trx_i_s_cache); + trx_i_s_cache_init(); } /*********************************************************************//** @@ -638,7 +638,7 @@ srv_free(void) mysql_mutex_destroy(&page_zip_stat_per_index_mutex); mysql_mutex_destroy(&srv_sys.tasks_mutex); - trx_i_s_cache_free(trx_i_s_cache); + trx_i_s_cache_free(); srv_thread_pool_end(); } diff --git a/storage/innobase/trx/trx0i_s.cc b/storage/innobase/trx/trx0i_s.cc index fdea942e797..70f97faafca 100644 --- a/storage/innobase/trx/trx0i_s.cc +++ b/storage/innobase/trx/trx0i_s.cc @@ -133,11 +133,7 @@ struct trx_i_s_cache_t { /** This is the intermediate buffer where data needed to fill the INFORMATION SCHEMA tables is fetched and later retrieved by the C++ code in handler/i_s.cc. */ -static trx_i_s_cache_t trx_i_s_cache_static; -/** This is the intermediate buffer where data needed to fill the -INFORMATION SCHEMA tables is fetched and later retrieved by the C++ -code in handler/i_s.cc. */ -trx_i_s_cache_t* trx_i_s_cache = &trx_i_s_cache_static; +static trx_i_s_cache_t cache; /** @return the heap number of a record lock @retval 0xFFFF for table locks */ @@ -1067,45 +1063,25 @@ static void fetch_data_into_cache(trx_i_s_cache_t *cache) /*******************************************************************//** Update the transactions cache if it has not been read for some time. -Called from handler/i_s.cc. -@return 0 - fetched, 1 - not */ -int -trx_i_s_possibly_fetch_data_into_cache( -/*===================================*/ - trx_i_s_cache_t* cache) /*!< in/out: cache */ +@retval false when fetched or cached +@retval true if fetched but the cache was truncated */ +bool trx_i_s_possibly_fetch_data_into_cache() { - if (!can_cache_be_updated(cache)) { - - return(1); - } - - /* We need to read trx_sys and record/table lock queues */ - fetch_data_into_cache(cache); - - /* update cache last read time */ - cache->last_read = my_interval_timer(); - - return(0); + bool is_truncated= false; + cache.rw_lock.wr_lock(SRW_LOCK_CALL); + if (can_cache_be_updated(&cache)) + { + /* We need to read trx_sys and record/table lock queues */ + fetch_data_into_cache(&cache); + /* update cache last read time */ + cache.last_read= my_interval_timer(); + is_truncated= cache.is_truncated; + } + cache.rw_lock.wr_unlock(); + return is_truncated; } -/*******************************************************************//** -Returns TRUE if the data in the cache is truncated due to the memory -limit posed by TRX_I_S_MEM_LIMIT. -@return TRUE if truncated */ -bool -trx_i_s_cache_is_truncated( -/*=======================*/ - trx_i_s_cache_t* cache) /*!< in: cache */ -{ - return(cache->is_truncated); -} - -/*******************************************************************//** -Initialize INFORMATION SCHEMA trx related cache. */ -void -trx_i_s_cache_init( -/*===============*/ - trx_i_s_cache_t* cache) /*!< out: cache to init */ +void trx_i_s_cache_init() { /* The latching is done in the following order: acquire trx_i_s_cache_t::rw_lock, rwlock @@ -1115,80 +1091,44 @@ trx_i_s_cache_init( acquire trx_i_s_cache_t::rw_lock, rdlock release trx_i_s_cache_t::rw_lock */ - cache->rw_lock.SRW_LOCK_INIT(trx_i_s_cache_lock_key); + cache.rw_lock.SRW_LOCK_INIT(trx_i_s_cache_lock_key); - cache->last_read = 0; + cache.last_read = 0; - table_cache_init(&cache->innodb_trx, sizeof(i_s_trx_row_t)); - table_cache_init(&cache->innodb_locks, sizeof(i_s_locks_row_t)); - table_cache_init(&cache->innodb_lock_waits, + table_cache_init(&cache.innodb_trx, sizeof(i_s_trx_row_t)); + table_cache_init(&cache.innodb_locks, sizeof(i_s_locks_row_t)); + table_cache_init(&cache.innodb_lock_waits, sizeof(i_s_lock_waits_row_t)); - cache->locks_hash.create(LOCKS_HASH_CELLS_NUM); + cache.locks_hash.create(LOCKS_HASH_CELLS_NUM); - cache->storage = ha_storage_create(CACHE_STORAGE_INITIAL_SIZE, + cache.storage = ha_storage_create(CACHE_STORAGE_INITIAL_SIZE, CACHE_STORAGE_HASH_CELLS); - cache->mem_allocd = 0; + cache.mem_allocd = 0; - cache->is_truncated = false; + cache.is_truncated = false; } -/*******************************************************************//** -Free the INFORMATION SCHEMA trx related cache. */ -void -trx_i_s_cache_free( -/*===============*/ - trx_i_s_cache_t* cache) /*!< in, own: cache to free */ +void trx_i_s_cache_free() { - cache->rw_lock.destroy(); - - cache->locks_hash.free(); - ha_storage_free(cache->storage); - table_cache_free(&cache->innodb_trx); - table_cache_free(&cache->innodb_locks); - table_cache_free(&cache->innodb_lock_waits); + cache.rw_lock.destroy(); + cache.locks_hash.free(); + ha_storage_free(cache.storage); + table_cache_free(&cache.innodb_trx); + table_cache_free(&cache.innodb_locks); + table_cache_free(&cache.innodb_lock_waits); } -/*******************************************************************//** -Issue a shared/read lock on the tables cache. */ -void -trx_i_s_cache_start_read( -/*=====================*/ - trx_i_s_cache_t* cache) /*!< in: cache */ +void trx_i_s_cache_start_read() noexcept { - cache->rw_lock.rd_lock(SRW_LOCK_CALL); + cache.rw_lock.rd_lock(SRW_LOCK_CALL); } -/*******************************************************************//** -Release a shared/read lock on the tables cache. */ -void -trx_i_s_cache_end_read( -/*===================*/ - trx_i_s_cache_t* cache) /*!< in: cache */ +void trx_i_s_cache_end_read() noexcept { - cache->last_read = my_interval_timer(); - cache->rw_lock.rd_unlock(); -} - -/*******************************************************************//** -Issue an exclusive/write lock on the tables cache. */ -void -trx_i_s_cache_start_write( -/*======================*/ - trx_i_s_cache_t* cache) /*!< in: cache */ -{ - cache->rw_lock.wr_lock(SRW_LOCK_CALL); -} - -/*******************************************************************//** -Release an exclusive/write lock on the tables cache. */ -void -trx_i_s_cache_end_write( -/*====================*/ - trx_i_s_cache_t* cache) /*!< in: cache */ -{ - cache->rw_lock.wr_unlock(); + cache.last_read= my_interval_timer(); + cache.rw_lock.rd_unlock(); } /*******************************************************************//** @@ -1198,55 +1138,33 @@ static i_s_table_cache_t* cache_select_table( /*===============*/ - trx_i_s_cache_t* cache, /*!< in: whole cache */ enum i_s_table table) /*!< in: which table */ { switch (table) { case I_S_INNODB_TRX: - return &cache->innodb_trx; + return &cache.innodb_trx; case I_S_INNODB_LOCKS: - return &cache->innodb_locks; + return &cache.innodb_locks; case I_S_INNODB_LOCK_WAITS: - return &cache->innodb_lock_waits; + return &cache.innodb_lock_waits; } ut_error; return NULL; } -/*******************************************************************//** -Retrieves the number of used rows in the cache for a given -INFORMATION SCHEMA table. -@return number of rows */ -ulint -trx_i_s_cache_get_rows_used( -/*========================*/ - trx_i_s_cache_t* cache, /*!< in: cache */ - enum i_s_table table) /*!< in: which table */ +ulint trx_i_s_cache_get_rows_used(i_s_table table) { - i_s_table_cache_t* table_cache; - - table_cache = cache_select_table(cache, table); - - return(table_cache->rows_used); + return cache_select_table(table)->rows_used; } -/*******************************************************************//** -Retrieves the nth row (zero-based) in the cache for a given -INFORMATION SCHEMA table. -@return row */ -void* -trx_i_s_cache_get_nth_row( -/*======================*/ - trx_i_s_cache_t* cache, /*!< in: cache */ - enum i_s_table table, /*!< in: which table */ - ulint n) /*!< in: row number */ +void *trx_i_s_cache_get_nth_row(i_s_table table, ulint n) { i_s_table_cache_t* table_cache; ulint i; void* row; - table_cache = cache_select_table(cache, table); + table_cache = cache_select_table(table); ut_a(n < table_cache->rows_used); From 8a7be7e9aac2ea79cd6ae63baab8ff547c1e3da9 Mon Sep 17 00:00:00 2001 From: Hemant Dangi Date: Tue, 26 May 2026 16:16:28 +0530 Subject: [PATCH 05/23] MDEV-38843: BF applier failed on a node causing complete Cluster lockup Issue: When a BF applier failed to apply a write set and the transaction rollback returned non-zero, wsrep-lib's apply_write_set() short-circuited log_dummy_write_set(). That call releases commit order through commit_order_leave() -- which drives the inconsistency vote -- and advances the storage engine checkpoint. Skipping it left the seqno stuck in commit order, so following appliers blocked on commit_order_enter and the node silently locked the cluster while staying in the primary component. Solution: wsrep-lib now always calls log_dummy_write_set() on the apply-error path, so the apply error reaches the provider and the node is voted out and evicted instead of hanging. The rollback result is still folded into ret, and Wsrep_high_priority_service::rollback() returns its real result again. --- .../suite/galera_3nodes/r/MDEV-38843.result | 55 +++++++++++ .../suite/galera_3nodes/t/MDEV-38843.cnf | 8 ++ .../suite/galera_3nodes/t/MDEV-38843.test | 94 +++++++++++++++++++ sql/wsrep_high_priority_service.cc | 6 ++ wsrep-lib | 2 +- 5 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 mysql-test/suite/galera_3nodes/r/MDEV-38843.result create mode 100644 mysql-test/suite/galera_3nodes/t/MDEV-38843.cnf create mode 100644 mysql-test/suite/galera_3nodes/t/MDEV-38843.test diff --git a/mysql-test/suite/galera_3nodes/r/MDEV-38843.result b/mysql-test/suite/galera_3nodes/r/MDEV-38843.result new file mode 100644 index 00000000000..dc8e4760828 --- /dev/null +++ b/mysql-test/suite/galera_3nodes/r/MDEV-38843.result @@ -0,0 +1,55 @@ +connection node_3; +connection node_2; +connection node_1; +connection node_1; +connection node_2; +connection node_3; +connection node_1; +CREATE TABLE t1 (f1 INTEGER PRIMARY KEY, f2 INT) ENGINE=InnoDB; +INSERT INTO t1 VALUES (1, 0),(2, 0),(3, 0),(4, 0); +connection node_2; +SET SESSION wsrep_on=OFF; +DELETE FROM t1 WHERE f1 = 2; +SET SESSION wsrep_on=ON; +SET @@global.debug_dbug='+d,simulate_rollback_failure_in_applier'; +connection node_1; +UPDATE t1 SET f2 = 1 WHERE f1 = 2; +connection node_2; +SET SESSION wsrep_on = ON; +SET SESSION wsrep_sync_wait = 15; +SET SESSION wsrep_on = ON; +SET SESSION wsrep_sync_wait = 15; +connection node_1; +SET SESSION wsrep_on = ON; +SET SESSION wsrep_sync_wait = 15; +SHOW STATUS LIKE 'wsrep_cluster_size'; +Variable_name Value +wsrep_cluster_size 2 +INSERT INTO t1 VALUES (5, 0); +SELECT * FROM t1 ORDER BY f1; +f1 f2 +1 0 +2 1 +3 0 +4 0 +5 0 +connection node_3; +SELECT * FROM t1 ORDER BY f1; +f1 f2 +1 0 +2 1 +3 0 +4 0 +5 0 +connection node_2; +SET SESSION wsrep_on=OFF; +# restart +connection node_1; +DROP TABLE t1; +connection node_2; +CALL mtr.add_suppression("Can't find record in 't1'"); +CALL mtr.add_suppression("Update_rows_v1 apply failed"); +CALL mtr.add_suppression("Inconsistency detected: Inconsistent by consensus on"); +CALL mtr.add_suppression("WSREP: Failed to apply write set: "); +CALL mtr.add_suppression("Wsrep_high_priority_service::rollback: trans_rollback returned"); +CALL mtr.add_suppression("last left .* greater than drain seqno"); diff --git a/mysql-test/suite/galera_3nodes/t/MDEV-38843.cnf b/mysql-test/suite/galera_3nodes/t/MDEV-38843.cnf new file mode 100644 index 00000000000..8dd4fa1540c --- /dev/null +++ b/mysql-test/suite/galera_3nodes/t/MDEV-38843.cnf @@ -0,0 +1,8 @@ +!include ../galera_3nodes.cnf + +[mysqld] +wsrep-slave-threads=4 +wsrep-ignore-apply-errors=0 + +[ENV] +galera_cluster_size=3 diff --git a/mysql-test/suite/galera_3nodes/t/MDEV-38843.test b/mysql-test/suite/galera_3nodes/t/MDEV-38843.test new file mode 100644 index 00000000000..e56a35a6bad --- /dev/null +++ b/mysql-test/suite/galera_3nodes/t/MDEV-38843.test @@ -0,0 +1,94 @@ +# +# MDEV-38843: BF applier failed on a node causing complete Cluster lockup +# +# On a 3-node cluster, make node 2's applier fail to apply a write set: +# a local (wsrep_on=OFF) DELETE removes a row, then an UPDATE of that row +# from node 1 cannot be applied on node 2. The DBUG injection +# simulate_rollback_failure_in_applier additionally forces the applier's +# transaction rollback to fail, exercising the apply-error path. +# +# Verify that node 2 loses the inconsistency vote and disconnects instead +# of hanging, that the surviving nodes 1 and 3 keep committing, and that +# node 2 can restart and rejoin the cluster. +# + +--source include/galera_cluster.inc +--source include/have_innodb.inc +--source include/have_debug.inc +--source include/have_debug_sync.inc +--source include/galera_have_debug_sync.inc + +# Save original auto_increment_offset values. +--let $node_1=node_1 +--let $node_2=node_2 +--let $node_3=node_3 +--source ../galera/include/auto_increment_offset_save.inc + +--connection node_1 +CREATE TABLE t1 (f1 INTEGER PRIMARY KEY, f2 INT) ENGINE=InnoDB; +INSERT INTO t1 VALUES (1, 0),(2, 0),(3, 0),(4, 0); + +--connection node_2 +# Wait until node 2 has applied the rows before creating the local +# inconsistency. +--let $wait_condition = SELECT COUNT(*) = 4 FROM t1 +--source include/wait_condition.inc + +# Introduce inconsistency so the applier will fail on the next UPDATE. +SET SESSION wsrep_on=OFF; +DELETE FROM t1 WHERE f1 = 2; +SET SESSION wsrep_on=ON; + +# Force the applier's transaction rollback to return non-zero on apply error. +# Pre-fix, this would skip the call that passes the apply error to the +# Galera provider, leaving the seqno stuck in commit order and the cluster +# hanging. Post-fix the call always runs and the vote completes. +SET @@global.debug_dbug='+d,simulate_rollback_failure_in_applier'; + +--connection node_1 +UPDATE t1 SET f2 = 1 WHERE f1 = 2; + +--connection node_2 +# Node 2 lost the consensus vote — it must disconnect from the cluster +# rather than silently hang. Without the fix, this step times out. +--source include/wsrep_wait_disconnect.inc +--let $members=0 +--source include/wsrep_wait_membership.inc + +--connection node_1 +# Survivors form a 2-member primary component. +--let $members=2 +--source include/wsrep_wait_membership.inc +--source include/wait_until_ready.inc +SHOW STATUS LIKE 'wsrep_cluster_size'; + +# The cluster keeps making progress after the eviction. +INSERT INTO t1 VALUES (5, 0); +SELECT * FROM t1 ORDER BY f1; + +--connection node_3 +--let $wait_condition = SELECT COUNT(*)=5 FROM t1 +--source include/wait_condition.inc +SELECT * FROM t1 ORDER BY f1; + +--connection node_2 +# Restart the evicted node so it rejoins via IST/SST. +SET SESSION wsrep_on=OFF; +--source include/shutdown_mysqld.inc +--source include/start_mysqld.inc +--source include/galera_wait_ready.inc + +--connection node_1 +DROP TABLE t1; + +# Suppressions for the expected log noise on node_2. +--connection node_2 +CALL mtr.add_suppression("Can't find record in 't1'"); +CALL mtr.add_suppression("Update_rows_v1 apply failed"); +CALL mtr.add_suppression("Inconsistency detected: Inconsistent by consensus on"); +CALL mtr.add_suppression("WSREP: Failed to apply write set: "); +CALL mtr.add_suppression("Wsrep_high_priority_service::rollback: trans_rollback returned"); +CALL mtr.add_suppression("last left .* greater than drain seqno"); + +# Restore original auto_increment_offset values. +--source ../galera/include/auto_increment_offset_restore.inc diff --git a/sql/wsrep_high_priority_service.cc b/sql/wsrep_high_priority_service.cc index 8aef448db26..97501766a4e 100644 --- a/sql/wsrep_high_priority_service.cc +++ b/sql/wsrep_high_priority_service.cc @@ -379,6 +379,12 @@ int Wsrep_high_priority_service::rollback(const wsrep::ws_handle& ws_handle, assert(ws_handle == wsrep::ws_handle()); } int ret= (trans_rollback_stmt(m_thd) || trans_rollback(m_thd)); + DBUG_EXECUTE_IF("simulate_rollback_failure_in_applier", ret= 1;); + if (ret) + WSREP_WARN("Wsrep_high_priority_service::rollback: trans_rollback " + "returned %d for thd %lu (killed=%d, seqno=%lld)", + ret, thd_get_thread_id(m_thd), m_thd->killed, + (long long) wsrep_thd_trx_seqno(m_thd)); WSREP_DEBUG("::rollback() thread: %lu, client_state %s " "client_mode %s trans_state %s killed %d", diff --git a/wsrep-lib b/wsrep-lib index 14ce8cab76e..5eeef4009d5 160000 --- a/wsrep-lib +++ b/wsrep-lib @@ -1 +1 @@ -Subproject commit 14ce8cab76e9388f7266b91b163a62c654b1d329 +Subproject commit 5eeef4009d52455c44b607e852cb23fbb6e84c99 From d404a47c25b8d5d3b37d9d62c14311934523fd17 Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Wed, 11 Jun 2025 13:52:31 +0300 Subject: [PATCH 06/23] Add missing notify_state_change() in Wsrep_client_service. The change was forgotten from previous rebase/wsrep-lib update. --- sql/wsrep_client_service.h | 1 + 1 file changed, 1 insertion(+) diff --git a/sql/wsrep_client_service.h b/sql/wsrep_client_service.h index f53d9be083d..7608e3d0e3b 100644 --- a/sql/wsrep_client_service.h +++ b/sql/wsrep_client_service.h @@ -68,6 +68,7 @@ public: void debug_sync(const char*) override; void debug_crash(const char*) override; int bf_rollback() override; + void notify_state_change() override { } private: friend class Wsrep_server_service; THD* m_thd; From 70a88a6b6e11d3cb840d5b27d05d967aff099ef0 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Tue, 2 Jun 2026 13:08:07 +0530 Subject: [PATCH 07/23] MDEV-34998 Assertion `n_fields > n_cols || type & 32' failed in dict_index_t::init_change_cols on ALTER Problem: ======= During ALTER TABLE with column type modification and index addition, InnoDB aborts with an assertion failure when processing collation changes for single-column secondary indexes. Solution: ======== dict_index_t::init_change_cols(): A single-column prefix secondary index can have n_cols == n_fields when the column is the same as the primary key column and undergoes a collation or type change during ALTER TABLE. --- mysql-test/suite/innodb/r/change_column_collation.result | 7 +++++++ mysql-test/suite/innodb/t/change_column_collation.test | 8 ++++++++ storage/innobase/include/dict0mem.h | 4 +++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/mysql-test/suite/innodb/r/change_column_collation.result b/mysql-test/suite/innodb/r/change_column_collation.result index 48d3cc16176..efeae748880 100644 --- a/mysql-test/suite/innodb/r/change_column_collation.result +++ b/mysql-test/suite/innodb/r/change_column_collation.result @@ -143,3 +143,10 @@ SET SYSTEM_VERSIONING_ALTER_HISTORY= KEEP; ALTER TABLE t ADD COLUMN v VARCHAR(128) GENERATED ALWAYS AS (CRC32('MariaDB')); ALTER TABLE t ADD INDEX (v); DROP TABLE t; +# +# MDEV-39448 Assertion `n_fields > n_cols || type & 32' failed +# in dict_index_t::init_change_cols on ALTER +# +CREATE OR REPLACE TABLE t1 (c1 VARCHAR(1) KEY) ENGINE=InnoDB; +ALTER TABLE t1 MODIFY c1 VARCHAR(2049),ADD INDEX (c1); +DROP TABLE t1; diff --git a/mysql-test/suite/innodb/t/change_column_collation.test b/mysql-test/suite/innodb/t/change_column_collation.test index edc298fe3e9..d1f34c199b1 100644 --- a/mysql-test/suite/innodb/t/change_column_collation.test +++ b/mysql-test/suite/innodb/t/change_column_collation.test @@ -153,4 +153,12 @@ SET SYSTEM_VERSIONING_ALTER_HISTORY= KEEP; ALTER TABLE t ADD COLUMN v VARCHAR(128) GENERATED ALWAYS AS (CRC32('MariaDB')); ALTER TABLE t ADD INDEX (v); DROP TABLE t; + +--echo # +--echo # MDEV-39448 Assertion `n_fields > n_cols || type & 32' failed +--echo # in dict_index_t::init_change_cols on ALTER +--echo # +CREATE OR REPLACE TABLE t1 (c1 VARCHAR(1) KEY) ENGINE=InnoDB; +ALTER TABLE t1 MODIFY c1 VARCHAR(2049),ADD INDEX (c1); +DROP TABLE t1; --source include/wait_until_count_sessions.inc diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 9b731d50edc..b87d8bbb4ae 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -1353,7 +1353,9 @@ public: @param n_cols number of columns whose collation is changing */ void init_change_cols(unsigned n_cols) { - ut_ad(n_fields > n_cols || type & DICT_FTS); + /* Allow n_fields == n_cols when single column indexes + undergoes type change */ + ut_ad(n_fields >= n_cols || type & DICT_FTS); change_col_info= static_cast (mem_heap_zalloc(heap, sizeof(col_info))); change_col_info->n_cols= n_cols; From fcd3f81e08daea471d371d3be051e6feabb06399 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Wed, 27 May 2026 16:03:43 +0200 Subject: [PATCH 08/23] MDEV-37556 Memory leak in proxy protocol with name resolution enabled When proxy protocol is used and --skip-name-resolve is not set, thd_set_peer_addr() is called twice per connection: once for the real TCP peer (in check_connection), and again for the proxied address (in handle_proxy_header). Each call invokes ip_to_hostname(), which allocates a hostname string (unless loopback connection is used) and stores it as thd->main_security_ctx.host. That code missed to free previously allocated hostname, which results into memory leak. This is now fixed. Also added debug-only test to mysql_client_test, which fakes DNS and IP resolution the same way some perfschema tests do, to emulate remote TCP connection in MTR. --- .../main/mysql_client_test_comp-master.opt | 2 +- .../mysql_client_test_nonblock-master.opt | 2 +- sql/sql_connect.cc | 7 ++ tests/mysql_client_test.c | 79 +++++++++++++++++++ 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/mysql-test/main/mysql_client_test_comp-master.opt b/mysql-test/main/mysql_client_test_comp-master.opt index ccaa4a8ee6e..1f1c9ef7872 100644 --- a/mysql-test/main/mysql_client_test_comp-master.opt +++ b/mysql-test/main/mysql_client_test_comp-master.opt @@ -1,4 +1,4 @@ --loose-enable-performance-schema --max-allowed-packet=32000000 ---proxy-protocol-networks=::1/32,127.0.0.0/8,localhost +--proxy-protocol-networks=::1/32,127.0.0.0/8,localhost,2001:db8::6:6 --sequence=on diff --git a/mysql-test/main/mysql_client_test_nonblock-master.opt b/mysql-test/main/mysql_client_test_nonblock-master.opt index f306d27feac..01a77359b03 100644 --- a/mysql-test/main/mysql_client_test_nonblock-master.opt +++ b/mysql-test/main/mysql_client_test_nonblock-master.opt @@ -1,4 +1,4 @@ --general-log --general-log-file=$MYSQLTEST_VARDIR/log/master.log --log-output=FILE,TABLE --max-allowed-packet=32000000 ---proxy-protocol-networks=::1,::ffff:127.0.0.1/97,localhost +--proxy-protocol-networks=::1,::ffff:127.0.0.1/97,localhost,2001:db8::6:6 --sequence=on diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc index 0841d589e2f..26cfbc6edda 100644 --- a/sql/sql_connect.cc +++ b/sql/sql_connect.cc @@ -877,6 +877,13 @@ int thd_set_peer_addr(THD *thd, { int rc; + /* Free any previously resolved hostname before overwriting it. */ + if (thd->main_security_ctx.host != my_localhost) + { + my_free((void *) thd->main_security_ctx.host); + thd->main_security_ctx.host= NULL; + } + rc = ip_to_hostname(addr, thd->main_security_ctx.ip, &thd->main_security_ctx.host, diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index d4eacd3e3cc..15af8daa9d8 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -20747,6 +20747,84 @@ static void test_proxy_header_ignore() } +/* + MDEV-37556 memory leak in proxy protocol for non-loopback + connections. + + Uses debug_dbug variable to set vio_peer_addr_fake_ipv6 etc + to emulate a remote connection +*/ +static void test_proxy_header_dbug_remote_connection() +{ +#ifndef DBUG_OFF + int rc; + MYSQL *m; + MYSQL_RES *result; + MYSQL_ROW row; + v2_proxy_header v2_header; + char addr_bin[16]; + const char *proxy_ipv6 = "2001:db8::6:6"; + int protocol= MYSQL_PROTOCOL_TCP; + + myheader("test_proxy_header_dbug_remote_connection"); + + /* Save and override debug_dbug so name resolution does not hit real DNS. */ + rc= mysql_query(mysql, + "SET @save_debug_dbug = @@GLOBAL.debug_dbug"); + myquery(rc); + rc= mysql_query(mysql, + "SET GLOBAL debug_dbug='+d,vio_peer_addr_fake_ipv6" + ",getnameinfo_fake_ipv6,getaddrinfo_fake_good_ipv6'"); + myquery(rc); + + /* Create a user identified by the hostname that the debug points will produce. */ + rc= mysql_query(mysql, + "CREATE USER 'u'@'santa.claus.ipv6.example.com' IDENTIFIED BY 'password'"); + myquery(rc); + + /* Build a v2 proxy header announcing proxy_ipv6 as the client address. */ + memset(&v2_header, 0, sizeof(v2_header)); + memcpy(v2_header.sig, "\x0D\x0A\x0D\x0A\x00\x0D\x0A\x51\x55\x49\x54\x0A", 12); + v2_header.ver_cmd = (0x2 << 4) | 0x1; /* PROXY command */ + v2_header.fam = 0x21; /* TCPv6 */ + v2_header.len = htons(36); + inet_pton(AF_INET6, proxy_ipv6, addr_bin); + memcpy(v2_header.addr.ip6.src_addr, addr_bin, 16); + memcpy(v2_header.addr.ip6.dst_addr, addr_bin, 16); + v2_header.addr.ip6.src_port = htons(2222); + v2_header.addr.ip6.dst_port = htons(3306); + + m = mysql_client_init(NULL); + DIE_UNLESS(m != NULL); + mysql_optionsv(m, MARIADB_OPT_PROXY_HEADER, &v2_header, (size_t)52); + mysql_optionsv(m, MYSQL_OPT_PROTOCOL, &protocol); + + if (!mysql_real_connect(m, opt_host, "u", "password", NULL, opt_port, NULL, 0)) + DIE(0); + + /* Verify that the server sees the proxied port, and fake hostname. */ + rc= mysql_query(m, + "SELECT host FROM information_schema.processlist" + " WHERE ID = connection_id()"); + DIE_UNLESS(!rc); + result= mysql_store_result(m); + DIE_UNLESS(result); + row= mysql_fetch_row(result); + DIE_UNLESS(strcmp(row[0], "santa.claus.ipv6.example.com:2222") == 0); + mysql_free_result(result); + + mysql_close(m); + + rc= mysql_query(mysql, "DROP USER 'u'@'santa.claus.ipv6.example.com'"); + myquery(rc); + + /* Restore debug_dbug to whatever it was before this test. */ + rc= mysql_query(mysql, "SET GLOBAL debug_dbug = @save_debug_dbug"); + myquery(rc); +#endif /* !DBUG_OFF */ +} + + static void test_proxy_header() { myheader("test_proxy_header"); @@ -20755,6 +20833,7 @@ static void test_proxy_header() test_proxy_header_tcp("::ffff:192.0.2.1",2222); test_proxy_header_localhost(); test_proxy_header_ignore(); + test_proxy_header_dbug_remote_connection(); } From 087a948e2e4620901d2c358b46907a15b0987059 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Wed, 29 Apr 2026 15:50:19 +1000 Subject: [PATCH 09/23] MDEV-36344: UBSAN DsMrr_impl::dsmrr_init on null ptr Under SQL_SELECT::test_quick_select there isn't a mrr buffer. The TRP_RANGE.mrr_buf_size is explictly sets its size to 0 in get_best_index_intersect. Rather than hit undefined behaviour in what eventually results in full_buf being nullptr, jump the case and go directly to use_default_impl. --- sql/multi_range_read.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sql/multi_range_read.cc b/sql/multi_range_read.cc index 03d952c0f9e..d7e5d1e4f7c 100644 --- a/sql/multi_range_read.cc +++ b/sql/multi_range_read.cc @@ -1193,7 +1193,7 @@ int DsMrr_impl::dsmrr_init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, buf_manager.reset_buffer_sizes= reset_buffer_sizes; buf_manager.redistribute_buffer_space= redistribute_buffer_space; } - else + else if (full_buf) { /* index strategy doesn't need buffer, give all space to rowids*/ rowid_buffer.set_buffer_space(full_buf, full_buf_end); @@ -1201,6 +1201,8 @@ int DsMrr_impl::dsmrr_init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, (int)is_mrr_assoc * sizeof(range_id_t))) goto use_default_impl; } + else + goto use_default_impl; // setup_two_handlers() will call dsmrr_close() will clears the filter. // Save its value and restore afterwards. From 3c684f1e694281a48c257496f09ccc61dde960fc Mon Sep 17 00:00:00 2001 From: Oleg Smirnov Date: Mon, 15 Jun 2026 13:28:20 +1000 Subject: [PATCH 10/23] MDEV-36344: UBSAN Lifo_buffer::have_space_for change Adjust Lifo_buffer::have_space_for to not lead itself to UBSAN overflows. --- sql/sql_lifo_buffer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/sql_lifo_buffer.h b/sql/sql_lifo_buffer.h index afe47b5f415..099e22055ed 100644 --- a/sql/sql_lifo_buffer.h +++ b/sql/sql_lifo_buffer.h @@ -174,7 +174,7 @@ public: uchar *end_of_space() override { return pos; } bool have_space_for(size_t bytes) override { - return (pos + bytes < end); + return (size_t) (end - pos) > bytes; } void write() override From 821fe49dbbb29235f721ebdfbefad17002f0272c Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Mon, 15 Jun 2026 16:41:00 +0200 Subject: [PATCH 11/23] MDEV-40001 my_sync() does not use NtFlushBuffersFileEx() Copy the logic from Innodb. Use a fallback to FlushFileBuffers(), if NtFlushBuffersFileEx() fails. my_winfile.c is renamed to my_winfile.cc, since it is using a C++ feature, initialization of global variable using non-constant expression. Exported functions remain extern "C" --- mysys/CMakeLists.txt | 2 +- mysys/{my_winfile.c => my_winfile.cc} | 30 ++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) rename mysys/{my_winfile.c => my_winfile.cc} (95%) diff --git a/mysys/CMakeLists.txt b/mysys/CMakeLists.txt index 1914e7e9a69..9366994a174 100644 --- a/mysys/CMakeLists.txt +++ b/mysys/CMakeLists.txt @@ -55,7 +55,7 @@ IF (WIN32) my_wintoken.c my_wincond.c my_winerr.c - my_winfile.c + my_winfile.cc my_minidump.cc my_win_popen.cc) ENDIF() diff --git a/mysys/my_winfile.c b/mysys/my_winfile.cc similarity index 95% rename from mysys/my_winfile.c rename to mysys/my_winfile.cc index 35bc6b35399..95ffab33d6c 100644 --- a/mysys/my_winfile.c +++ b/mysys/my_winfile.cc @@ -49,6 +49,9 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335 USA */ #include "mysys_priv.h" #include #include +#include + +extern "C" { /* Associates a file descriptor with an existing operating-system file handle.*/ File my_open_osfhandle(HANDLE handle, int oflag) @@ -709,12 +712,35 @@ int my_win_stat( const char *path, struct _stati64 *buf) DBUG_RETURN(-1); } +/* Dynamically load NtFlushBuffersFileEx, used in my_win_fsync_internal */ +typedef NTSTATUS(WINAPI *pNtFlushBuffersFileEx)( + HANDLE FileHandle, ULONG Flags, PVOID Parameters, ULONG ParametersSize, + PIO_STATUS_BLOCK IoStatusBlock); + + +static int my_win_fsync_internal(HANDLE h) +{ + static const pNtFlushBuffersFileEx my_NtFlushBuffersFileEx= + (pNtFlushBuffersFileEx) GetProcAddress(GetModuleHandle("ntdll"), + "NtFlushBuffersFileEx"); + if (my_NtFlushBuffersFileEx) + { + IO_STATUS_BLOCK iosb{}; + NTSTATUS status= my_NtFlushBuffersFileEx(h, + FLUSH_FLAGS_FILE_DATA_SYNC_ONLY, NULL, 0, &iosb); + if (!status) + return 0; + } + if (FlushFileBuffers(h)) + return 0; + return -1; +} int my_win_fsync(File fd) { DBUG_ENTER("my_win_fsync"); - if(FlushFileBuffers(my_get_osfhandle(fd))) + if (!my_win_fsync_internal(my_get_osfhandle(fd))) DBUG_RETURN(0); my_osmaperr(GetLastError()); DBUG_RETURN(-1); @@ -735,4 +761,6 @@ int my_win_dup(File fd) DBUG_RETURN(-1); } +} /* extern "C" */ + #endif /*_WIN32*/ From b9809c07b412456b194ab342e26d6432e85edf09 Mon Sep 17 00:00:00 2001 From: Kristian Nielsen Date: Tue, 16 Jun 2026 15:43:46 +0200 Subject: [PATCH 12/23] Document the correct use of replication terminology Signed-off-by: Kristian Nielsen --- CODING_STANDARDS.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index 136602b37bd..6177969cc00 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -52,6 +52,19 @@ New features should be based against the latest MariaDB development branch, whic Bug fixes should be based against the earliest maintained branch in which the bug can be reproduced. The earliest maintained branch is found at https://mariadb.org/about/#maintenance-policy. +## Terminology + +In replication, the standard terminology is to use "master" for the server +writing the binlog, and "slave" for the server replicating the binlog. This +terminology must be used consistently everywhere in the git repository, +including source code, comments, error messages, commit messages, test +cases, documentation, etc. + +The words "source" and "replica" may _only_ be used in connection with +aliases for the standard commands (like SHOW REPLICA STATUS), mainly for +MySQL compatibility. The word "primary" to refer to a master must _never_ be +used. + ## Coding Style (C / C++ files) Everyone has a preferred coding style, there is no real correct style for all projects around the world. From 7fc6fce20dbb89b04af77bee540e4451a6a7c9c6 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Wed, 17 Jun 2026 16:12:52 +0200 Subject: [PATCH 13/23] update CODING_STANDARDS.md for agent era * moved human-oriented hopefully eventually consistent codiing style document to https://mariadb.org/about/coding-style/ * rewrote the document in an agent friendly way: main rule first, don't repeat rules that the agent would follow by default anyway, highlight differences with the defaults, don't overdo explaining. * asked claude to look through sql/ and mysys/ extract common patterns and add them here as rules --- CODING_STANDARDS.md | 478 ++++++++++++-------------------------------- 1 file changed, 132 insertions(+), 346 deletions(-) diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index 6177969cc00..8217dc4695d 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -1,393 +1,179 @@ # Coding Standards -This is a working document outlining the coding standard for the general MariaDB codebase. -The document can be found in the 11.0 and newer trees in the root directory as "CODING_STANDARDS.md" +This document outlines the coding standard for this repository. -It does not cover the coding standards for individual plugins, these should have their own coding standards documentation. +It can be overwritten or amended by `CODING_STANDARDS.md` documents in +individual subdirectories. -## Using Git with the MariaDB codebase +This is a concise agent-oriented version, which highlights differences +between "default" coding style that an agent would use anyway and MariaDB. +See human-oriented version at https://mariadb.org/about/coding-style/ -### Git commit messages +## Match the surrounding codebase -Git commit messages must conform to the 50/72 rule. -This is a de facto git standard which is automatically enforced by some editors. -This means: +**This is the primary rule.** When writing code for MariaDB: -* 50 characters max for the first (description) line (see exception later) -* A blank line. -* 72 characters max for every subsequent line. +- Keep the code consistent. +- Use the same terminology, naming conventions, types, helpers, and data + structures as the existing code in the area you are touching. +- Before writing a new utility function or container, grep `mysys/` and + `include/` — it very likely already exists. +- Do not introduce STL containers or other constructs that duplicate + things already in the codebase. -In addition if there is a Jira ticket number, this should be the first thing in the description. -As an example: +## Formatting -``` -MDEV-12345 Fixing Rockwell Turbo Encabulator +### Assignment spacing -The new principle involved is that instead of power being generated by -the relative motion of conductors and fluxes, it’s produced by the -modial interaction of magneto-reluctance and capacitive diractance. -``` - -The only explicitly allowed exception to the 50-72 rules is that if the first line can be MDEV-###### title', even if the title would make the line longer than 50 characters. - -The commit messages are typically rendered in [Markdown format](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax), so markdown formatting is permitted for the message body. - -### Branch handling - -When updating your code, please make sure you perform a rebase, not a merge with the latest branch. -Pull requests should be a simple fast-forward of the branch they are intended to land on. - -The correct way to rebase (if working on top of 10.11 branch): - -```sh -git fetch upstream/10.11 # This assumes upstream is github.com/MariaDB/server -git rebase upstream/10.11 -git push --force my_branch -``` - -### Target branch - -Pull requests should be based against the correct MariaDB version. -New features should be based against the latest MariaDB development branch, which is the current GitHub default branch: https://github.com/MariaDB/server/blob/-/VERSION -Bug fixes should be based against the earliest maintained branch in which the bug can be reproduced. -The earliest maintained branch is found at https://mariadb.org/about/#maintenance-policy. - -## Terminology - -In replication, the standard terminology is to use "master" for the server -writing the binlog, and "slave" for the server replicating the binlog. This -terminology must be used consistently everywhere in the git repository, -including source code, comments, error messages, commit messages, test -cases, documentation, etc. - -The words "source" and "replica" may _only_ be used in connection with -aliases for the standard commands (like SHOW REPLICA STATUS), mainly for -MySQL compatibility. The word "primary" to refer to a master must _never_ be -used. - -## Coding Style (C / C++ files) - -Everyone has a preferred coding style, there is no real correct style for all projects around the world. -What is important is that we stick to one common style throughout this code base. - -### Indentation - -We should use a variant of the [Allman indentation style](https://en.wikipedia.org/wiki/Indentation_style#Allman_style). -The variation is to use two spaces instead of tabs and has a couple of minor rule changes as below. - -Allman style specifies that braces associated with a statement should be on the following line with the same indentation and the statements inside the braces are next level indented. -The closing braces are also on a new line at the same indentation as the original statement. - -For example: +No space before `=`, one space after — **non-standard but grep-friendly**: ```cpp -while (x == y) -{ - something(); - somethingelse(); -} -finalthing(); +a= 1; // correct +a = 1; // wrong ``` -#### Switch / Case statements +### Naming -For switch / case statements the `case` needs to be inline with the `switch`. -Preferably switch (expr) should be followed by '{' on the same line to -make the lineup of 'case:' nice: - -For example: +- Variables and functions: `snake_case` +- Classes and structs: `Upper_snake_case` — first letter uppercase, rest + snake_case: ```cpp -switch(level) { -case ERROR: - sql_print_error("Error: %s", message.c_ptr_safe()); - break; -case WARNING: - sql_print_warning("Warning: %s", message.c_ptr_safe()); - break; -... -} +class Buffered_logs { ... }; ``` -#### If statements +### Pointer declarator -If the `if` statement only executes one line of code it is possible to write the statement without the braces such as this: +`*` binds to the name, not the type: ```cpp -if (opt_console) - opt_error_log= 0; +void my_function(THD *thd); +char *buf; ``` -Prefer reducing indent level with the use of early return statements (or in special circumstances goto). -Rather than: +### Line length + +80 characters max. When breaking a long line: +- Leave binary operators at the **end** of the broken line. +- Use `()` to guide alignment: ```cpp -if (condition) -{ - -} -return error_code; -``` - -Use: - -```cpp -if (!condition) - return error_code; - -return success; -``` - -#### Functions - -Consecutive functions should be separated with 2 empty lines in between - -```cpp -void my_function_1() -{ - -} - - -void my_function_2() -{ - -} -``` - -#### Preprocessor directives - -Compiler preprocessor directives should have no indentation to them, even if in the middle of indented code. -For example: - -```c - case SSL_TYPE_NONE: // SSL is not required - if (opt_require_secure_transport) - { - enum enum_vio_type type= vio_type(vio); -#ifdef HAVE_OPENSSL - return type != VIO_TYPE_SSL && -#ifndef _WIN32 - type != VIO_TYPE_SOCKET; -#else - type != VIO_TYPE_NAMEDPIPE; -#endif -#else -#ifndef _WIN32 - return type != VIO_TYPE_SOCKET; -#else - return type != VIO_TYPE_NAMEDPIPE; -#endif -#endif - } -``` - -Comments reflecting the original `#if` condition can be appended to `#else` / `#endif` to provide additional clarity. This can be useful for large code blocks between the start and end preprocessor directives or nested preprocessor directives. -For example: - -```c -#ifndef EMBEDDED_LIBRARY -... -#else /* ! EMBEDDED_LIBRARY */ -... -#endif /* ! EMBEDDED_LIBRARY */ -``` - -### File names - -File names should be lower case with underscore word separators. -C file names use the `.c` extension, C++ files use the `.cc` extension and header files use the `.h` extension. - -### Language standards - -For pure-C files we use C99 and for C++ we use C++11. -The code need to be able to compile on multiple platforms using different compilers (for example: Windows / Linux, x86_64 / ARM). - -### Line lengths - -Lines should be no more than 80 characters. -The reason for this is that it makes it easier to have multiple editor -windows open side by side and still keep code readable without line -wraps. - -When breaking long lines: -- use '()' to group expressions so that the editor can automatically help - you with the indentation. -- When breaking expressions, leave the operator (+,- etc) last on the previous - line. - -```cpp -rows= tab->table->file->multi_range_read_info(tab->ref.key, 10, 20, tab->ref.key_parts, &bufsz, &flags, &cost); --> rows= tab->table->file->multi_range_read_info(tab->ref.key, 10, 20, - tab->ref.key_parts, &bufsz, - &flags, &cost); -``` - -```cpp -tmp= aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb; --> -tmp= (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+ - bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb); + tab->ref.key_parts, + &bufsz, &flags, &cost); ``` ### Comments -Single line / inline code comments can use the double slash (`//`) style of coding, whereas multi-line code comments should use `/*` as a start and `*/` at the end, with the text indented by 2 spaces, for example: +- Single-line / inline: `//` +- Multi-line: `/*` / `*/` with 2-space indent on the body: ```cpp /* - This is a multi-line code comment. - It has an indentation of two spaces. + This explains a non-obvious invariant. + Second line. */ ``` -### Variables classes, and functions +### Whitespace + +- No trailing whitespace on any line. +- No trailing blank lines at end of file. +- Line endings: LF (`\n`), not CRLF. +- No tabs + +## Types + +Avoid `long` and `unsigned long` (and the `ulong` alias) — `long` is 32-bit +on Windows but 64-bit on Linux/macOS, creating portability issues. +Absolutely avoid them in anything user-visible, configurable variables +and command-line options, no exceptions here. + +Special-purpose: +- `File` — integer file descriptor +- `my_socket` — integer socket descriptor +- `size_t` / `ptrdiff_t` — buffer sizes and pointer arithmetic + +## Memory allocation + +Choose the allocator by lifetime: + +- **Query-scoped (short-lived):** use `MEM_ROOT` — the entire pool is freed + at once at query end. Preferred forms: + ```cpp + new (thd->mem_root) T(...) + alloc_root(mem_root, n) + thd->strmake(str, len) + ``` + pay attention to the `MEM_ROOT` life time. +- **Long-lived or large:** use `my_malloc` / `my_free` or plain `new` / `delete`: + ```cpp + my_malloc(PSI_INSTRUMENT_ME, n, MYF(MY_WME)) + ``` +- **Copying bytes/strings:** `my_memdup()` / `my_strdup()` / `my_strndup()` + instead of malloc+memcpy/strcpy. +- **Multiple objects in one call:** `my_multi_malloc()`. + +## Strings + +- `LEX_CSTRING` / `LEX_STRING` — a `(const char *, length)` pair; use for + charset-less constant strings or where a charset is always the same, + as in Lex_ident* family. +- `String` — dynamic, charset-aware string class; use when charset matters + or the string grows. +- use `StringBuffer` template. + +String helper functions (prefer over stdlib equivalents): + +| Use | Instead of | +|-----|-----------| +| `strnmov(dst, src)` | `strcpy` (returns end pointer, chains well) | +| `strmake(dst, src, len)` | `strncpy` (always null-terminates) | +| `strxnmov(dst, s1, s2, ..., NullS)` | manual multi-part concatenation | + +## Error handling + +- Functions that can fail return `bool`: **`false` = success, `true` = error.** +- Integer error codes: **0 = success, non-zero = error code** -Variables and functions should be descriptive and in "snake case", for example: ```cpp -void my_function(uint16 variable_name) -{ +if (do_something(thd)) // true means error + goto err; ``` -Class names should also be "snake case" but should start with an upper-case character. -Such as this: +- Report errors with `my_error(ER_CODE, MYF(0), ...)`. +- Check for a prior error with `thd->is_error()`. +- Use `goto err` with a cleanup label for failure paths. -```cpp -class Buffered_logs -{ +## Data structures + +Prefer these over STL equivalents: + +- `List` / `List_iterator` — doubly-linked list +- `Dynamic_array` / `DYNAMIC_ARRAY`— growable array +- `Hash` / `HASH` — hash table +- `TREE` — ordered binary search tree (with built-in `MEM_ROOT` pooling) +- `Queue` / `QUEUE` — priority queue +- `IO_CACHE` — buffered sequential file I/O + +## Git + +### Commit messages + +``` +MDEV-12345 exactly issue summary as in Jira + +Body wrapped at 72 characters per line. Subject is normally +50 characters max, but may exceed that when it is the exact +MDEV ticket title. ``` -Assignments should not have a space on the left side of the equals, and one space on the right hand side. For example: +- JIRA ticket number (if any) must be the first thing on the subject line. +- Body is rendered as Markdown. -```cpp -a= 1; // Correct -a = 1; // Incorrect for the server code, - // ok for Storage Engines if they use it (aka Connect) -``` +### Target branch -The above makes it easy to use 'grep' to find all assignments to a variable. - -Please do not write conditions like this: - -```cpp -if (0 == *error_code) -``` - -Please do this instead: - -```cpp -if (*error_code == 0) -// Or even better -if (!*error_code) -``` - -Only use one-character variables (i,j,k...) in short loops. For anything else -use descriptive names! - -### Variable declarations - -Variables should be declared at the start of it's context (start of function, inside the 'if' statement. - -The benefits of this: -- Code lines gets shorter -- It is easier to see the stack space used by a function. -- It is easier to find the declaration of the variable. -- If one has to add an 'if (error) goto end' construct, one can do - that without having to move variable declarations around. - - -### Variable initializations -Variables can be initialized using assignment operator or initializer and expression list. -For Example: - -```cpp -int milliseconds= 1000; -char type= 't'; -``` - -Or - -```cpp -int milliseconds{1000}; -char type{'t'}; -``` - - -### Constant integers - -Constant integers that are used to define elements such as buffer sizes should be defined rather than used directly. -This is because the integer could change and uses of it could be missed. -For example: - -```cpp -char *buffer= my_malloc(PSI_INSTRUMENT_ME, 1024, MYF(MY_WME)); - -snprint(buffer, 1024, "%d: %s", integer, text); -``` - -Could become: - -```cpp -constexpr int buffer_size= 1024; -char *buffer= my_malloc(PSI_INSTRUMENT_ME, buffer_size, MYF(MY_WME)); - -snprint(buffer, buffer_size, "%d: %s", integer, text); -``` - -Alternatively the integer can be defined using an `enum` or `#define`. - -### Spacing - -#### Whitespace - -* Lines should not have any trailing whitespace. -* There should not be any trailing blank lines at the end of a file. -* Line endings are POSIX style (`\n`). -* Two spaces for each indentation level, not tabs. - -#### Pointers - -The `*` of a pointer should be on the side of the variable name such as: - -```cpp -void my_function(THD *thd) -{ -``` - -As yet there is no standard as to whether the `*` in a casting should have a space or not. -Both of these are valid: - -```cpp -name= (const char*)db_name; -name= (const char *) db_name; -``` - -#### Function variables - -There should be a space after each comma in a definition and usage of a function. -For example: - -```cpp -my_function(thd, db_name); -``` - -### Types - -In general the usage of types such as `char`, `int` and `long` should be discouraged but there are shortened versions of the unsigned variants available for these in `my_global.h`. -They can be different sizes across platforms and `char` can be either unsigned or signed depending on platform, and therefore are not portable. -Instead these should be used as appropriate: - -* 8-bit signed / unsigned int -> `int8` / `uint8` -* 16-bit signed / unsigned int -> `int16` / `uint16` -* 32-bit signed / unsigned int -> `int32` / `uint32` -* 64-bit signed / unsigned int -> `int64` / `uint64` -* Integer file descriptor -> `File` -* Integer socket descriptor -> `my_socket` - -`size_t` and `ptrdiff_t` are used in the source where appropriate, buffer sizes for example. -It should be noted that these are implementation dependent but are useful when used in the correct context. - -Further types can be found in the `include/` directory files. -There are also general utility functions in `mysys`. +- New features → `main` branch. +- Bug fixes → oldest maintained branch that reproduces the bug, but not + older than three years since its first GA release. From f80219f1ffade1ec861b25b5da7b837f94f09889 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sat, 6 Jun 2026 20:48:27 +0200 Subject: [PATCH 14/23] MDEV-39292 fix incorrect merge --- debian/autobake-deb.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/debian/autobake-deb.sh b/debian/autobake-deb.sh index 0e256ab6817..1249a097bcc 100755 --- a/debian/autobake-deb.sh +++ b/debian/autobake-deb.sh @@ -162,7 +162,8 @@ then # On Gitlab the output log must stay under 4MB so make the # build less verbose sed '/Add support for verbose builds/,/^$/d' -i debian/rules -elif [[ -d storage/columnstore/columnstore/debian ]] && [[ "$LSBNAME" = !(buster|bionic) ]] +elif grep -qs "$architecture" storage/columnstore/columnstore/debian/control && \ + [[ "$LSBNAME" = !(buster|bionic) ]] then # ColumnStore is explicitly disabled in the native Debian build. Enable it # now when build is triggered by autobake-deb.sh (MariaDB.org) and when the From 0bf0e13a2238a97593feda1fea133340861c0736 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Mon, 15 Jun 2026 22:54:56 +0200 Subject: [PATCH 15/23] MDEV-40058 cached_sha2_password crashes on zero-length password valid encrypted password cannot have zero length and must end with '\0' --- plugin/auth_mysql_sha2/mysql_sha2.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugin/auth_mysql_sha2/mysql_sha2.c b/plugin/auth_mysql_sha2/mysql_sha2.c index d9dbd75f6c9..410def8936c 100644 --- a/plugin/auth_mysql_sha2/mysql_sha2.c +++ b/plugin/auth_mysql_sha2/mysql_sha2.c @@ -124,6 +124,8 @@ static int auth(MYSQL_PLUGIN_VIO *vio, MYSQL_SERVER_AUTH_INFO *info) plain_text[i]^= scramble[i % SCRAMBLE_LENGTH]; pkt= plain_text; pkt_len= (int)plain_text_len; + if (pkt_len <= 0 || pkt[pkt_len-1]) + return CR_ERROR; } /* now pkt contains plaintext password */ From d1693276d5344ba794805e8b5902aaff61edb77f Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Tue, 16 Jun 2026 00:23:43 +0200 Subject: [PATCH 16/23] MDEV-40059 too long character_set_collations crash --- mysql-test/main/ctype_collate_implicit.result | 8 ++++++++ mysql-test/main/ctype_collate_implicit.test | 10 ++++++++++ sql/charset_collations.cc | 4 ++++ 3 files changed, 22 insertions(+) diff --git a/mysql-test/main/ctype_collate_implicit.result b/mysql-test/main/ctype_collate_implicit.result index 7d42ab1bc94..c269f2306e3 100644 --- a/mysql-test/main/ctype_collate_implicit.result +++ b/mysql-test/main/ctype_collate_implicit.result @@ -289,3 +289,11 @@ t1 CREATE TABLE `t1` ( `a` text CHARACTER SET utf8mb3 COLLATE utf8mb3_general_ci DEFAULT NULL ) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci DROP TABLE t1; +# +# MDEV-40059 too long character_set_collations crash +# +SET @@session.character_set_collations='AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=x'; +ERROR 42000: Variable 'character_set_collations' can't be set to the value of 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA...' +SET @@session.character_set_collations='utf8mb3=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'; +ERROR 42000: Variable 'character_set_collations' can't be set to the value of 'utf8mb3=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA...' +# End of 11.4 tests diff --git a/mysql-test/main/ctype_collate_implicit.test b/mysql-test/main/ctype_collate_implicit.test index eb8e3a741f4..2da30d5421b 100644 --- a/mysql-test/main/ctype_collate_implicit.test +++ b/mysql-test/main/ctype_collate_implicit.test @@ -235,3 +235,13 @@ SET @@character_set_collations=''; EXECUTE stmt; SHOW CREATE TABLE t1; DROP TABLE t1; + +--echo # +--echo # MDEV-40059 too long character_set_collations crash +--echo # +--error ER_WRONG_VALUE_FOR_VAR +SET @@session.character_set_collations='AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=x'; +--error ER_WRONG_VALUE_FOR_VAR +SET @@session.character_set_collations='utf8mb3=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'; + +--echo # End of 11.4 tests diff --git a/sql/charset_collations.cc b/sql/charset_collations.cc index 71321cc2a3c..5ed50b07861 100644 --- a/sql/charset_collations.cc +++ b/sql/charset_collations.cc @@ -54,6 +54,10 @@ bool Charset_collation_map_st::insert_or_replace( bool error_on_conflicting_duplicate, myf utf8_flag) { + if (cs_name.length > MY_CS_CHARACTER_SET_NAME_SIZE || + cl_name.length > MY_CS_COLLATION_NAME_SIZE) + return true; + char charset_name_c[MY_CS_CHARACTER_SET_NAME_SIZE + 1/*for '\0'*/]; strmake(charset_name_c, cs_name.str, cs_name.length); CHARSET_INFO *cs= get_charset_by_csname(charset_name_c, From 4ec1ff493ea8da5867f9763b2ee4ef70684e4d80 Mon Sep 17 00:00:00 2001 From: drrtuy Date: Wed, 17 Jun 2026 10:32:10 +0100 Subject: [PATCH 17/23] chore: avoid building DuckDB at x86_32. --- storage/duckdb/CMakeLists.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/storage/duckdb/CMakeLists.txt b/storage/duckdb/CMakeLists.txt index 34d58b2cfcf..6dfb18ac422 100644 --- a/storage/duckdb/CMakeLists.txt +++ b/storage/duckdb/CMakeLists.txt @@ -13,6 +13,14 @@ IF(NOT (CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64" OR return() ENDIF() +# Catch -m32 / multilib builds (e.g. 32-bit build inside an x86_64 CI +# container): CMAKE_SYSTEM_PROCESSOR still reports x86_64, but the target is +# 32-bit. DuckDB (and its jemalloc extension) is 64-bit only. +IF(NOT CMAKE_SIZEOF_VOID_P EQUAL 8) + MESSAGE_ONCE(duckdb "DuckDB: not a 64-bit build, skipping") + return() +ENDIF() + # Check C++17 compiler support MY_CHECK_CXX_COMPILER_FLAG("-std=c++17") IF(NOT have_CXX__std_c__17) From c8f382faa795c6e64dd3514586b2b8e191b57db3 Mon Sep 17 00:00:00 2001 From: drrtuy Date: Wed, 17 Jun 2026 11:00:35 +0100 Subject: [PATCH 18/23] disable sfinae-incomplete warning for DuckDB code. --- storage/duckdb/cmake/duckdb_target_setup.cmake | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/storage/duckdb/cmake/duckdb_target_setup.cmake b/storage/duckdb/cmake/duckdb_target_setup.cmake index 5a79dc85fcf..9798fc90a43 100644 --- a/storage/duckdb/cmake/duckdb_target_setup.cmake +++ b/storage/duckdb/cmake/duckdb_target_setup.cmake @@ -16,6 +16,12 @@ MACRO(duckdb_setup_target _target) TARGET_COMPILE_OPTIONS(${_target} PRIVATE -U_GLIBCXX_DEBUG -U_GLIBCXX_ASSERTIONS ) + # GCC 16+ warns about DuckDB's CompressionInfo in SFINAE contexts. + # Silence it — upstream DuckDB issue, not ours to fix. + MY_CHECK_CXX_COMPILER_FLAG("-Wsfinae-incomplete") + IF(have_CXX__Wsfinae_incomplete) + TARGET_COMPILE_OPTIONS(${_target} PRIVATE -Wno-sfinae-incomplete) + ENDIF() IF(DUCKDB_WERROR) TARGET_COMPILE_OPTIONS(${_target} PRIVATE -Werror) ENDIF() From 0cb2cc959eb4ecaed42deadb5eb93e13dd6835e3 Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Thu, 18 Jun 2026 16:13:33 +0700 Subject: [PATCH 19/23] MDEV-40004: Server crashes in sp_head::register_instr_mem_root_for_deallocation upon shutdown n shutdown server could crash in case triggers executed during server run and some of triggers instructions were re-compiled. The crash is caused by attempt to allocate a memory for storing pointers on mem_roots used for memory allocation taken place on re-parsing failing trigger's statements. The reason of crash is dereferencing of nullptr returning by the function current_thd(). To fix the issue, use dummy THD on shutdown the table definition cache. tdc_start_shutdown() is solely invoked from the function clean_up() but the later is called from many places around the source code, not only from mysql_main(), particularly clean_up() is called from unireg_abort(). Therefore, the extra argument added into the signature of the function cleanup() to allow explicit request of use dummy THD on shutdown the table definition cache. --- mysql-test/main/sp_validation.result | 17 +++++++++++ mysql-test/main/sp_validation.test | 24 +++++++++++++++ sql/mysqld.cc | 8 ++--- sql/table_cache.cc | 44 +++++++++++++++++++++++++++- sql/table_cache.h | 2 +- 5 files changed, 89 insertions(+), 6 deletions(-) diff --git a/mysql-test/main/sp_validation.result b/mysql-test/main/sp_validation.result index 324ac71cb1a..192a5e1d240 100644 --- a/mysql-test/main/sp_validation.result +++ b/mysql-test/main/sp_validation.result @@ -2122,6 +2122,23 @@ i 2 1 1 +DROP TEMPORARY TABLE t1; DROP PACKAGE BODY pkg1; DROP PACKAGE pkg1; SET sql_mode=@save_sql_mode; +# +# MDEV-40004: Server crashes in sp_head::register_instr_mem_root_for_deallocation upon shutdown +# +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (1),(2); +CREATE TRIGGER tr BEFORE UPDATE ON t1 FOR EACH ROW DELETE FROM v1; +CREATE VIEW v1 AS SELECT 0 AS a; +UPDATE t1 SET a = 3; +ERROR HY000: The target table v1 of the DELETE is not updatable +CREATE OR REPLACE VIEW v1 AS SELECT 0 AS a; +UPDATE t1 SET a = 4; +ERROR HY000: The target table v1 of the DELETE is not updatable +# restart +# Clean up +DROP VIEW v1; +DROP TABLE t1; diff --git a/mysql-test/main/sp_validation.test b/mysql-test/main/sp_validation.test index 839024148dc..fc84c6de975 100644 --- a/mysql-test/main/sp_validation.test +++ b/mysql-test/main/sp_validation.test @@ -2923,9 +2923,33 @@ $ --delimiter ; CALL pkg1.p2(); +DROP TEMPORARY TABLE t1; DROP PACKAGE BODY pkg1; DROP PACKAGE pkg1; SET sql_mode=@save_sql_mode; +--echo # +--echo # MDEV-40004: Server crashes in sp_head::register_instr_mem_root_for_deallocation upon shutdown +--echo # +CREATE TABLE t1 (a INT); + +INSERT INTO t1 VALUES (1),(2); +CREATE TRIGGER tr BEFORE UPDATE ON t1 FOR EACH ROW DELETE FROM v1; +CREATE VIEW v1 AS SELECT 0 AS a; + +--error ER_NON_UPDATABLE_TABLE +UPDATE t1 SET a = 3; + +CREATE OR REPLACE VIEW v1 AS SELECT 0 AS a; + +--error ER_NON_UPDATABLE_TABLE +UPDATE t1 SET a = 4; + +--source include/restart_mysqld.inc + +--echo # Clean up +DROP VIEW v1; +DROP TABLE t1; + --enable_ps2_protocol diff --git a/sql/mysqld.cc b/sql/mysqld.cc index f8befb83cf2..a8c382f93cb 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -1565,7 +1565,7 @@ void handle_connections_sockets(); static bool read_init_file(char *file_name); pthread_handler_t handle_slave(void *arg); -static void clean_up(bool print_message); +static void clean_up(bool print_message, bool use_dummy_thd= false); static int test_if_case_insensitive(const char *dir_name); #ifndef EMBEDDED_LIBRARY @@ -1974,7 +1974,7 @@ static void mysqld_exit(int exit_code) #endif /* !EMBEDDED_LIBRARY */ -static void clean_up(bool print_message) +static void clean_up(bool print_message, bool use_dummy_thd) { DBUG_PRINT("exit",("clean_up")); if (cleanup_done++) @@ -2009,7 +2009,7 @@ static void clean_up(bool print_message) lex_free(); /* Free some memory */ item_create_cleanup(); cleanup_json_schema_keyword_hash(); - tdc_start_shutdown(); + tdc_start_shutdown(use_dummy_thd); #ifdef HAVE_REPLICATION semi_sync_master_deinit(); #endif @@ -6164,7 +6164,7 @@ int mysqld_main(int argc, char **argv) close_connections(); ha_pre_shutdown(); - clean_up(1); + clean_up(1, true); sd_notify(0, "STATUS=MariaDB server is down"); /* (void) pthread_attr_destroy(&connection_attrib); */ diff --git a/sql/table_cache.cc b/sql/table_cache.cc index 48cd63401e6..7dd8c0c9ba6 100644 --- a/sql/table_cache.cc +++ b/sql/table_cache.cc @@ -632,7 +632,7 @@ bool tdc_init(void) minimal in order to reduce number of references to pluggable engines. */ -void tdc_start_shutdown(void) +void tdc_start_shutdown(bool use_dummy_thd) { DBUG_ENTER("tdc_start_shutdown"); if (tdc_inited) @@ -645,8 +645,50 @@ void tdc_start_shutdown(void) */ tdc_size= 0; tc_size= 0; + + THD *thd= nullptr; + bool reset_back_current_thd= false; + + if (use_dummy_thd && _current_thd() == nullptr) + { + /* + There is no active THD, use the surrogate one to handle memory + allocations in Sql_alloc::operator new() that uses current_thd() + for getting THD to get access to memory allocation API + */ + thd= new THD(0); + DBUG_ASSERT(thd != nullptr); + + thd->store_globals(); + thd->init(); + thd->set_query_inner((char*) STRING_WITH_LEN("intern:tdc_start_shutdown"), + default_charset_info); + + reset_back_current_thd= true; + } /* Free all cached but unused TABLEs and TABLE_SHAREs. */ + /* + purge_tables() iterates along opened tables close every one of them. + In case tables use triggers it results in destroying sp_head objects + for every associated trigger. Destroying an object of the sp_head class + leads to deleting sp_instr objects. In case some of SP instruction + were previously re-parsed by the reason of changes in tables metadata, + the method + sp_head::register_instr_mem_root_for_deallocation() + is called to add a memory root used for re-parsing to the list of + mem_roots to be freed on sp_head destruction. Mem_roots for later + destruction are placed on the List that use current_thd() for memory + allocation. That is the reason why the dummy THD is created before + in case there is no active THD at the moment the function + tdc_start_shutdown is invoked. + */ purge_tables(); + + if (reset_back_current_thd) + { + thd->reset_globals(); + delete thd; + } } DBUG_VOID_RETURN; } diff --git a/sql/table_cache.h b/sql/table_cache.h index 71704ff2ed0..4847e53b63c 100644 --- a/sql/table_cache.h +++ b/sql/table_cache.h @@ -66,7 +66,7 @@ extern ulong tc_size; extern uint32 tc_instances; extern bool tdc_init(void); -extern void tdc_start_shutdown(void); +extern void tdc_start_shutdown(bool use_dummy_thd); extern void tdc_deinit(void); extern ulong tdc_records(void); extern void tdc_purge(bool all); From 29e43967f4f2c1a1d4af3372354bd203b8418152 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Thu, 18 Jun 2026 18:58:24 +0200 Subject: [PATCH 20/23] only check for duckdb stuff if target duckdb is enabled --- storage/duckdb/CMakeLists.txt | 49 ++++++++++++++++------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/storage/duckdb/CMakeLists.txt b/storage/duckdb/CMakeLists.txt index 6dfb18ac422..2105ed5fc71 100644 --- a/storage/duckdb/CMakeLists.txt +++ b/storage/duckdb/CMakeLists.txt @@ -1,7 +1,3 @@ -IF("NO" STREQUAL "${PLUGIN_DUCKDB}") - return() -ENDIF() - IF(NOT CMAKE_SYSTEM_NAME STREQUAL "Linux") MESSAGE_ONCE(duckdb "DuckDB: not Linux, skipping") return() @@ -33,23 +29,6 @@ ENDIF() SET(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -U_GLIBCXX_DEBUG -U_GLIBCXX_ASSERTIONS") -INCLUDE(${CMAKE_CURRENT_SOURCE_DIR}/cmake/duckdb.cmake) -INCLUDE(${CMAKE_CURRENT_SOURCE_DIR}/cmake/duckdb_target_setup.cmake) - -# Provide MariaDB server include paths to sub-libraries. -# MYSQL_ADD_PLUGIN normally does this, but it runs after ADD_SUBDIRECTORY. -INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/include - ${CMAKE_SOURCE_DIR}/sql - ${CMAKE_BINARY_DIR}/include - ${PCRE_INCLUDE_DIRS} - ${SSL_INCLUDE_DIRS} - ${ZLIB_INCLUDE_DIRS}) - -# Build sub-libraries. -ADD_SUBDIRECTORY(common) -ADD_SUBDIRECTORY(convertor) -ADD_SUBDIRECTORY(runtime) - # Plugin sources (handler layer + UDFs whose extern "C" symbols must be # in the final .so for dlsym() loading). SET(DUCKDB_PLUGIN_SOURCES @@ -58,13 +37,31 @@ SET(DUCKDB_PLUGIN_SOURCES duckdb_udf.cc ) -MYSQL_ADD_PLUGIN(duckdb ${DUCKDB_PLUGIN_SOURCES} - STORAGE_ENGINE - MODULE_ONLY - LINK_LIBRARIES duckdb_runtime duckdb_convertor duckdb_common ${DUCKDB_LIBRARY} -) +MYSQL_ADD_PLUGIN(duckdb ${DUCKDB_PLUGIN_SOURCES} STORAGE_ENGINE MODULE_ONLY) IF(TARGET duckdb) + + INCLUDE(${CMAKE_CURRENT_SOURCE_DIR}/cmake/duckdb.cmake) + INCLUDE(${CMAKE_CURRENT_SOURCE_DIR}/cmake/duckdb_target_setup.cmake) + + # Provide MariaDB server include paths to sub-libraries. + # MYSQL_ADD_PLUGIN normally does this, but it runs after ADD_SUBDIRECTORY. + INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/include + ${CMAKE_SOURCE_DIR}/sql + ${CMAKE_BINARY_DIR}/include + ${PCRE_INCLUDE_DIRS} + ${SSL_INCLUDE_DIRS} + ${ZLIB_INCLUDE_DIRS}) + + # Build sub-libraries. + ADD_SUBDIRECTORY(common) + ADD_SUBDIRECTORY(convertor) + ADD_SUBDIRECTORY(runtime) + + target_link_libraries(duckdb duckdb_runtime duckdb_convertor duckdb_common + ${DUCKDB_LIBRARY} + ) + duckdb_setup_target(duckdb) TARGET_INCLUDE_DIRECTORIES(duckdb PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/common From 1fb075512a7aeab8646a163cbb6f265c49f4c075 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Thu, 18 Jun 2026 18:30:17 +0200 Subject: [PATCH 21/23] refactor submodule.cmake to fetch less don't update all submodules automatically, only update those that are actually used by the build --- cmake/mariadb_connector_c.cmake | 2 +- cmake/ssl.cmake | 1 + cmake/submodules.cmake | 84 ++++++++++++++++-------------- cmake/wsrep.cmake | 8 +-- storage/columnstore/CMakeLists.txt | 2 +- storage/duckdb/cmake/duckdb.cmake | 8 +-- storage/maria/CMakeLists.txt | 1 + storage/rocksdb/CMakeLists.txt | 2 + 8 files changed, 53 insertions(+), 55 deletions(-) diff --git a/cmake/mariadb_connector_c.cmake b/cmake/mariadb_connector_c.cmake index 9e25b94117d..47dc56b67d6 100644 --- a/cmake/mariadb_connector_c.cmake +++ b/cmake/mariadb_connector_c.cmake @@ -45,7 +45,7 @@ SET(CLIENT_PLUGIN_PVIO_SHMEM STATIC) SET(CLIENT_PLUGIN_PVIO_SOCKET STATIC) MESSAGE("== Configuring MariaDB Connector/C") -ADD_SUBDIRECTORY(libmariadb) +ADD_SUBMODULE_SUBDIRECTORY(libmariadb) IF(MSVC AND TARGET mariadb_obj AND TARGET mariadbclient) # With MSVC, do not produce LTCG-compiled static client libraries. diff --git a/cmake/ssl.cmake b/cmake/ssl.cmake index a050a2ba343..6bf1d72622d 100644 --- a/cmake/ssl.cmake +++ b/cmake/ssl.cmake @@ -62,6 +62,7 @@ MACRO (MYSQL_USE_BUNDLED_SSL) SET(HAVE_des ON CACHE INTERNAL "wolfssl does support DES API") SET(HAVE_hkdf ON CACHE INTERNAL "wolfssl does support EVP_PKEY API") CHANGE_SSL_SETTINGS("bundled") + ADD_SUBMODULE(extra/wolfssl/wolfssl) ADD_SUBDIRECTORY(extra/wolfssl) MESSAGE_ONCE(SSL_LIBRARIES "SSL_LIBRARIES = ${SSL_LIBRARIES}") ENDMACRO() diff --git a/cmake/submodules.cmake b/cmake/submodules.cmake index 34dcfbea77a..b7ea8f743d2 100644 --- a/cmake/submodules.cmake +++ b/cmake/submodules.cmake @@ -1,50 +1,54 @@ -# update submodules automatically - OPTION(UPDATE_SUBMODULES "Update submodules automatically" ON) -IF(NOT UPDATE_SUBMODULES) - RETURN() -ENDIF() -IF(GIT_EXECUTABLE AND EXISTS "${CMAKE_SOURCE_DIR}/.git") +IF(NOT UPDATE_SUBMODULES) + SET(SUBMODULE_UPDATE_CONFIG_MESSAGE "Disabled by -DUPDATE_SUBMODULES=OFF") +ELSEIF(NOT GIT_EXECUTABLE) + SET(SUBMODULE_UPDATE_CONFIG_MESSAGE "git executable was not found") +ELSEIF(NOT EXISTS "${CMAKE_SOURCE_DIR}/.git") + SET(SUBMODULE_UPDATE_CONFIG_MESSAGE "Not inside a git repository") +ELSE() EXECUTE_PROCESS(COMMAND "${GIT_EXECUTABLE}" config --get cmake.update-submodules WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" - OUTPUT_VARIABLE cmake_update_submodules + OUTPUT_VARIABLE CMAKE_UPDATE_SUBMODULES RESULT_VARIABLE git_config_get_result) - IF(cmake_update_submodules MATCHES no) + IF(CMAKE_UPDATE_SUBMODULES MATCHES no) SET(update_result 0) - SET(SUBMODULE_UPDATE_CONFIG_MESSAGE -"\n\nTo update submodules automatically, set cmake.update-submodules to 'yes', or 'force' to update automatically: - ${GIT_EXECUTABLE} config cmake.update-submodules yes") + SET(SUBMODULE_UPDATE_CONFIG_MESSAGE "Disabled by git config. To enable set cmake.update-submodules to 'yes', or 'force': ${GIT_EXECUTABLE} config cmake.update-submodules yes") ELSEIF(git_config_get_result EQUAL 128) - SET(update_result 0) - ELSE() - SET(UPDATE_SUBMODULES_COMMAND - "${GIT_EXECUTABLE}" submodule update --init --recursive) - # Old Git may not work with "--depth 1". - # See also: https://github.com/git/git/commit/fb43e31f2b43076e7a30c9cd00d0241cb8cf97eb - IF(NOT GIT_VERSION_STRING VERSION_LESS "2.8.0") - SET(UPDATE_SUBMODULES_COMMAND ${UPDATE_SUBMODULES_COMMAND} --depth 1) - ENDIF() - IF(cmake_update_submodules MATCHES force) - MESSAGE(STATUS "Updating submodules (forced)") - EXECUTE_PROCESS(COMMAND ${UPDATE_SUBMODULES_COMMAND} --force - WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" - RESULT_VARIABLE update_result) - ELSEIF(cmake_update_submodules MATCHES yes) - EXECUTE_PROCESS(COMMAND ${UPDATE_SUBMODULES_COMMAND} - WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" - RESULT_VARIABLE update_result) - ELSE() - MESSAGE(STATUS "Updating submodules") - EXECUTE_PROCESS(COMMAND ${UPDATE_SUBMODULES_COMMAND} - WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" - RESULT_VARIABLE update_result) - ENDIF() + SET(SUBMODULE_UPDATE_CONFIG_MESSAGE "Git executable ${GIT_EXECUTABLE} failed to run") ENDIF() ENDIF() -IF(update_result OR NOT EXISTS ${CMAKE_SOURCE_DIR}/libmariadb/CMakeLists.txt) - MESSAGE(FATAL_ERROR "No MariaDB Connector/C! Run - ${GIT_EXECUTABLE} submodule update --init --recursive -Then restart the build.${SUBMODULE_UPDATE_CONFIG_MESSAGE}") -ENDIF() +FUNCTION(ADD_SUBMODULE dir) + IF (ARGV1) + SET(file "${ARGV1}") + ELSE() + SET(file CMakeLists.txt) + ENDIF() + IF(SUBMODULE_UPDATE_CONFIG_MESSAGE) + IF(NOT EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${dir}/${file}) + MESSAGE(FATAL_ERROR "Cannot download ${CMAKE_CURRENT_SOURCE_DIR}/${dir} submodule: ${SUBMODULE_UPDATE_CONFIG_MESSAGE}") + ENDIF() + ELSE() + MESSAGE(STATUS "Downloading ${CMAKE_CURRENT_SOURCE_DIR}/${dir} submodule...") + SET(UPDATE_SUBMODULES_COMMAND + "${GIT_EXECUTABLE}" submodule update --init --recursive --depth 1) + IF(CMAKE_UPDATE_SUBMODULES MATCHES force) + EXECUTE_PROCESS(COMMAND ${UPDATE_SUBMODULES_COMMAND} --force ${dir} + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" + RESULT_VARIABLE update_result) + ELSE() + EXECUTE_PROCESS(COMMAND ${UPDATE_SUBMODULES_COMMAND} ${dir} + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" + RESULT_VARIABLE update_result) + ENDIF() + IF(update_result OR NOT EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${dir}/${file}) + MESSAGE(FATAL_ERROR "Failed to download ${CMAKE_CURRENT_SOURCE_DIR}/${dir} submodule") + ENDIF() + ENDIF() +ENDFUNCTION() + +MACRO(ADD_SUBMODULE_SUBDIRECTORY dir) + ADD_SUBMODULE(${dir}) + ADD_SUBDIRECTORY(${dir}) +ENDMACRO() diff --git a/cmake/wsrep.cmake b/cmake/wsrep.cmake index 95df72400c7..91f040bcdb6 100644 --- a/cmake/wsrep.cmake +++ b/cmake/wsrep.cmake @@ -35,12 +35,8 @@ IF(WITH_WSREP) # Set the patch version SET(WSREP_PATCH_VERSION "22") - IF(NOT EXISTS "${CMAKE_SOURCE_DIR}/wsrep-lib/wsrep-API/v26/wsrep_api.h") - MESSAGE(FATAL_ERROR "No MariaDB wsrep-API code! Run - ${GIT_EXECUTABLE} submodule update --init --recursive -Then restart the build. -") - ENDIF() + ADD_SUBMODULE(wsrep-lib) + # Obtain wsrep API version FILE(STRINGS "${CMAKE_SOURCE_DIR}/wsrep-lib/wsrep-API/v26/wsrep_api.h" WSREP_API_VERSION LIMIT_COUNT 1 REGEX "WSREP_INTERFACE_VERSION") diff --git a/storage/columnstore/CMakeLists.txt b/storage/columnstore/CMakeLists.txt index 491955e0e2f..3ce9edec1ff 100644 --- a/storage/columnstore/CMakeLists.txt +++ b/storage/columnstore/CMakeLists.txt @@ -56,7 +56,7 @@ CMAKE_SYSTEM_PROCESSOR STREQUAL "amd64" OR CMAKE_SYSTEM_PROCESSOR STREQUAL "aarch64") MY_CHECK_AND_SET_COMPILER_FLAG("-fno-lto") SET(PCRE_INCLUDES "${PCRE_INCLUDE_DIRS}") - add_subdirectory(columnstore) + add_submodule_subdirectory(columnstore) IF(TARGET columnstore) # Redo logic in cmake/plugin to prevent the attempted creation of *Symlinks package diff --git a/storage/duckdb/cmake/duckdb.cmake b/storage/duckdb/cmake/duckdb.cmake index 1cd6fca5bac..2b6fbd9363d 100644 --- a/storage/duckdb/cmake/duckdb.cmake +++ b/storage/duckdb/cmake/duckdb.cmake @@ -25,13 +25,6 @@ SET(DUCKDB_SUBMODULE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/third_parties/duckdb") SET(DUCKDB_INCLUDE_DIR "${DUCKDB_SUBMODULE_DIR}/src/include") -IF(NOT EXISTS "${DUCKDB_SUBMODULE_DIR}/CMakeLists.txt") - MESSAGE(FATAL_ERROR - "DuckDB submodule not found at ${DUCKDB_SUBMODULE_DIR}\n" - "Run: git submodule update --init ${DUCKDB_SUBMODULE_DIR}" - ) -ENDIF() - INCLUDE(ExternalProject) # Map MariaDB build type to a DuckDB-friendly one. @@ -56,6 +49,7 @@ SET(_DUCKDB_STATIC_LIBS ) MESSAGE(STATUS "=== Building DuckDB from submodule (${DUCKDB_SUBMODULE_DIR}) ===") +ADD_SUBMODULE(third_parties/duckdb) ExternalProject_Add(duckdb_build PREFIX "${_DUCKDB_BUILD_DIR}" diff --git a/storage/maria/CMakeLists.txt b/storage/maria/CMakeLists.txt index 9bdd7298400..9d528f980dc 100644 --- a/storage/maria/CMakeLists.txt +++ b/storage/maria/CMakeLists.txt @@ -131,6 +131,7 @@ SET(CPACK_RPM_s3-engine_PACKAGE_SUMMARY "Amazon S3 archival storage engine for M SET(CPACK_RPM_s3-engine_PACKAGE_DESCRIPTION "The S3 storage engine allows one to archive MariaDB tables in Amazon S3 (or any third-party public or private cloud that implements S3 API), but still have them accessible in MariaDB in read-only mode." PARENT_SCOPE) IF(TARGET s3) + ADD_SUBMODULE(libmarias3 Makefile.am) MYSQL_ADD_EXECUTABLE(aria_s3_copy aria_s3_copy.cc ${S3_SOURCES} COMPONENT s3-engine) TARGET_LINK_LIBRARIES(aria_s3_copy aria myisam mysys mysys_ssl ${CURL_LIBRARIES} ${ZLIB_LIBRARIES}) INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}/libmarias3) diff --git a/storage/rocksdb/CMakeLists.txt b/storage/rocksdb/CMakeLists.txt index 0f99f19348e..5be48c507ea 100644 --- a/storage/rocksdb/CMakeLists.txt +++ b/storage/rocksdb/CMakeLists.txt @@ -163,6 +163,8 @@ IF(NOT TARGET rocksdb) RETURN() ENDIF() +ADD_SUBMODULE(rocksdb) + INSTALL_MANPAGES(rocksdb-engine mariadb-ldb.1 myrocks_hotbackup.1) CHECK_CXX_SOURCE_COMPILES(" From f139e207175b5bbb9f8f0bc24227c61379746218 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Mon, 15 Jun 2026 11:59:29 +0300 Subject: [PATCH 22/23] MDEV-40027 : Galera Cluster-peer > Donor command execution An appropriately privileged user (with SUPER privileges) could execute shell commands as the uid of the mariadbd process because the values of the system variable wsrep_sst_auth, which can be modified at runtime, were not properly sanitized when used to construct a shell command. Combined rsync and mariabackup test cases and added test case for incorrect values for wsrep_sst_auth. --- .../galera/r/galera_sst_cn_injection.result | 24 +++++ ...a_sst_rsync_encrypt_with_key_server.result | 27 ----- ...server.cnf => galera_sst_cn_injection.cnf} | 1 - .../t/galera_sst_cn_injection.combinations | 6 ++ ...rver.test => galera_sst_cn_injection.test} | 27 +++-- ...st_mariabackup_encrypt_with_key_server.cnf | 13 --- ...era_sst_rsync_encrypt_with_key_server.test | 99 ------------------- .../sys_vars/r/wsrep_sst_auth_basic.result | 22 +++++ .../sys_vars/t/wsrep_sst_auth_basic.test | 22 +++++ scripts/wsrep_sst_mariabackup.sh | 3 +- sql/wsrep_sst.cc | 95 +++++++++--------- sql/wsrep_utils.cc | 48 +++++++++ sql/wsrep_utils.h | 11 +++ 13 files changed, 192 insertions(+), 206 deletions(-) create mode 100644 mysql-test/suite/galera/r/galera_sst_cn_injection.result delete mode 100644 mysql-test/suite/galera/r/galera_sst_rsync_encrypt_with_key_server.result rename mysql-test/suite/galera/t/{galera_sst_rsync_encrypt_with_key_server.cnf => galera_sst_cn_injection.cnf} (91%) create mode 100644 mysql-test/suite/galera/t/galera_sst_cn_injection.combinations rename mysql-test/suite/galera/t/{galera_sst_mariabackup_encrypt_with_key_server.test => galera_sst_cn_injection.test} (68%) delete mode 100644 mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.cnf delete mode 100644 mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.test diff --git a/mysql-test/suite/galera/r/galera_sst_cn_injection.result b/mysql-test/suite/galera/r/galera_sst_cn_injection.result new file mode 100644 index 00000000000..9b177d38865 --- /dev/null +++ b/mysql-test/suite/galera/r/galera_sst_cn_injection.result @@ -0,0 +1,24 @@ +connection node_2; +connection node_1; +SELECT 1; +1 +1 +# +# MDEV-39413 wsrep unsafe handling of parameters +# +call mtr.add_suppression("WSREP: Illegal character in state transfer request:"); +call mtr.add_suppression("WSREP: Bad remote auth string. SST canceled."); +call mtr.add_suppression("WSREP: SST preparation failed"); +call mtr.add_suppression("WSREP: SST failed:"); +call mtr.add_suppression(".*State transfer to:*"); +call mtr.add_suppression("WSREP: Illegal character in variable:"); +connection node_2; +# force SST again +# using a cert with shell-unsafe CN +# start the server +# the server failed to start +FOUND 1 /Bad remote auth string. SST canceled./ in mysqld.1.err +# cleanup +# restart +call mtr.add_suppression(".*WSREP.*Will never receive state. Need to abort."); +call mtr.add_suppression("WSREP_SST:"); diff --git a/mysql-test/suite/galera/r/galera_sst_rsync_encrypt_with_key_server.result b/mysql-test/suite/galera/r/galera_sst_rsync_encrypt_with_key_server.result deleted file mode 100644 index 48b296a96f5..00000000000 --- a/mysql-test/suite/galera/r/galera_sst_rsync_encrypt_with_key_server.result +++ /dev/null @@ -1,27 +0,0 @@ -connection node_2; -connection node_1; -SELECT 1; -1 -1 -connection node_2; -FOUND 1 /wsrep_sst_rsync/ in mysqld.1.err -connection node_1; -call mtr.add_suppression('Invalid value for WSREP_SST_OPT_REMOTE_USER'); -call mtr.add_suppression('Failed to read from: wsrep_sst_rsync'); -call mtr.add_suppression('Process completed with error: wsrep_sst_rsync'); -call mtr.add_suppression('Command did not run: wsrep_sst_rsync'); -call mtr.add_suppression('State transfer to .* failed'); -call mtr.add_suppression('Will never receive state. Need to abort'); -call mtr.add_suppression('Error while getting data from donor node'); -call mtr.add_suppression('Cleanup after exit with status'); -call mtr.add_suppression('Removing .*/sst_in_progress'); -call mtr.add_suppression('Parent mysqld process .* terminated unexpectedly'); -connection node_2; -connection node_1; -FOUND 1 /Invalid value for WSREP_SST_OPT_REMOTE_USER/ in mysqld.1.err -connection node_2; -# restart -call mtr.add_suppression('Will never receive state. Need to abort'); -call mtr.add_suppression('Parent mysqld process .* terminated unexpectedly'); -call mtr.add_suppression('Cleanup after exit with status'); -call mtr.add_suppression('State transfer to .* failed'); diff --git a/mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.cnf b/mysql-test/suite/galera/t/galera_sst_cn_injection.cnf similarity index 91% rename from mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.cnf rename to mysql-test/suite/galera/t/galera_sst_cn_injection.cnf index 9b919145a31..9ec227ec3db 100644 --- a/mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.cnf +++ b/mysql-test/suite/galera/t/galera_sst_cn_injection.cnf @@ -1,7 +1,6 @@ !include ../galera_2nodes.cnf [mysqld] -wsrep_sst_method=rsync wsrep_sst_auth="root:" wsrep_debug=1 diff --git a/mysql-test/suite/galera/t/galera_sst_cn_injection.combinations b/mysql-test/suite/galera/t/galera_sst_cn_injection.combinations new file mode 100644 index 00000000000..1f8f972f5cb --- /dev/null +++ b/mysql-test/suite/galera/t/galera_sst_cn_injection.combinations @@ -0,0 +1,6 @@ +[rsync] +wsrep-sst-method=rsync + +[mariabackup] +wsrep-sst-method=mariabackup + diff --git a/mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.test b/mysql-test/suite/galera/t/galera_sst_cn_injection.test similarity index 68% rename from mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.test rename to mysql-test/suite/galera/t/galera_sst_cn_injection.test index 0418be4d797..8458e6fe4b3 100644 --- a/mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.test +++ b/mysql-test/suite/galera/t/galera_sst_cn_injection.test @@ -15,24 +15,16 @@ SELECT 1; --let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size'; --source include/wait_condition.inc -# Confirm that transfer was SSL-encrypted ---let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err ---let SEARCH_PATTERN = Using openssl based encryption with socat: with key and crt ---source include/search_pattern_in_file.inc - --echo # --echo # MDEV-39413 wsrep unsafe handling of parameters --echo # -call mtr.add_suppression('Invalid value for WSREP_SST_OPT_REMOTE_USER'); -call mtr.add_suppression('Failed to read from: wsrep_sst_mariabackup'); -call mtr.add_suppression('Process completed with error: wsrep_sst_mariabackup'); -call mtr.add_suppression('Command did not run: wsrep_sst_mariabackup'); -call mtr.add_suppression('State transfer to .* failed'); -call mtr.add_suppression('Will never receive state. Need to abort'); -call mtr.add_suppression('Error while getting data from donor node'); -call mtr.add_suppression('Cleanup after exit with status'); -call mtr.add_suppression('Removing .*/xtrabackup_galera_info file due to signal'); +call mtr.add_suppression("WSREP: Illegal character in state transfer request:"); +call mtr.add_suppression("WSREP: Bad remote auth string. SST canceled."); +call mtr.add_suppression("WSREP: SST preparation failed"); +call mtr.add_suppression("WSREP: SST failed:"); +call mtr.add_suppression(".*State transfer to:*"); +call mtr.add_suppression("WSREP: Illegal character in variable:"); --connection node_2 --source include/shutdown_mysqld.inc @@ -51,7 +43,8 @@ call mtr.add_suppression('Removing .*/xtrabackup_galera_info file due to signal' --exec $MYSQLD_LAST_CMD --echo # the server failed to start ---let SEARCH_PATTERN = Invalid value for WSREP_SST_OPT_REMOTE_USER +--let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err +--let SEARCH_PATTERN = Bad remote auth string. SST canceled. --source include/search_pattern_in_file.inc --echo # cleanup @@ -60,6 +53,10 @@ call mtr.add_suppression('Removing .*/xtrabackup_galera_info file due to signal' # pkill exit code varies by platform 0 or 1 --error 0,1 --exec pkill -f 'socat.*server-new-cert' + --exec echo ssl-cert=$MYSQL_TEST_DIR/std_data/server-cert.pem >> $MYSQLTEST_VARDIR/my.cnf --exec echo ssl-key=$MYSQL_TEST_DIR/std_data/server-key.pem >> $MYSQLTEST_VARDIR/my.cnf --source $MYSQL_TEST_DIR/include/start_mysqld.inc + +call mtr.add_suppression(".*WSREP.*Will never receive state. Need to abort."); +call mtr.add_suppression("WSREP_SST:"); diff --git a/mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.cnf b/mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.cnf deleted file mode 100644 index 0dc79df5a80..00000000000 --- a/mysql-test/suite/galera/t/galera_sst_mariabackup_encrypt_with_key_server.cnf +++ /dev/null @@ -1,13 +0,0 @@ -!include ../galera_2nodes.cnf - -[mysqld] -wsrep_sst_method=mariabackup -wsrep_sst_auth="root:" -wsrep_debug=1 - -ssl-cert=@ENV.MYSQL_TEST_DIR/std_data/server-cert.pem -ssl-key=@ENV.MYSQL_TEST_DIR/std_data/server-key.pem -ssl-ca=@ENV.MYSQL_TEST_DIR/std_data/cacert.pem - -[sst] -ssl-mode=VERIFY_CA diff --git a/mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.test b/mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.test deleted file mode 100644 index f0458e1f62f..00000000000 --- a/mysql-test/suite/galera/t/galera_sst_rsync_encrypt_with_key_server.test +++ /dev/null @@ -1,99 +0,0 @@ -# -# Verifies that wsrep_sst_rsync.sh rejects a joiner-supplied certificate -# whose CN contains shell-unsafe characters. -# -# Brings up a 2-node cluster with rsync SST and ssl-mode=VERIFY_CA, then -# forces a fresh SST on node_2 using std_data/server-new-cert.pem -- a -# cert whose CN intentionally contains shell metacharacters. Confirms -# that the donor (node_1) logs -# "Invalid value for WSREP_SST_OPT_REMOTE_USER" -# i.e. the rsync SST script refuses the value rather than interpolating -# it into stunnel.conf or the rsync magic file. -# - ---source include/galera_cluster.inc ---source include/have_innodb.inc - -SELECT 1; - ---connection node_2 ---let $wait_condition = SELECT VARIABLE_VALUE = 'Synced' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'WSREP_LOCAL_STATE_COMMENT' ---source include/wait_condition.inc - -# Confirm the initial SST went via rsync + stunnel (sanity check for the -# test configuration). ---let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err ---let SEARCH_PATTERN = wsrep_sst_rsync ---source include/search_pattern_in_file.inc - - -# Reject shell-unsafe joiner-supplied auth (rsync) - -# Suppressions are per-server. node_1 will log the donor-side rejection -# ("Invalid value for WSREP_SST_OPT_REMOTE_USER"); node_2 will log the -# joiner-side "Will never receive state" abort. Add to both. ---connection node_1 -call mtr.add_suppression('Invalid value for WSREP_SST_OPT_REMOTE_USER'); -call mtr.add_suppression('Failed to read from: wsrep_sst_rsync'); -call mtr.add_suppression('Process completed with error: wsrep_sst_rsync'); -call mtr.add_suppression('Command did not run: wsrep_sst_rsync'); -call mtr.add_suppression('State transfer to .* failed'); -call mtr.add_suppression('Will never receive state. Need to abort'); -call mtr.add_suppression('Error while getting data from donor node'); -call mtr.add_suppression('Cleanup after exit with status'); -call mtr.add_suppression('Removing .*/sst_in_progress'); -call mtr.add_suppression('Parent mysqld process .* terminated unexpectedly'); - ---connection node_2 ---source include/shutdown_mysqld.inc - -# force SST again ---remove_file $MYSQLTEST_VARDIR/mysqld.2/data/grastate.dat -# using a cert with shell-unsafe CN ---exec echo '[mysqld.2]' >> $MYSQLTEST_VARDIR/my.cnf ---exec echo ssl-cert=$MYSQL_TEST_DIR/std_data/server-new-cert.pem >> $MYSQLTEST_VARDIR/my.cnf ---exec echo ssl-key=$MYSQL_TEST_DIR/std_data/server-new-key.pem >> $MYSQLTEST_VARDIR/my.cnf - -# start the server -# Joiner mariadbd exits when SST is aborted; the exit code varies by -# platform (clean 0 on some systems, signalled 134 / 1 on others). ---error 0,1,134 ---exec $MYSQLD_LAST_CMD -# the donor refused the SST request - ---connection node_1 -# safe() in wsrep_sst_common.sh logs this when it rejects the joiner CN; -# wsrep_sst_rsync.sh wraps the joiner-supplied REMOTE_USER with $(safe ..) -# at line 249 so the value never reaches the stunnel.conf heredoc. ---let SEARCH_PATTERN = Invalid value for WSREP_SST_OPT_REMOTE_USER ---source include/search_pattern_in_file.inc - -# cleanup -# Kill joiner's stunnel / rsync that may linger after the aborted SST. -# Use a perl block because --exec with pkill -f matches the mtr cmdline -# itself (which contains the pattern) and tears down the wrong process. -perl; - open(my $fh, '-|', 'ps', '-eo', 'pid,args') or die "ps: $!"; - while (<$fh>) { - next unless /server-new-cert/; - next unless /^\s*(\d+)\s+(?:.*\/)?(stunnel|socat|rsync)\b/; - kill 'TERM', $1; - } - close $fh; -EOF ---exec echo ssl-cert=$MYSQL_TEST_DIR/std_data/server-cert.pem >> $MYSQLTEST_VARDIR/my.cnf ---exec echo ssl-key=$MYSQL_TEST_DIR/std_data/server-key.pem >> $MYSQLTEST_VARDIR/my.cnf - -# Switch back to node_2 before restarting it; the connection associates -# with the soon-to-be-restarted server so mtr auto-reconnects and the -# wait_condition + late suppressions land on the new instance. ---connection node_2 ---source $MYSQL_TEST_DIR/include/start_mysqld.inc - ---let $wait_condition = SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size' ---source include/wait_condition.inc - -call mtr.add_suppression('Will never receive state. Need to abort'); -call mtr.add_suppression('Parent mysqld process .* terminated unexpectedly'); -call mtr.add_suppression('Cleanup after exit with status'); -call mtr.add_suppression('State transfer to .* failed'); diff --git a/mysql-test/suite/sys_vars/r/wsrep_sst_auth_basic.result b/mysql-test/suite/sys_vars/r/wsrep_sst_auth_basic.result index e6b532c6bba..4ffd6df81e8 100644 --- a/mysql-test/suite/sys_vars/r/wsrep_sst_auth_basic.result +++ b/mysql-test/suite/sys_vars/r/wsrep_sst_auth_basic.result @@ -46,7 +46,29 @@ SELECT @@global.wsrep_sst_auth; NULL SET @@global.wsrep_sst_auth=user:pass; ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ':pass' at line 1 +SET @@global.wsrep_sst_auth='user new:pass'; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user new:pass' +SET @@global.wsrep_sst_auth='user$new:pass'; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user$new:pass' +SET @@global.wsrep_sst_auth="user'new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user'new:pass' +SET @@global.wsrep_sst_auth="user;new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user;new:pass' +SET @@global.wsrep_sst_auth="user|new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user|new:pass' +SET @@global.wsrep_sst_auth="user&new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user&new:pass' +SET @@global.wsrep_sst_auth="user\\new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user\new:pass' +SET @@global.wsrep_sst_auth="user\"new:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user"new:pass' +SET @@global.wsrep_sst_auth="user\nnew:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user +new:pass' +SET @@global.wsrep_sst_auth="user\tnew:pass"; +ERROR 42000: Variable 'wsrep_sst_auth' can't be set to the value of 'user new:pass' # restore the initial value SET @@global.wsrep_sst_auth = @wsrep_sst_auth_global_saved; +call mtr.add_suppression("WSREP: Illegal character in variable:"); # End of test diff --git a/mysql-test/suite/sys_vars/t/wsrep_sst_auth_basic.test b/mysql-test/suite/sys_vars/t/wsrep_sst_auth_basic.test index aa901ef9ff7..1961bc89652 100644 --- a/mysql-test/suite/sys_vars/t/wsrep_sst_auth_basic.test +++ b/mysql-test/suite/sys_vars/t/wsrep_sst_auth_basic.test @@ -37,9 +37,31 @@ SET @@global.wsrep_sst_auth=1; SELECT @@global.wsrep_sst_auth; --error ER_PARSE_ERROR SET @@global.wsrep_sst_auth=user:pass; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth='user new:pass'; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth='user$new:pass'; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user'new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user;new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user|new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user&new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user\\new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user\"new:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user\nnew:pass"; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_sst_auth="user\tnew:pass"; --echo --echo # restore the initial value SET @@global.wsrep_sst_auth = @wsrep_sst_auth_global_saved; +call mtr.add_suppression("WSREP: Illegal character in variable:"); + --echo # End of test diff --git a/scripts/wsrep_sst_mariabackup.sh b/scripts/wsrep_sst_mariabackup.sh index 1ced7087f70..e3e52d92fcb 100644 --- a/scripts/wsrep_sst_mariabackup.sh +++ b/scripts/wsrep_sst_mariabackup.sh @@ -1117,7 +1117,8 @@ if [ "$WSREP_SST_OPT_ROLE" = 'donor' ]; then wsrep_log_info "Using '$itmpdir' as mariadb-backup working directory" if [ -n "$WSREP_SST_OPT_USER" ]; then - INNOEXTRA="$INNOEXTRA --user='$WSREP_SST_OPT_USER'" + WSREP_SST_OPT_USER_SAFE="$(safe WSREP_SST_OPT_USER)" + INNOEXTRA="$INNOEXTRA --user='$WSREP_SST_OPT_USER_SAFE'" fi if [ -n "$WSREP_SST_OPT_PSWD" ]; then diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index d40c7a77026..5fd81bec2c4 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -172,48 +172,6 @@ static void* wsrep_sst_joiner_monitor_thread(void *arg __attribute__((unused))) return NULL; } -/* return true if character can be a part of a filename */ -static bool filename_char(int const c) -{ - return isalnum(c) || (c == '-') || (c == '_') || (c == '.'); -} - -/* return true if string is comma seprated list */ -static bool comma_char(int const c) -{ - return (c == ','); -} - -/* return true if character can be a part of an address string */ -static bool address_char(int const c) -{ - return filename_char(c) || - (c == ':') || (c == '[') || (c == ']') || (c == '/'); -} - -/* return true if character can be a part of an address string list */ -static bool names_list(int const c) -{ - return address_char(c) || comma_char(c); -} - -static bool check_request_str(const char* const str, - bool (*check) (int c), - bool log_warn = true) -{ - for (size_t i(0); str[i] != '\0'; ++i) - { - if (!check(str[i])) - { - if (log_warn) WSREP_WARN("Illegal character in state transfer request: %i (%c).", - str[i], str[i]); - return true; - } - } - - return false; -} - bool wsrep_sst_method_check (sys_var *self, THD* thd, set_var* var) { if ((! var->save_result.string_value.str) || @@ -226,8 +184,8 @@ bool wsrep_sst_method_check (sys_var *self, THD* thd, set_var* var) } /* check also that method name is alphanumeric string */ - if (check_request_str(var->save_result.string_value.str, - filename_char, false)) + if (wsrep_check_request_str(var->save_result.string_value.str, + wsrep_filename_char, false)) { my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, var->save_result.string_value.str ? @@ -279,8 +237,8 @@ bool wsrep_sst_receive_address_check (sys_var *self, THD* thd, set_var* var) } /* check also that address contains only accepted characters */ - if (check_request_str(var->save_result.string_value.str, - address_char, false)) + if (wsrep_check_request_str(var->save_result.string_value.str, + wsrep_address_char, false)) { goto err; } @@ -302,7 +260,37 @@ bool wsrep_sst_receive_address_update (sys_var *self, THD* thd, bool wsrep_sst_auth_check (sys_var *self, THD* thd, set_var* var) { + /* Allow empty value */ + if (!var->save_result.string_value.str || var->save_result.string_value.length == 0) return 0; + + /* Check length */ + if ((var->save_result.string_value.length > (FN_REFLEN - 1))) // safety + { + goto err; + } + + { + /* Split sst_auth on ':'-character */ + std::string auth= var->save_result.string_value.str; + std::string r_user= auth.substr(0, auth.find(":")); + + /* check also that user contains only accepted characters, + password part is not validated. */ + if (wsrep_check_request_str(r_user.c_str(), + wsrep_filename_char, true)) + { + goto err; + } + } + + return 0; + +err: + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, + var->save_result.string_value.str ? + var->save_result.string_value.str : "NULL"); + return 1; } static bool sst_auth_real_set (const char* value) @@ -372,8 +360,8 @@ bool wsrep_sst_donor_check (sys_var *self, THD* thd, set_var* var) return 0; /* check also that donor string contains only accepted characters */ - if (check_request_str(var->save_result.string_value.str, - names_list, false)) + if (wsrep_check_request_str(var->save_result.string_value.str, + wsrep_names_list, false)) { goto err; } @@ -2030,7 +2018,7 @@ int wsrep_sst_donate(const std::string& msg, const char* method= msg.data(); size_t method_len= strlen (method); - if (check_request_str(method, filename_char, true)) + if (wsrep_check_request_str(method, wsrep_filename_char, true)) { WSREP_ERROR("Bad SST method name. SST canceled."); return WSREP_CB_FAILURE; @@ -2052,7 +2040,14 @@ int wsrep_sst_donate(const std::string& msg, addr= data; } - if (check_request_str(addr, address_char, true)) + if (remote_auth() && + wsrep_check_request_str(remote_auth(), wsrep_shell_char, true)) + { + WSREP_ERROR("Bad remote auth string. SST canceled."); + return WSREP_CB_FAILURE; + } + + if (wsrep_check_request_str(addr, wsrep_address_char, true)) { WSREP_ERROR("Bad SST address string. SST canceled."); return WSREP_CB_FAILURE; diff --git a/sql/wsrep_utils.cc b/sql/wsrep_utils.cc index a8098b560e6..5cda63e52fb 100644 --- a/sql/wsrep_utils.cc +++ b/sql/wsrep_utils.cc @@ -1,4 +1,5 @@ /* Copyright 2010-2015 Codership Oy + Copyright 2025-2026 MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -598,3 +599,50 @@ size_t wsrep_host_len(const char* const addr, size_t const addr_len) return (colon ? colon - addr : addr_len); } } + +/* return true if character can be a part of a filename */ +bool wsrep_filename_char(int const c) +{ + return isalnum(c) || (c == '-') || (c == '_') || (c == '.'); +} + +/* return true if string is comma seprated list */ +bool wsrep_comma_char(int const c) +{ + return (c == ','); +} + +/* return true if character can be a part of an address string */ +bool wsrep_address_char(int const c) +{ + return wsrep_filename_char(c) || + (c == ':') || (c == '[') || (c == ']') || (c == '/'); +} + +bool wsrep_shell_char(int const c) +{ + return (c != '`') && (c != '\'') && (c != '$') && (c != ' '); +} + +/* return true if character can be a part of an address string list */ +bool wsrep_names_list(int const c) +{ + return wsrep_address_char(c) || wsrep_comma_char(c); +} + +bool wsrep_check_request_str(const char* const str, + bool (*check) (int c), + bool log_warn) +{ + for (size_t i(0); str[i] != '\0'; ++i) + { + if (!check(str[i])) + { + if (log_warn) WSREP_WARN("Illegal character in variable: %i (%c).", + str[i], str[i]); + return true; + } + } + + return false; +} diff --git a/sql/wsrep_utils.h b/sql/wsrep_utils.h index d34b15e3b09..648ed50672d 100644 --- a/sql/wsrep_utils.h +++ b/sql/wsrep_utils.h @@ -1,4 +1,5 @@ /* Copyright (C) 2013-2015 Codership Oy + Copyright (C) 2025-2026 MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -423,4 +424,14 @@ private: } // namespace wsrep +// Functions to validate dynamic variable safety +bool wsrep_filename_char(int const c); +bool wsrep_comma_char(int const c); +bool wsrep_address_char(int const c); +bool wsrep_shell_char(int const c); +bool wsrep_names_list(int const c); +bool wsrep_check_request_str(const char* const str, + bool (*check) (int c), + bool log_warn = true); + #endif /* WSREP_UTILS_H */ From 8b27beead6277f1fc15a750f7c226be3761e14cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Wed, 17 Jun 2026 09:38:33 +0300 Subject: [PATCH 23/23] MDEV-40056 : Analyze Galera Dynamic Variables Susceptible to RCEs Fix potential issue with wsrep_node_address by allowing only correctly constructed address. --- .../r/wsrep_node_address_basic.result | 20 +++++++++++++++++++ .../sys_vars/t/wsrep_node_address_basic.test | 17 ++++++++++++++-- sql/wsrep_utils.cc | 18 +++++++++-------- sql/wsrep_utils.h | 12 +++++------ sql/wsrep_var.cc | 20 +++++++++---------- 5 files changed, 61 insertions(+), 26 deletions(-) diff --git a/mysql-test/suite/sys_vars/r/wsrep_node_address_basic.result b/mysql-test/suite/sys_vars/r/wsrep_node_address_basic.result index e9a93d2fcd6..3c011674b61 100644 --- a/mysql-test/suite/sys_vars/r/wsrep_node_address_basic.result +++ b/mysql-test/suite/sys_vars/r/wsrep_node_address_basic.result @@ -31,6 +31,26 @@ ERROR 42000: Variable 'wsrep_node_address' can't be set to the value of 'NULL' SELECT @@global.wsrep_node_address; @@global.wsrep_node_address +SET @@global.wsrep_node_address="127.0.0.1'ps"; +ERROR 42000: Variable 'wsrep_node_address' can't be set to the value of '127.0.0.1'ps' +SELECT @@global.wsrep_node_address; +@@global.wsrep_node_address + +SET @@global.wsrep_node_address="127.0.0.1 ps"; +ERROR 42000: Variable 'wsrep_node_address' can't be set to the value of '127.0.0.1 ps' +SELECT @@global.wsrep_node_address; +@@global.wsrep_node_address + +SET @@global.wsrep_node_address="127.0.0.1`ps"; +ERROR 42000: Variable 'wsrep_node_address' can't be set to the value of '127.0.0.1`ps' +SELECT @@global.wsrep_node_address; +@@global.wsrep_node_address + +SET @@global.wsrep_node_address="127.0.0.1$ps"; +ERROR 42000: Variable 'wsrep_node_address' can't be set to the value of '127.0.0.1$ps' +SELECT @@global.wsrep_node_address; +@@global.wsrep_node_address + SET @@global.wsrep_node_address=ON; SELECT @@global.wsrep_node_address; @@global.wsrep_node_address diff --git a/mysql-test/suite/sys_vars/t/wsrep_node_address_basic.test b/mysql-test/suite/sys_vars/t/wsrep_node_address_basic.test index fccb40de6bf..65772c515c9 100644 --- a/mysql-test/suite/sys_vars/t/wsrep_node_address_basic.test +++ b/mysql-test/suite/sys_vars/t/wsrep_node_address_basic.test @@ -29,8 +29,21 @@ SELECT @@global.wsrep_node_address; --error ER_WRONG_VALUE_FOR_VAR SET @@global.wsrep_node_address=NULL; SELECT @@global.wsrep_node_address; -# The values being assigned to wsrep_node_address are not verified so the -# following alues are currently valid too. +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_node_address="127.0.0.1'ps"; +SELECT @@global.wsrep_node_address; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_node_address="127.0.0.1 ps"; +SELECT @@global.wsrep_node_address; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_node_address="127.0.0.1`ps"; +SELECT @@global.wsrep_node_address; +--error ER_WRONG_VALUE_FOR_VAR +SET @@global.wsrep_node_address="127.0.0.1$ps"; +SELECT @@global.wsrep_node_address; + +# Values assigned to wsrep_node_address are only checked for unsafe characters, +# so non-address strings like ON/OFF/junk are currently accepted too. SET @@global.wsrep_node_address=ON; SELECT @@global.wsrep_node_address; SET @@global.wsrep_node_address='OFF'; diff --git a/sql/wsrep_utils.cc b/sql/wsrep_utils.cc index 5cda63e52fb..1cb4fcfb562 100644 --- a/sql/wsrep_utils.cc +++ b/sql/wsrep_utils.cc @@ -601,37 +601,39 @@ size_t wsrep_host_len(const char* const addr, size_t const addr_len) } /* return true if character can be a part of a filename */ -bool wsrep_filename_char(int const c) +bool wsrep_filename_char(const unsigned char c) { return isalnum(c) || (c == '-') || (c == '_') || (c == '.'); } -/* return true if string is comma seprated list */ -bool wsrep_comma_char(int const c) +/* return true if string is comma separated list */ +bool wsrep_comma_char(const unsigned char c) { return (c == ','); } /* return true if character can be a part of an address string */ -bool wsrep_address_char(int const c) +bool wsrep_address_char(const unsigned char c) { return wsrep_filename_char(c) || (c == ':') || (c == '[') || (c == ']') || (c == '/'); } -bool wsrep_shell_char(int const c) +bool wsrep_shell_char(const unsigned char c) { - return (c != '`') && (c != '\'') && (c != '$') && (c != ' '); + return (c != '`') && (c != '\'') && (c != '$') && + (c != ' ') && (c != '\t') && (c != '\n') && + (c != '\r') && (c != '\v') && (c != '\f'); } /* return true if character can be a part of an address string list */ -bool wsrep_names_list(int const c) +bool wsrep_names_list(const unsigned char c) { return wsrep_address_char(c) || wsrep_comma_char(c); } bool wsrep_check_request_str(const char* const str, - bool (*check) (int c), + bool (*check) (const unsigned char), bool log_warn) { for (size_t i(0); str[i] != '\0'; ++i) diff --git a/sql/wsrep_utils.h b/sql/wsrep_utils.h index 648ed50672d..9d66519c7de 100644 --- a/sql/wsrep_utils.h +++ b/sql/wsrep_utils.h @@ -425,13 +425,13 @@ private: } // namespace wsrep // Functions to validate dynamic variable safety -bool wsrep_filename_char(int const c); -bool wsrep_comma_char(int const c); -bool wsrep_address_char(int const c); -bool wsrep_shell_char(int const c); -bool wsrep_names_list(int const c); +bool wsrep_filename_char(const unsigned char c); +bool wsrep_comma_char(const unsigned char c); +bool wsrep_address_char(const unsigned char c); +bool wsrep_shell_char(const unsigned char c); +bool wsrep_names_list(const unsigned char c); bool wsrep_check_request_str(const char* const str, - bool (*check) (int c), + bool (*check) (const unsigned char), bool log_warn = true); #endif /* WSREP_UTILS_H */ diff --git a/sql/wsrep_var.cc b/sql/wsrep_var.cc index 152563a061d..306a07151ca 100644 --- a/sql/wsrep_var.cc +++ b/sql/wsrep_var.cc @@ -1,4 +1,5 @@ /* Copyright 2008-2023 Codership Oy + Copyright 2025-2026 MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -632,10 +633,10 @@ static int wsrep_cluster_address_verify (const char* cluster_address_str) bool wsrep_cluster_address_check (sys_var *self, THD* thd, set_var* var) { - char addr_buf[FN_REFLEN]; + char addr_buf[FN_REFLEN+1]; if ((! var->save_result.string_value.str) || - (var->save_result.string_value.length >= sizeof(addr_buf))) // safety + (var->save_result.string_value.length >= FN_REFLEN)) // safety goto err; strmake(addr_buf, var->save_result.string_value.str, @@ -738,20 +739,19 @@ bool wsrep_node_name_update (sys_var *self, THD* thd, enum_var_type type) return 0; } -// TODO: do something more elaborate, like checking connectivity bool wsrep_node_address_check (sys_var *self, THD* thd, set_var* var) { - char addr_buf[FN_REFLEN]; - if ((! var->save_result.string_value.str) || - (var->save_result.string_value.length > (FN_REFLEN - 1))) // safety + (var->save_result.string_value.length >= FN_REFLEN)) // safety goto err; - memcpy(addr_buf, var->save_result.string_value.str, - var->save_result.string_value.length); - addr_buf[var->save_result.string_value.length]= 0; + if (var->save_result.string_value.length) + { + if (wsrep_check_request_str(var->save_result.string_value.str, + wsrep_address_char, false)) + goto err; + } - // TODO: for now 'allow' 0-length string to be valid (default) return 0; err: