| Bug #30938 | BACKUP DATABASE crashes server if a table with a unloaded SE exists | ||
|---|---|---|---|
| Submitted: | 10 Sep 2007 20:16 | Modified: | 25 Feb 2008 15:50 |
| Reporter: | Jan Kneschke | Email Updates: | |
| Status: | Closed | Impact on me: | |
| Category: | MySQL Server: Backup | Severity: | S3 (Non-critical) |
| Version: | 5.2.5 | OS: | Any |
| Assigned to: | Rafal Somla | CPU Architecture: | Any |
[10 Sep 2007 20:18]
Jan Kneschke
FRM file for a paradox-SE
Attachment: weigon_test.frm (application/octet-stream, text), 8.78 KiB.
[10 Sep 2007 20:53]
Rafal Somla
REFINED PROBLEM DESCRIPTION The problem sits in the code collecting tables to be backed-up (Backup_info::add_db_items() in sql_backup.cc). The code here iterates over all rows in the I_S.tables table and selects the ones which match given database. But when reading a row from I_S.tables it tries to resolve table's handler from engine name, without checking whether the name is NULL or not. This is most probably where it crashes (when resolving empty engine name).
[10 Sep 2007 21:07]
Rafal Somla
SUGGESTED SOLUTION
An easy fix would be to change Backup_info::Table_ref constructor:
Backup_info::Table_ref::Table_ref(TABLE *t):
backup::Table_ref(m_db_name,m_name)
{
String engine_name;
plugin_ref plugin;
t->field[1]->val_str(&m_db_name);
t->field[2]->val_str(&m_name);
t->field[4]->val_str(&engine_name);
LEX_STRING name_lex;
name_lex.str= const_cast<char*>(engine_name.ptr());
name_lex.length= engine_name.length();
plugin= ::ha_resolve_by_name(::current_thd,&name_lex);
m_hton= plugin ? plugin_data(plugin, handlerton*) : 0;
}
We can check if engine_name is empty before calling ha_resolve_by_name(). If yes then we just set m_hton to NULL and return. The rest of the code should handle it correctly (but it should be checked).
However, ultimately, the code iterating over table info in I_S database should be changed as now it is very simplistic and we start hitting bugs here. For example, when collecting tables from a database 'foo' we would like to iterate over results of this query:
SELECT * FROM information_schema.tables WHERE db='foo' AND engine is not NULL
(approximately). The problem is that I need to learn how to do it from inside the server...
[10 Sep 2007 21:09]
MySQL Verification Team
Thank you for the bug report.
070910 18:06:46 [Note] Event Scheduler: Loaded 0 events
070910 18:06:46 [Note] /home/miguel/dbs/5.2/libexec/mysqld: ready for connections.
Version: '5.2.6-alpha-debug' socket: '/tmp/mysql.sock' port: 3306 Source distribution
[New Thread -1263359088 (LWP 26582)]
070910 18:07:23 [Note] Backup: Starting backup process
070910 18:07:23 [Note] Backup: Backing up selected databases
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1263359088 (LWP 26582)]
0x08734f05 in Table_ref (this=0xb4b28630, t=0x9ca9398) at sql_backup.cc:846
846 m_hton= plugin_data(::ha_resolve_by_name(::current_thd,&name_lex),
(gdb) bt full
#0 0x08734f05 in Table_ref (this=0xb4b28630, t=0x9ca9398) at sql_backup.cc:846
No locals.
#1 0x08735321 in backup::Backup_info::add_db_items (this=0xb4b2877c, db=@0x9d88668) at sql_backup.cc:793
type = {Ptr = 0x9cb2b58 "BASE TABLE", str_length = 10, Alloced_length = 0, alloced = false, str_charset = 0x897d620}
t = {<backup::Table_ref> = {m_db = {m_name = 0xb4b28638}, m_name = 0xb4b2864c}, m_db_name = {Ptr = 0x9cb29d6 "test", str_length = 4, Alloced_length = 0,
alloced = false, str_charset = 0x897d620}, m_name = {Ptr = 0x9cb2a97 "weigon_test", str_length = 11, Alloced_length = 0, alloced = false, str_charset = 0x897d620},
m_hton = 0x96520c8}
ha = (class handler *) 0x9c7dfa0
res = 0
[12 Sep 2007 18:37]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/34118 ChangeSet@1.2601, 2007-09-12 20:36:58+02:00, rafal@quant.(none) +8 -0 BUG#30938 (BACKUP DATABASE crashes server if a table with a unloaded SE exists). Backup of a database failed if it contained a table using invalid storage engine (e.g. plugin not loaded or unknown engine). This was because code collecting list of tables in a database didn't exclude tables without a valid storage engine and later kernel attempted to open such tables. This patch fixes this by filtering out tables for which 'engine' field in I_S.TABLES is NULL.
[14 Sep 2007 14:35]
Chuck Bell
Patch approved. Suggestion: We may want to consider making the error message accessible by the merlin code so that they can report it to the user in a nice(r) way.
[1 Oct 2007 14:14]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/34717 ChangeSet@1.2601, 2007-10-01 16:14:32+02:00, rafal@quant.(none) +9 -0 BUG#30938 (BACKUP DATABASE crashes server if a table with a unloaded SE exists). Backup of a database failed if it contained a table using invalid storage engine (e.g. plugin not loaded or unknown engine). This was because code collecting list of tables in a database (Backup_archive::add_db_items() in sql_backup.cc) didn't exclude tables without a valid storage engine and later kernel attempted to open such tables. This patch fixes this by filtering out tables for which 'engine' field in I_S.TABLES is NULL. Additionally, the way we access handlerton structure of table's storage engine has been changed. Before, we located SE plugin sing its name and then read handlerton pointer from the plugin: plugin= ::ha_resolve_by_name(::current_thd,&name_lex); m_hton= plugin ? plugin_data(plugin, handlerton*) : 0; Now we open a table and read handlerton pointer from opened TABLE structure. This is safer because it gives the open table code a chance to detect/deal with non-existent or non-functional storage engines. If a table was opened successfully we assume that its storage engine is fully functional. Tables are opened just before they are added to the archive inside Backup_archive::add_db_items(). Each table is closed after it has been added so that only one table is open at a time.
[2 Oct 2007 13:32]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/34757 ChangeSet@1.2602, 2007-10-02 15:30:45+02:00, rafal@quant.(none) +1 -0 BUG#30938 (BACKUP DATABASE crashes server if a table with a unloaded SE exists): In case of errors, e.g., when openning temporary tables, users could appreciate more precise information about the cause of a problem. However function open_temporary_table() used here doesn't provide such information. When it does, we should improve the way we report errors from this function. This patch adds a comment to remind us about that.
[2 Oct 2007 13:38]
Andrei Elkin
Approved after discussion the open_temporary_table() issue on skype.
[8 Oct 2007 7:29]
Rafal Somla
Pushed into 5.2-backup tree.
[25 Feb 2008 15:50]
Paul DuBois
Noted in 6.0.5 changelog. BACKUP DATABASE crashed if it attempted to backup tables for which the required storage engine was unloaded or otherwise unknown.
[14 Mar 2008 1:28]
Paul DuBois
Correction: No changelog entry needed; this bug did not appear in any released version.
[11 Jan 2010 20:45]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/96562 3051 Chuck Bell 2010-01-11 BUG#30938 (BACKUP DATABASE crashes server if a table with a unloaded SE exists). Backup of a database failed if it contained a table using invalid storage engine (e.g. plugin not loaded or unknown engine). This was because code collecting list of tables in a database (Backup_archive::add_db_items() in sql_backup.cc) didn't exclude tables without a valid storage engine and later kernel attempted to open such tables. This patch fixes this by filtering out tables for which 'engine' field in I_S.TABLES is NULL. Additionally, the way we access handlerton structure of table's storage engine has been changed. Before, we located SE plugin sing its name and then read handlerton pointer from the plugin: plugin= ::ha_resolve_by_name(::current_thd,&name_lex); m_hton= plugin ? plugin_data(plugin, handlerton*) : 0; Now we open a table and read handlerton pointer from opened TABLE structure. This is safer because it gives the open table code a chance to detect/deal with non-existent or non-functional storage engines. If a table was opened successfully we assume that its storage engine is fully functional. Tables are opened just before they are added to the archive inside Backup_archive::add_db_items(). Each table is closed after it has been added so that only one table is open at a time. original changeset: 2476.776.2
Description: I have a few tables in 5.2.5 from a earlier install which reference my own SE (paradox). If the SE plugin is not loaded, a BACKUP DATABASE ... will crash the server. How to repeat: Put the attached weigon_test.frm into the test-db and run: > BACKUP DATABASE empty TO "empty.backup"; 070910 21:27:28 [Warning] Backup: Skipping view vTestViewMetaData in database conj ==2360== ==2360== Invalid read of size 8 ==2360== at 0x8D4114: backup::Backup_info::Table_ref::Table_ref(st_table*) (mysql_priv.h:659) ==2360== by 0x8D3A5D: backup::Backup_info::add_db_items(backup::Archive_info::Db_item&) (sql_backup.cc:793) ==2360== by 0x8D33DA: backup::Backup_info::add_dbs(List<st_mysql_lex_string>&) (sql_backup.cc:598) ==2360== by 0x8D28F2: execute_backup_command(THD*, st_lex*) (sql_backup.cc:191) ==2360== by 0x5DC76B: mysql_execute_command(THD*) (sql_parse.cc:1873) ==2360== by 0x5DDEE0: mysql_parse(THD*, char const*, unsigned, char const**) (sql_parse.cc:5310) ==2360== by 0x5D7364: dispatch_command(enum_server_command, THD*, char*, unsigned) (sql_parse.cc:811) ==2360== by 0x5D6F6E: do_command(THD*) (sql_parse.cc:601) ==2360== by 0x5D6255: handle_one_connection (sql_connect.cc:1094) ==2360== by 0x4B2B192: start_thread (in /lib64/libpthread-2.4.so) ==2360== by 0x51A245C: clone (in /lib64/libc-2.4.so) ==2360== Address 0x28 is not stack'd, malloc'd or (recently) free'd (gdb) bt full #0 Table_ref (this=0x440848d0, t=0x14a2ce0) at ../mysql_priv.h:659 engine_name = {Ptr = 0x14a5479 "", str_length = 0, Alloced_length = 0, alloced = false, str_charset = 0xcac3e0} name_lex = {<st_mysql_lex_string> = {str = 0x14a5479 "", length = 0}, <No data fields>} #1 0x00000000008d3a5e in backup::Backup_info::add_db_items (this=0x44084ac0, db=@0x14ef8f0) at sql_backup.cc:793 type = {Ptr = 0x14a53b8 "BASE TABLE", str_length = 10, Alloced_length = 0, alloced = false, str_charset = 0xcac3e0} t = {<backup::Table_ref> = {m_db = {m_name = 0x440848e0}, m_name = 0x44084900}, m_db_name = {Ptr = 0x14a5236 "mysql", str_length = 5, Alloced_length = 0, alloced = false, str_charset = 0xcac3e0}, m_name = {Ptr = 0x14a52f7 "ndb_apply_status", str_length = 16, Alloced_length = 0, alloced = false, str_charset = 0xcac3e0}, m_hton = 0xd37a10} ha = (class handler *) 0x14a4460 res = 0 #2 0x00000000008d33db in backup::Backup_info::add_dbs (this=0x44084ac0, dbs=@0x14a5479) at sql_backup.cc:598 it = (struct Archive_info::Db_item *) 0x0 db = {<backup::Db_ref> = {m_name = 0x44084a08}, m_name = {Ptr = 0x149fd10 "empty", str_length = 5, Alloced_length = 5, alloced = false, str_charset = 0xcac3e0}} it = {<base_list_iterator> = {list = 0x144a4a8, el = 0x149fd28, prev = 0x144a4a8, current = 0x149fd28}, <No data fields>} s = (LEX_STRING *) 0x0 unknown_dbs = {Ptr = 0x0, str_length = 0, Alloced_length = 0, alloced = false, str_charset = 0xccd3c0} Suggested fix: Check if the engine is known, before accessing it.