MDEV-40085 TRUNCATE of temporary table with ENCRYPTED=NO crashes under innodb_encrypt_tables=FORCE#5308
MDEV-40085 TRUNCATE of temporary table with ENCRYPTED=NO crashes under innodb_encrypt_tables=FORCE#5308Thirunarayanan wants to merge 1 commit into
Conversation
under innodb_encrypt_tables=FORCE Problem: ========= ha_innobase::truncate() recreates the table by calling ha_innobase::create(). create_table_info_t::check_table_options() rejects ENCRYPTED=NO when innodb_encrypt_tables=FORCE, which is the rule that forbids creating a new unencrypted table under FORCE. TRUNCATE therefore failed in create(). For a temporary table, truncate() frees m_prebuilt before calling create(). If the create() fails and left m_prebuilt=nullptr and a subsequent ha_innobase::reset()/update_thd() dereferenced it. Solution: ======== check_table_options(): skip the innodb_encrypt_tables=FORCE check on the internal recreate path (indicated by a non-null m_trx, which is set only during truncate()). ha_innobase::truncate(): validate the create options before dropping the existing table. Any genuine validation failure (missing encryption key, unsupported option combination etc) now returns an error with the original table and m_prebuilt left intact, instead of dropping the table and then failing in create()
|
|
There was a problem hiding this comment.
Code Review
This pull request introduces validation of table creation options during temporary table truncation in InnoDB to prevent dropping the table if validation fails, along with corresponding test cases. Feedback on these changes highlights that using m_trx to detect internal recreate/truncate paths is fragile and suggests replacing it with an explicit boolean flag. Additionally, a potential null pointer dereference on trx during error handling should be resolved with a null check, and spacing around assignment operators should be corrected to match coding standards.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| compile_time_assert(FIL_DEFAULT_ENCRYPTION_KEY == 1); | ||
| } | ||
| if (srv_encrypt_tables != 2) { | ||
| if (m_trx || srv_encrypt_tables != 2) { |
There was a problem hiding this comment.
Using m_trx as a proxy to detect the internal recreate/truncate path is fragile and poses a maintainability risk. If a future refactoring passes the active transaction pointer (trx) to create_table_info_t during a normal CREATE TABLE (which is a very reasonable change since the constructor accepts it), the innodb_encrypt_tables=FORCE check will be silently bypassed for all new tables, creating a security vulnerability.
Instead, consider adding an explicit boolean flag (e.g., m_is_recreate or similar) to create_table_info_t to clearly indicate when the validation is performed as part of an internal recreate/truncate operation.
| { | ||
| char norm_name[FN_REFLEN], remote_path[FN_REFLEN]; | ||
| create_table_info_t validate(m_user_thd, table, &info, norm_name, | ||
| remote_path, true, trx); | ||
| int err= validate.initialize(); | ||
| if (!err) | ||
| err= validate.prepare_create_table(ib_table->name.m_name, false); | ||
| if (err) | ||
| { | ||
| trx_rollback_for_mysql(trx); | ||
| trx->free(); | ||
| DBUG_RETURN(err); | ||
| } | ||
| } |
There was a problem hiding this comment.
To prevent potential null pointer dereferences, we should add a NULL check for trx before calling trx->free() and trx_rollback_for_mysql(trx). Additionally, please ensure there are spaces around the assignment operators (=) to adhere to the project's coding standards.
{
char norm_name[FN_REFLEN], remote_path[FN_REFLEN];
create_table_info_t validate(m_user_thd, table, &info, norm_name,
remote_path, true, trx);
int err = validate.initialize();
if (!err)
err = validate.prepare_create_table(ib_table->name.m_name, false);
if (err)
{
if (trx)
{
trx_rollback_for_mysql(trx);
trx->free();
}
DBUG_RETURN(err);
}
}References
- Follow the project's specific coding standards (e.g., CODING_STANDARDS.md) for code formatting and spacing, such as spacing around assignment operators, rather than generic style guides like the Google C++ Style Guide. (link)
Problem:
ha_innobase::truncate() recreates the table by calling ha_innobase::create(). create_table_info_t::check_table_options() rejects ENCRYPTED=NO when innodb_encrypt_tables=FORCE, which is the rule that forbids creating a new unencrypted table under FORCE.
TRUNCATE therefore failed in create(). For a temporary table, truncate() frees m_prebuilt before calling create(). If the create() fails and left m_prebuilt=nullptr and a subsequent ha_innobase::reset()/update_thd() dereferenced it.
Solution:
check_table_options(): skip the innodb_encrypt_tables=FORCE check on the internal recreate path (indicated by a non-null m_trx, which is set only during truncate()).
ha_innobase::truncate(): validate the create options before dropping the existing table. Any genuine validation failure (missing encryption key, unsupported option combination etc) now returns an error with the original table and m_prebuilt left intact, instead of dropping the table and then failing in create()