|
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.
|
|
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;
|
|
|
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.
|