From 5b940bdcfc34126301969a5dda448da890b4564d Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Tue, 8 Oct 2024 13:08:10 +0300 Subject: [PATCH] MDEV-25060 Freeing overrun buffer, various crashes, ASAN heap-buffer-overflow in _mi_put_key_in_record Rec buffer size depends on vreclength like this: length= MY_MAX(length, info->s->vreclength); The problem is rec buffer is allocated before vreclength is calculated. The fix reallocates rec buffer if vreclength changed. 1. Rec buffer allocated f0 mi_alloc_rec_buff (...) at ../src/storage/myisam/mi_open.c:738 f1 0x00005f4928244516 in mi_open (...) at ../src/storage/myisam/mi_open.c:671 f2 0x00005f4928210b98 in ha_myisam::open (...) at ../src/storage/myisam/ha_myisam.cc:847 f3 0x00005f49273aba41 in handler::ha_open (...) at ../src/sql/handler.cc:3105 f4 0x00005f4927995a65 in open_table_from_share (...) at ../src/sql/table.cc:4320 f5 0x00005f492769f084 in open_table (...) at ../src/sql/sql_base.cc:2024 f6 0x00005f49276a3ea9 in open_and_process_table (...) at ../src/sql/sql_base.cc:3819 f7 0x00005f49276a29b8 in open_tables (...) at ../src/sql/sql_base.cc:4303 f8 0x00005f49276a6f3f in open_and_lock_tables (...) at ../src/sql/sql_base.cc:5250 f9 0x00005f49275162de in open_and_lock_tables (...) at ../src/sql/sql_base.h:509 f10 0x00005f4927a30d7a in open_only_one_table (...) at ../src/sql/sql_admin.cc:412 f11 0x00005f4927a2c0c2 in mysql_admin_table (...) at ../src/sql/sql_admin.cc:603 f12 0x00005f4927a2fda8 in Sql_cmd_optimize_table::execute (...) at ../src/sql/sql_admin.cc:1517 f13 0x00005f49278102e3 in mysql_execute_command (...) at ../src/sql/sql_parse.cc:6180 f14 0x00005f49278012d7 in mysql_parse (...) at ../src/sql/sql_parse.cc:8236 2. vreclength calculated f0 ha_myisam::setup_vcols_for_repair (...) at ../src/storage/myisam/ha_myisam.cc:1002 f1 0x00005f49282138b4 in ha_myisam::optimize (...) at ../src/storage/myisam/ha_myisam.cc:1250 f2 0x00005f49273b4961 in handler::ha_optimize (...) at ../src/sql/handler.cc:4896 f3 0x00005f4927a2d254 in mysql_admin_table (...) at ../src/sql/sql_admin.cc:875 f4 0x00005f4927a2fda8 in Sql_cmd_optimize_table::execute (...) at ../src/sql/sql_admin.cc:1517 f5 0x00005f49278102e3 in mysql_execute_command (...) at ../src/sql/sql_parse.cc:6180 f6 0x00005f49278012d7 in mysql_parse (...) at ../src/sql/sql_parse.cc:8236 FYI backtrace was done with set print frame-info location set print frame-arguments presence set width 80 --- storage/myisam/ha_myisam.cc | 31 +++++++++++++++++++++++-------- storage/myisam/ha_myisam.h | 2 +- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index c5d13273b23..05f8238bc20 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -997,11 +997,11 @@ int ha_myisam::write_row(const uchar *buf) return mi_write(file,buf); } -void ha_myisam::setup_vcols_for_repair(HA_CHECK *param) +int ha_myisam::setup_vcols_for_repair(HA_CHECK *param) { DBUG_ASSERT(file->s->base.reclength <= file->s->vreclength); if (!table->vfield) - return; + return 0; if (file->s->base.reclength == file->s->vreclength) { @@ -1018,14 +1018,18 @@ void ha_myisam::setup_vcols_for_repair(HA_CHECK *param) } } if (!indexed_vcols) - return; + return 0; file->s->vreclength= new_vreclength; + if (!mi_alloc_rec_buff(file, -1, &file->rec_buff)) + return HA_ERR_OUT_OF_MEM; + bzero(file->rec_buff, mi_get_rec_buff_len(file, file->rec_buff)); } DBUG_ASSERT(file->s->base.reclength < file->s->vreclength || !table->s->stored_fields); param->init_fix_record= init_compute_vcols; param->fix_record= compute_vcols; table->use_all_columns(); + return 0; } int ha_myisam::check(THD* thd, HA_CHECK_OPT* check_opt) @@ -1061,7 +1065,11 @@ int ha_myisam::check(THD* thd, HA_CHECK_OPT* check_opt) (uint) (share->global_changed ? 1 : 0))))) return HA_ADMIN_ALREADY_DONE; - setup_vcols_for_repair(param); + if ((error = setup_vcols_for_repair(param))) + { + thd_proc_info(thd, old_proc_info); + return error; + } error = chk_status(param, file); // Not fatal error = chk_size(param, file); @@ -1157,7 +1165,8 @@ int ha_myisam::analyze(THD *thd, HA_CHECK_OPT* check_opt) if (!(share->state.changed & STATE_NOT_ANALYZED)) return HA_ADMIN_ALREADY_DONE; - setup_vcols_for_repair(param); + if ((error = setup_vcols_for_repair(param))) + return error; error = chk_key(param, file); if (!error) @@ -1192,7 +1201,8 @@ int ha_myisam::repair(THD* thd, HA_CHECK_OPT *check_opt) param->backup_time= check_opt->start_time; start_records=file->state->records; - setup_vcols_for_repair(param); + if ((error = setup_vcols_for_repair(param))) + return error; while ((error=repair(thd,*param,0)) && param->retry_repair) { @@ -1245,7 +1255,8 @@ int ha_myisam::optimize(THD* thd, HA_CHECK_OPT *check_opt) param->tmpfile_createflag= O_RDWR | O_TRUNC; param->sort_buffer_length= THDVAR(thd, sort_buffer_size); - setup_vcols_for_repair(param); + if ((error = setup_vcols_for_repair(param))) + return error; if ((error= repair(thd,*param,1)) && param->retry_repair) { @@ -1665,7 +1676,11 @@ int ha_myisam::enable_indexes(key_map map, bool persist) param->stats_method= (enum_handler_stats_method)THDVAR(thd, stats_method); param->tmpdir=&mysql_tmpdir_list; - setup_vcols_for_repair(param); + if ((error = setup_vcols_for_repair(param))) + { + thd_proc_info(thd, save_proc_info); + DBUG_RETURN(error); + } if ((error= (repair(thd,*param,0) != HA_ADMIN_OK)) && param->retry_repair) { diff --git a/storage/myisam/ha_myisam.h b/storage/myisam/ha_myisam.h index e72636e9e22..24790040cf8 100644 --- a/storage/myisam/ha_myisam.h +++ b/storage/myisam/ha_myisam.h @@ -48,7 +48,7 @@ class ha_myisam final : public handler char *data_file_name, *index_file_name; bool can_enable_indexes; int repair(THD *thd, HA_CHECK ¶m, bool optimize); - void setup_vcols_for_repair(HA_CHECK *param); + int setup_vcols_for_repair(HA_CHECK *param); public: ha_myisam(handlerton *hton, TABLE_SHARE *table_arg);