Commit 2b26729e authored by Alexey Kopytov's avatar Alexey Kopytov

Bug #45796: invalid memory reads and writes when altering merge

            and base tables 

myrg_attach_children() could reuse a buffer that was allocated 
previously based on a definition of a child table. The problem 
was that the child's definition might have been changed, so 
reusing the buffer could lead to crashes or valgrind errors 
under some circumstances. 
 
Fixed by changing myrg_attach_children() so that the 
rec_per_key_part buffer is reused only when the child table
have not changed, and reallocated otherwise (the old buffer is 
deallocated if necessary).


include/myisammrg.h:
  Added a pointer to need_compat_check as an argument to
  myrg_attach_children().
mysql-test/r/merge.result:
  Added a test case for bug #45796.
mysql-test/t/merge.test:
  Added a test case for bug #45796.
storage/myisammrg/ha_myisammrg.cc:
  Pass a pointer to need_compat_check to myrg_attach_children().
storage/myisammrg/myrg_open.c:
  Changed myrg_attach_children() so that the 
  rec_per_key_part buffer is reused only when the child table
  have not changed, and reallocated otherwise (the old buffer 
  is deallocated if necessary).
parent 05e498ea
...@@ -88,7 +88,8 @@ extern MYRG_INFO *myrg_parent_open(const char *parent_name, ...@@ -88,7 +88,8 @@ extern MYRG_INFO *myrg_parent_open(const char *parent_name,
void *callback_param); void *callback_param);
extern int myrg_attach_children(MYRG_INFO *m_info, int handle_locking, extern int myrg_attach_children(MYRG_INFO *m_info, int handle_locking,
MI_INFO *(*callback)(void*), MI_INFO *(*callback)(void*),
void *callback_param); void *callback_param,
my_bool *need_compat_check);
extern int myrg_detach_children(MYRG_INFO *m_info); extern int myrg_detach_children(MYRG_INFO *m_info);
extern int myrg_panic(enum ha_panic_function function); extern int myrg_panic(enum ha_panic_function function);
extern int myrg_rfirst(MYRG_INFO *file,uchar *buf,int inx); extern int myrg_rfirst(MYRG_INFO *file,uchar *buf,int inx);
......
...@@ -2127,4 +2127,18 @@ SELECT * FROM m1; ...@@ -2127,4 +2127,18 @@ SELECT * FROM m1;
ERROR HY000: Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist ERROR HY000: Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
DROP VIEW v1; DROP VIEW v1;
DROP TABLE m1, t1; DROP TABLE m1, t1;
#
# Bug #45796: invalid memory reads and writes when altering merge and
# base tables
#
CREATE TABLE t1(c1 INT) ENGINE=MyISAM;
CREATE TABLE m1(c1 INT) ENGINE=MERGE UNION=(t1);
ALTER TABLE m1 ADD INDEX idx_c1(c1);
SELECT * FROM m1;
ERROR HY000: Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist
ALTER TABLE t1 ADD INDEX idx_c1(c1);
SELECT * FROM m1;
c1
DROP TABLE m1;
DROP TABLE t1;
End of 5.1 tests End of 5.1 tests
...@@ -1535,4 +1535,24 @@ SELECT * FROM m1; ...@@ -1535,4 +1535,24 @@ SELECT * FROM m1;
DROP VIEW v1; DROP VIEW v1;
DROP TABLE m1, t1; DROP TABLE m1, t1;
--echo #
--echo # Bug #45796: invalid memory reads and writes when altering merge and
--echo # base tables
--echo #
CREATE TABLE t1(c1 INT) ENGINE=MyISAM;
CREATE TABLE m1(c1 INT) ENGINE=MERGE UNION=(t1);
ALTER TABLE m1 ADD INDEX idx_c1(c1);
# Open the MERGE table and allocate buffers based on children's definition.
--error ER_WRONG_MRG_TABLE
SELECT * FROM m1;
# Change the child table definition.
ALTER TABLE t1 ADD INDEX idx_c1(c1);
# Check that old buffers are not reused
SELECT * FROM m1;
DROP TABLE m1;
DROP TABLE t1;
--echo End of 5.1 tests --echo End of 5.1 tests
...@@ -545,7 +545,8 @@ int ha_myisammrg::attach_children(void) ...@@ -545,7 +545,8 @@ int ha_myisammrg::attach_children(void)
if (myrg_attach_children(this->file, this->test_if_locked | if (myrg_attach_children(this->file, this->test_if_locked |
current_thd->open_options, current_thd->open_options,
myisammrg_attach_children_callback, this)) myisammrg_attach_children_callback, this,
(my_bool *) &need_compat_check))
{ {
DBUG_PRINT("error", ("my_errno %d", my_errno)); DBUG_PRINT("error", ("my_errno %d", my_errno));
DBUG_RETURN(my_errno ? my_errno : -1); DBUG_RETURN(my_errno ? my_errno : -1);
......
...@@ -365,11 +365,14 @@ MYRG_INFO *myrg_parent_open(const char *parent_name, ...@@ -365,11 +365,14 @@ MYRG_INFO *myrg_parent_open(const char *parent_name,
The callback returns the MyISAM table handle of the child table. The callback returns the MyISAM table handle of the child table.
Check table definition match. Check table definition match.
@param[in] m_info MERGE parent table structure @param[in] m_info MERGE parent table structure
@param[in] handle_locking if contains HA_OPEN_FOR_REPAIR, warn about @param[in] handle_locking if contains HA_OPEN_FOR_REPAIR, warn about
incompatible child tables, but continue incompatible child tables, but continue
@param[in] callback function to call for each child table @param[in] callback function to call for each child table
@param[in] callback_param data pointer to give to the callback @param[in] callback_param data pointer to give to the callback
@param[in] need_compat_check pointer to ha_myisammrg::need_compat_check
(we need this one to decide if previously
allocated buffers can be reused).
@return status @return status
@retval 0 OK @retval 0 OK
...@@ -382,7 +385,7 @@ MYRG_INFO *myrg_parent_open(const char *parent_name, ...@@ -382,7 +385,7 @@ MYRG_INFO *myrg_parent_open(const char *parent_name,
int myrg_attach_children(MYRG_INFO *m_info, int handle_locking, int myrg_attach_children(MYRG_INFO *m_info, int handle_locking,
MI_INFO *(*callback)(void*), MI_INFO *(*callback)(void*),
void *callback_param) void *callback_param, my_bool *need_compat_check)
{ {
ulonglong file_offset; ulonglong file_offset;
MI_INFO *myisam; MI_INFO *myisam;
...@@ -423,6 +426,11 @@ int myrg_attach_children(MYRG_INFO *m_info, int handle_locking, ...@@ -423,6 +426,11 @@ int myrg_attach_children(MYRG_INFO *m_info, int handle_locking,
m_info->reclength= myisam->s->base.reclength; m_info->reclength= myisam->s->base.reclength;
min_keys= myisam->s->base.keys; min_keys= myisam->s->base.keys;
key_parts= myisam->s->base.key_parts; key_parts= myisam->s->base.key_parts;
if (*need_compat_check && m_info->rec_per_key_part)
{
my_free((char *) m_info->rec_per_key_part, MYF(0));
m_info->rec_per_key_part= NULL;
}
if (!m_info->rec_per_key_part) if (!m_info->rec_per_key_part)
{ {
if(!(m_info->rec_per_key_part= (ulong*) if(!(m_info->rec_per_key_part= (ulong*)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment