Marko Mäkelä added a comment - 2022-08-09 23:15 The error is being emitted outside InnoDB: 10.6 558f1eff64e7708b594ef0315e23bdeb1d23ccf7 #0 my_error (nr=1860, MyFlags=0) at /mariadb/10.6/mysys/my_error.c:109 #1 0x0000562ef21ef3a2 in mysql_rename_table (base=base@entry=0x562ef4e0da48, old_db=old_db@entry=0x7fc21801fb58, old_name=old_name@entry=0x7fc22e5b0f80, new_db=new_db@entry=0x7fc218020270, new_name=new_name@entry=0x7fc22e5b0f90, id=id@entry=0x7fc22e5b0fa0, flags=<optimized out>) at /mariadb/10.6/sql/sql_table.cc:4974 #2 0x0000562ef2173510 in do_rename (thd=0x7fc218000c58, param=0x7fc22e5b0f80, ddl_log_state=0x7fc22e5b0f60, ren_table=0x7fc21801fb40, new_db=0x7fc218020270, skip_error=<optimized out>, force_if_exists=<optimized out>) at /mariadb/10.6/sql/sql_rename.cc:383 #3 0x0000562ef217399e in rename_tables (skip_error=false, force_if_exists=0x7fc22e5b0f2f, if_exists=false, ddl_log_state=0x7fc22e5b0f60, table_list=0x7fc21801fb40, thd=0x7fc218000c58) at /mariadb/10.6/sql/sql_rename.cc:533 #4 mysql_rename_tables (thd=thd@entry=0x7fc218000c58, table_list=table_list@entry=0x7fc21801fb40, silent=silent@entry=false, if_exists=false) at /mariadb/10.6/sql/sql_rename.cc:164 I do not think that we should change that. Only the assertion failure in InnoDB needs to be prevented.

Marko Mäkelä added a comment - 2022-08-30 07:23 diff --git a/mysql-test/suite/innodb/t/foreign_key_not_windows.test b/mysql-test/suite/innodb/t/foreign_key_not_windows.test index 7ad3723d5de..071dc52a4d4 100644 --- a/mysql-test/suite/innodb/t/foreign_key_not_windows.test +++ b/mysql-test/suite/innodb/t/foreign_key_not_windows.test @@ -38,6 +38,18 @@ eval CREATE TABLE `$d255`.`_$d250` --replace_result $d255 d255 eval CREATE TABLE `$d255`.`$d250` (a INT PRIMARY KEY, FOREIGN KEY(a) REFERENCES test.t(a)) ENGINE=InnoDB; + +--echo # +--echo # MDEV-29258 Failing assertion for name length on RENAME TABLE +--echo # + +let $d245=#################################################; +eval CREATE TABLE `$d255`.`$d245` (x INT) ENGINE=InnoDB; +eval DROP TABLE `$d255`.`$d250`; + +--error ER_IDENT_CAUSES_TOO_LONG_PATH +eval RENAME TABLE `$d255`.`$d245` TO `$d255`.`$d250`; +eval RENAME TABLE `$d255`.`$d245` TO a; --replace_result $d255 d255 eval DROP DATABASE `$d255`; DROP TABLE t; 10.3 0d1de5e1d19f1e96058ab5948e184f22e7bdd908 mysqld: /mariadb/10.3/storage/innobase/trx/trx0rec.cc:1870: ulint trx_undo_page_report_rename(trx_t*, const dict_table_t*, buf_block_t*, mtr_t*): Assertion `len <= (64U*3) * 2 + 1' failed.

Marko Mäkelä added a comment - 2022-08-30 07:56 The InnoDB problems in 10.3 were as follows: diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index a196303c39f..5d9f80eda70 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -2105,7 +2105,7 @@ fil_op_write_log( case MLOG_FILE_RENAME2: ut_ad(strchr(new_path, OS_PATH_SEPARATOR) != NULL); len = strlen(new_path) + 1; - log_ptr = mlog_open(mtr, 2 + len); + log_ptr = mlog_open(mtr, 2); ut_a(log_ptr); mach_write_to_2(log_ptr, len); log_ptr += 2; diff --git a/storage/innobase/trx/trx0rec.cc b/storage/innobase/trx/trx0rec.cc index e011b3f5d8e..5c76fb56c48 100644 --- a/storage/innobase/trx/trx0rec.cc +++ b/storage/innobase/trx/trx0rec.cc @@ -1867,9 +1867,9 @@ trx_undo_page_report_rename(trx_t* trx, const dict_table_t* table, byte* start = block->frame + first_free; size_t len = strlen(table->name.m_name); const size_t fixed = 2 + 1 + 11 + 11 + 2; - ut_ad(len <= NAME_LEN * 2 + 1); + ut_ad(len <= NAME_CHAR_LEN * 5 * 2 + 1); /* The -10 is used in trx_undo_left() */ - compile_time_assert((NAME_LEN * 1) * 2 + fixed + compile_time_assert(NAME_CHAR_LEN * 5 * 2 + fixed + TRX_UNDO_PAGE_HDR + TRX_UNDO_PAGE_HDR_SIZE < UNIV_PAGE_SIZE_MIN - 10 - FIL_PAGE_DATA_END); With these fixed, the test case would show a buffer overflow in the SQL layer: 10.3 0d1de5e1d19f1e96058ab5948e184f22e7bdd908 with InnoDB patches ==31744==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fa636c472d0 at pc 0x55ae1787c95a bp 0x7fa636c46d10 sp 0x7fa636c46d08 WRITE of size 1 at 0x7fa636c472d0 thread T27 #0 0x55ae1787c959 in strxmov /mariadb/10.3/strings/strxmov.c:53 #1 0x55ae165b2563 in rename_file_ext(char const*, char const*, char const*) /mariadb/10.3/sql/table.cc:4202 #2 0x55ae16532381 in mysql_rename_table(handlerton*, st_mysql_const_lex_string const*, st_mysql_const_lex_string const*, st_mysql_const_lex_string const*, st_mysql_const_lex_string const*, unsigned int) /mariadb/10.3/sql/sql_table.cc:5635 … Address 0x7fa636c472d0 is located in stack of thread T27 at offset 1120 in frame #0 0x55ae165b24ab in rename_file_ext(char const*, char const*, char const*) /mariadb/10.3/sql/table.cc:4199   This frame has 2 object(s): [32, 544) 'from_b' (line 4200) [608, 1120) 'to_b' (line 4200) <== Memory access at offset 1120 overflows this variable The buffer length is only FN_REFLEN (512 bytes). The parameter to points to a string "./@0023…@0023/@0023…@0023" (508 bytes), and the { {ext=".frm"}} would overflow the 512-byte buffer by 1 byte. In the embedded server, the ./ could be replaced with a path to the data directory. If we neglect that, the following fixes the problem and also allows the RENAME to proceed: diff --git a/sql/table.cc b/sql/table.cc index 64fb3150a39..f528f12a507 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -4197,7 +4197,8 @@ void update_create_info_from_table(HA_CREATE_INFO *create_info, TABLE *table) int rename_file_ext(const char * from,const char * to,const char * ext) { - char from_b[FN_REFLEN],to_b[FN_REFLEN]; + /* Reserve space for ./databasename/tablename.frm + NUL byte */ + char from_b[2 + FN_REFLEN + 4 + 1], to_b[2 + FN_REFLEN + 4 + 1]; (void) strxmov(from_b,from,ext,NullS); (void) strxmov(to_b,to,ext,NullS); return mysql_file_rename(key_file_frm, from_b, to_b, MYF(0)); The second RENAME in the test has to be adjusted accordingly, because the first RENAME will be accepted: eval RENAME TABLE `$d255`.`$d245` TO `$d255`.`$d250`; eval RENAME TABLE `$d255`.`$d250` TO a;

Marko Mäkelä added a comment - 2022-08-30 09:53 Starting with 10.6, mysql_rename_table() will flag the error. The check was tightened in MDEV-23842 Atomic RENAME TABLE : @@ -4728,37 +4754,20 @@ mysql_rename_table(handlerton *base, const LEX_CSTRING * old_db, length= build_table_filename(to, sizeof(to) - 1, new_db->str, new_name->str, "", flags & FN_TO_IS_TMP); // Check if we hit FN_REFLEN bytes along with file extension. - if (length+reg_ext_length > FN_REFLEN) + if (length+reg_ext_length >= FN_REFLEN) { my_error(ER_IDENT_CAUSES_TOO_LONG_PATH, MYF(0), (int) sizeof(to)-1, to); DBUG_RETURN(TRUE); } … I will try to revert that and test it with AddressSanitizer, to prevent a functionality regression.