Commit 4692537f authored by Michael Widenius's avatar Michael Widenius

Fixed non critical buffer overflow bug in open_binary_frm() that could cause ASSERT

Added more printing of errors to myisamchk.

mysys/mf_iocache.c:
  Write error message if failed seek.
sql/table.cc:
  Fixed buffer overflow bug:
  - It's not enough to check for mysql_version to to detect partion indicator as the version may have been updated by mysql_upgrade.
storage/myisam/ha_myisam.cc:
  Don't log same error twice.
  Don't reset log_all_errors if it's set
storage/myisam/mi_check.c:
  Fixed bug that caused repair() to not report error if called twice (as when doing retry)
  More printing of errors.
storage/myisam/sort.c:
  Set my_errno in case of out of memory errors.
parent 1a51fe36
...@@ -1744,7 +1744,7 @@ int my_b_flush_io_cache(IO_CACHE *info, ...@@ -1744,7 +1744,7 @@ int my_b_flush_io_cache(IO_CACHE *info,
*/ */
if (!append_cache && info->seek_not_done) if (!append_cache && info->seek_not_done)
{ /* File touched, do seek */ { /* File touched, do seek */
if (my_seek(info->file,pos_in_file,MY_SEEK_SET,MYF(0)) == if (my_seek(info->file,pos_in_file,MY_SEEK_SET,MYF(info->myflags & MY_WME)) ==
MY_FILEPOS_ERROR) MY_FILEPOS_ERROR)
{ {
UNLOCK_APPEND_BUFFER; UNLOCK_APPEND_BUFFER;
......
...@@ -989,14 +989,13 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head, ...@@ -989,14 +989,13 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head,
#endif #endif
next_chunk+= 5 + partition_info_len; next_chunk+= 5 + partition_info_len;
} }
if (share->mysql_version >= 50110) if (share->mysql_version >= 50110 && next_chunk < buff_end)
{ {
/* New auto_partitioned indicator introduced in 5.1.11 */ /* New auto_partitioned indicator introduced in 5.1.11 */
#ifdef WITH_PARTITION_STORAGE_ENGINE #ifdef WITH_PARTITION_STORAGE_ENGINE
share->auto_partitioned= *next_chunk; share->auto_partitioned= *next_chunk;
#endif #endif
next_chunk++; next_chunk++;
DBUG_ASSERT(next_chunk <= buff_end);
} }
keyinfo= share->key_info; keyinfo= share->key_info;
for (i= 0; i < keys; i++, keyinfo++) for (i= 0; i < keys; i++, keyinfo++)
......
...@@ -99,7 +99,7 @@ static void mi_check_print_msg(HA_CHECK *param, const char* msg_type, ...@@ -99,7 +99,7 @@ static void mi_check_print_msg(HA_CHECK *param, const char* msg_type,
T_AUTO_REPAIR)) T_AUTO_REPAIR))
{ {
my_message(ER_NOT_KEYFILE, msgbuf, MYF(MY_WME)); my_message(ER_NOT_KEYFILE, msgbuf, MYF(MY_WME));
if (thd->variables.log_warnings > 2) if (thd->variables.log_warnings > 2 && ! thd->log_all_errors)
sql_print_error("%s.%s: %s", param->db_name, param->table_name, msgbuf); sql_print_error("%s.%s: %s", param->db_name, param->table_name, msgbuf);
return; return;
} }
...@@ -1673,7 +1673,7 @@ bool ha_myisam::check_and_repair(THD *thd) ...@@ -1673,7 +1673,7 @@ bool ha_myisam::check_and_repair(THD *thd)
bool save_log_all_errors; bool save_log_all_errors;
sql_print_warning("Recovering table: '%s'",table->s->path.str); sql_print_warning("Recovering table: '%s'",table->s->path.str);
save_log_all_errors= thd->log_all_errors; save_log_all_errors= thd->log_all_errors;
thd->log_all_errors= (thd->variables.log_warnings > 2); thd->log_all_errors|= (thd->variables.log_warnings > 2);
if (myisam_recover_options & HA_RECOVER_FULL_BACKUP) if (myisam_recover_options & HA_RECOVER_FULL_BACKUP)
{ {
char buff[MY_BACKUP_NAME_EXTRA_LENGTH+1]; char buff[MY_BACKUP_NAME_EXTRA_LENGTH+1];
......
...@@ -1539,6 +1539,9 @@ int mi_repair(HA_CHECK *param, register MI_INFO *info, ...@@ -1539,6 +1539,9 @@ int mi_repair(HA_CHECK *param, register MI_INFO *info,
got_error=1; got_error=1;
new_file= -1; new_file= -1;
sort_param.sort_info=&sort_info; sort_param.sort_info=&sort_info;
param->retry_repair= 0;
param->warning_printed= 0;
param->error_printed= 0;
if (!(param->testflag & T_SILENT)) if (!(param->testflag & T_SILENT))
{ {
...@@ -1683,7 +1686,7 @@ int mi_repair(HA_CHECK *param, register MI_INFO *info, ...@@ -1683,7 +1686,7 @@ int mi_repair(HA_CHECK *param, register MI_INFO *info,
if (rep_quick && del+sort_info.dupp != info->state->del) if (rep_quick && del+sort_info.dupp != info->state->del)
{ {
mi_check_print_error(param,"Couldn't fix table with quick recovery: Found wrong number of deleted records"); mi_check_print_error(param,"Couldn't fix table with quick recovery: Found wrong number of deleted records");
mi_check_print_error(param,"Run recovery again without -q"); mi_check_print_error(param,"Run recovery again without --quick");
got_error=1; got_error=1;
param->retry_repair=1; param->retry_repair=1;
param->testflag|=T_RETRY_WITHOUT_QUICK; param->testflag|=T_RETRY_WITHOUT_QUICK;
...@@ -1914,7 +1917,7 @@ int flush_blocks(HA_CHECK *param, KEY_CACHE *key_cache, File file, ...@@ -1914,7 +1917,7 @@ int flush_blocks(HA_CHECK *param, KEY_CACHE *key_cache, File file,
{ {
if (flush_key_blocks(key_cache, file, dirty_part_map, FLUSH_RELEASE)) if (flush_key_blocks(key_cache, file, dirty_part_map, FLUSH_RELEASE))
{ {
mi_check_print_error(param,"%d when trying to write bufferts",my_errno); mi_check_print_error(param,"%d when trying to write buffers",my_errno);
return(1); return(1);
} }
if (!param->using_global_keycache) if (!param->using_global_keycache)
...@@ -2228,7 +2231,7 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info, ...@@ -2228,7 +2231,7 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info,
MYISAM_SHARE *share=info->s; MYISAM_SHARE *share=info->s;
HA_KEYSEG *keyseg; HA_KEYSEG *keyseg;
ulong *rec_per_key_part; ulong *rec_per_key_part;
char llbuff[22]; char llbuff[22], llbuff2[22];
MI_SORT_INFO sort_info; MI_SORT_INFO sort_info;
ulonglong UNINIT_VAR(key_map); ulonglong UNINIT_VAR(key_map);
DBUG_ENTER("mi_repair_by_sort"); DBUG_ENTER("mi_repair_by_sort");
...@@ -2244,12 +2247,16 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info, ...@@ -2244,12 +2247,16 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info,
printf("Data records: %s\n", llstr(start_records,llbuff)); printf("Data records: %s\n", llstr(start_records,llbuff));
} }
param->testflag|=T_REP; /* for easy checking */ param->testflag|=T_REP; /* for easy checking */
param->retry_repair= 0;
param->warning_printed= 0;
param->error_printed= 0;
if (info->s->options & (HA_OPTION_CHECKSUM | HA_OPTION_COMPRESS_RECORD)) if (info->s->options & (HA_OPTION_CHECKSUM | HA_OPTION_COMPRESS_RECORD))
param->testflag|=T_CALC_CHECKSUM; param->testflag|=T_CALC_CHECKSUM;
bzero((char*)&sort_info,sizeof(sort_info)); bzero((char*)&sort_info,sizeof(sort_info));
bzero((char *)&sort_param, sizeof(sort_param)); bzero((char *)&sort_param, sizeof(sort_param));
if (!(sort_info.key_block= if (!(sort_info.key_block=
alloc_key_blocks(param, alloc_key_blocks(param,
(uint) param->sort_key_blocks, (uint) param->sort_key_blocks,
...@@ -2261,7 +2268,7 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info, ...@@ -2261,7 +2268,7 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info,
init_io_cache(&info->rec_cache,info->dfile, init_io_cache(&info->rec_cache,info->dfile,
(uint) param->write_buffer_length, (uint) param->write_buffer_length,
WRITE_CACHE,new_header_length,1, WRITE_CACHE,new_header_length,1,
MYF(MY_WME | MY_WAIT_IF_FULL) & param->myf_rw))) MYF((param->myf_rw & MY_WAIT_IF_FULL) | MY_WME))))
goto err; goto err;
sort_info.key_block_end=sort_info.key_block+param->sort_key_blocks; sort_info.key_block_end=sort_info.key_block+param->sort_key_blocks;
info->opt_flag|=WRITE_CACHE_USED; info->opt_flag|=WRITE_CACHE_USED;
...@@ -2433,7 +2440,10 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info, ...@@ -2433,7 +2440,10 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info,
(my_bool) (!(param->testflag & T_VERBOSE)), (my_bool) (!(param->testflag & T_VERBOSE)),
(uint) param->sort_buffer_length)) (uint) param->sort_buffer_length))
{ {
param->retry_repair=1; param->retry_repair= 1;
if (! param->error_printed)
mi_check_print_error(param, "Couldn't fix table with create_index_by_sort(). Error: %d",
my_errno);
goto err; goto err;
} }
/* No need to calculate checksum again. */ /* No need to calculate checksum again. */
...@@ -2462,7 +2472,10 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info, ...@@ -2462,7 +2472,10 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info,
/* Don't repair if we loosed more than one row */ /* Don't repair if we loosed more than one row */
if (info->state->records+1 < start_records) if (info->state->records+1 < start_records)
{ {
info->state->records=start_records; mi_check_print_error(param,
"Couldn't fix table as SAFE_REPAIR was requested and we would loose too many rows. %s -> %s",
llstr(start_records, llbuff), llstr(info->state->records, llbuff2));
info->state->records= start_records;
goto err; goto err;
} }
} }
...@@ -2492,7 +2505,7 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info, ...@@ -2492,7 +2505,7 @@ int mi_repair_by_sort(HA_CHECK *param, register MI_INFO *info,
if (rep_quick && del+sort_info.dupp != info->state->del) if (rep_quick && del+sort_info.dupp != info->state->del)
{ {
mi_check_print_error(param,"Couldn't fix table with quick recovery: Found wrong number of deleted records"); mi_check_print_error(param,"Couldn't fix table with quick recovery: Found wrong number of deleted records");
mi_check_print_error(param,"Run recovery again without -q"); mi_check_print_error(param,"Run recovery again without --quick");
got_error=1; got_error=1;
param->retry_repair=1; param->retry_repair=1;
param->testflag|=T_RETRY_WITHOUT_QUICK; param->testflag|=T_RETRY_WITHOUT_QUICK;
...@@ -2664,6 +2677,9 @@ int mi_repair_parallel(HA_CHECK *param, register MI_INFO *info, ...@@ -2664,6 +2677,9 @@ int mi_repair_parallel(HA_CHECK *param, register MI_INFO *info,
printf("Data records: %s\n", llstr(start_records,llbuff)); printf("Data records: %s\n", llstr(start_records,llbuff));
} }
param->testflag|=T_REP; /* for easy checking */ param->testflag|=T_REP; /* for easy checking */
param->retry_repair= 0;
param->warning_printed= 0;
param->error_printed= 0;
if (info->s->options & (HA_OPTION_CHECKSUM | HA_OPTION_COMPRESS_RECORD)) if (info->s->options & (HA_OPTION_CHECKSUM | HA_OPTION_COMPRESS_RECORD))
param->testflag|=T_CALC_CHECKSUM; param->testflag|=T_CALC_CHECKSUM;
......
...@@ -151,6 +151,7 @@ int _create_index_by_sort(MI_SORT_PARAM *info,my_bool no_messages, ...@@ -151,6 +151,7 @@ int _create_index_by_sort(MI_SORT_PARAM *info,my_bool no_messages,
{ {
mi_check_print_error(info->sort_info->param, mi_check_print_error(info->sort_info->param,
"myisam_sort_buffer_size is too small"); "myisam_sort_buffer_size is too small");
my_errno= ENOMEM;
goto err; goto err;
} }
} }
...@@ -175,7 +176,8 @@ int _create_index_by_sort(MI_SORT_PARAM *info,my_bool no_messages, ...@@ -175,7 +176,8 @@ int _create_index_by_sort(MI_SORT_PARAM *info,my_bool no_messages,
if (memavl < MIN_SORT_BUFFER) if (memavl < MIN_SORT_BUFFER)
{ {
mi_check_print_error(info->sort_info->param,"MyISAM sort buffer too small"); /* purecov: tested */ mi_check_print_error(info->sort_info->param,"MyISAM sort buffer too small"); /* purecov: tested */
goto err; /* purecov: tested */ my_errno= ENOMEM; /* purecov: tested */
goto err; /* purecov: tested */
} }
(*info->lock_in_memory)(info->sort_info->param);/* Everything is allocated */ (*info->lock_in_memory)(info->sort_info->param);/* Everything is allocated */
......
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