mirror/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-01-15 19:42:28 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Added detection of memory overwrite with multi_malloc

Browse source 
This patch also fixes some bugs detected by valgrind after this
patch:

- Not enough copy_func elements was allocated by Create_tmp_table() which
  causes an memory overwrite in Create_tmp_table::add_fields()
  I added an ASSERT() to be able to detect this also without valgrind.
  The bug was that TMP_TABLE_PARAM::copy_fields was not correctly set
  when calling create_tmp_table().
- Aria::empty_bits is not allocated if there is no varchar/char/blob
  fields in the table.  Fixed code to take this into account.
  This cannot cause any issues as this is just a memory access
  into other Aria memory and the content of the memory would not be used.
- Aria::last_key_buff was not allocated big enough. This may have caused
  issues with rtrees and ma_extra(HA_EXTRA_REMEMBER_POS) as they
  would use the same memory area.
- Aria and MyISAM didn't take extended key parts into account, which
  caused problems when copying rec_per_key from engine to sql level.
- Mark asan builds with 'asan' in version strihng to detect these in
  not_valgrind_build.inc.
  This is needed to not have main.sp-no-valgrind fail with asan.
This commit is contained in:
Monty 2023-02-26 18:33:10 +02:00
parent 0de3be8cfd
commit 57c526ffb8
18 changed files with 136 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
BUILD/compile-pentium64-asan-max
View file

@ -17,7 +17,7 @@
path=`dirname $0`
. "$path/SETUP.sh"
extra_flags="$pentium64_cflags $debug_cflags -lasan -O -g -fsanitize=address -USAFEMALLOC -UFORCE_INIT_OF_VARS -Wno-uninitialized -Wno-maybe-uninitialized"
extra_flags="$pentium64_cflags $debug_cflags -lasan -O -g -fsanitize=address -USAFEMALLOC -UFORCE_INIT_OF_VARS -Wno-uninitialized -Wno-maybe-uninitialized -DMYSQL_SERVER_SUFFIX=-asan-max"
extra_configs="$pentium_configs $debug_configs $valgrind_configs $max_configs $disable_asan_plugins"
export LDFLAGS="-ldl"

4
mysql-test/include/not_valgrind_build.inc
View file

@ -1,4 +1,4 @@
if (`select version() like '%valgrind%'`)
if (`select version() like '%valgrind%' || version() like '%asan%'`)
{
skip Does not run with binaries built with valgrind;
skip Does not run with binaries built with valgrind or asan;
}

2
mysql-test/suite/sys_vars/t/innodb_log_checkpoint_now_basic.test
View file

@ -1,5 +1,7 @@
--source include/have_innodb.inc
--source include/have_debug.inc
# Valgrind builds may block on this one
--source include/not_valgrind.inc
SET @start_global_value = @@global.innodb_log_checkpoint_now;
SELECT @start_global_value;

19
mysys/mulalloc.c
View file

@ -17,6 +17,11 @@
#include "mysys_priv.h"
#include <stdarg.h>
#ifndef DBUG_OFF
/* Put a protected barrier after every element when using my_multi_malloc() */
#define ALLOC_BARRIER
#endif
/*
Malloc many pointers at the same time
Only ptr1 can be free'd, and doing this will free all
@ -45,6 +50,9 @@ void* my_multi_malloc(PSI_memory_key key, myf myFlags, ...)
{
length=va_arg(args,uint);
tot_length+=ALIGN_SIZE(length);
#ifdef ALLOC_BARRIER
tot_length+= ALIGN_SIZE(1);
#endif
}
va_end(args);
@ -58,6 +66,10 @@ void* my_multi_malloc(PSI_memory_key key, myf myFlags, ...)
*ptr=res;
length=va_arg(args,uint);
res+=ALIGN_SIZE(length);
#ifdef ALLOC_BARRIER
TRASH_FREE(res, ALIGN_SIZE(1));
res+= ALIGN_SIZE(1);
#endif
}
va_end(args);
DBUG_RETURN((void*) start);
@ -89,6 +101,9 @@ void *my_multi_malloc_large(PSI_memory_key key, myf myFlags, ...)
{
length=va_arg(args,ulonglong);
tot_length+=ALIGN_SIZE(length);
#ifdef ALLOC_BARRIER
tot_length+= ALIGN_SIZE(1);
#endif
}
va_end(args);
@ -102,6 +117,10 @@ void *my_multi_malloc_large(PSI_memory_key key, myf myFlags, ...)
*ptr=res;
length=va_arg(args,ulonglong);
res+=ALIGN_SIZE(length);
#ifdef ALLOC_BARRIER
TRASH_FREE(res, ALIGN_SIZE(1));
res+= ALIGN_SIZE(1);
#endif
}
va_end(args);
DBUG_RETURN((void*) start);

12
mysys/my_alloc.c
View file

@ -23,6 +23,11 @@
#undef EXTRA_DEBUG
#define EXTRA_DEBUG
#ifndef DBUG_OFF
/* Put a protected barrier after every element when using multi_alloc_root() */
#define ALLOC_BARRIER
#endif
/* data packed in MEM_ROOT -> min_malloc */
#define MALLOC_FLAG(A) ((A & 1) ? MY_THREAD_SPECIFIC : 0)
@ -311,6 +316,9 @@ void *multi_alloc_root(MEM_ROOT *root, ...)
{
length= va_arg(args, uint);
tot_length+= ALIGN_SIZE(length);
#ifdef ALLOC_BARRIER
tot_length+= ALIGN_SIZE(1);
#endif
}
va_end(args);
@ -324,6 +332,10 @@ void *multi_alloc_root(MEM_ROOT *root, ...)
*ptr= res;
length= va_arg(args, uint);
res+= ALIGN_SIZE(length);
#ifdef ALLOC_BARRIER
TRASH_FREE(res, ALIGN_SIZE(1));
res+= ALIGN_SIZE(1);
#endif
}
va_end(args);
DBUG_RETURN((void*) start);

5
sql/opt_subselect.cc
View file

@ -4174,6 +4174,7 @@ bool setup_sj_materialization_part1(JOIN_TAB *sjm_tab)
}
sjm->sjm_table_param.field_count= subq_select->item_list.elements;
sjm->sjm_table_param.func_count= sjm->sjm_table_param.field_count;
sjm->sjm_table_param.force_not_null_cols= TRUE;
if (!(sjm->table= create_tmp_table(thd, &sjm->sjm_table_param,
@ -5789,14 +5790,14 @@ TABLE *create_dummy_tmp_table(THD *thd)
DBUG_ENTER("create_dummy_tmp_table");
TABLE *table;
TMP_TABLE_PARAM sjm_table_param;
sjm_table_param.init();
sjm_table_param.field_count= 1;
List<Item> sjm_table_cols;
const LEX_CSTRING dummy_name= { STRING_WITH_LEN("dummy") };
Item *column_item= new (thd->mem_root) Item_int(thd, 1);
if (!column_item)
DBUG_RETURN(NULL);
sjm_table_param.init();
sjm_table_param.field_count= sjm_table_param.func_count= 1;
sjm_table_cols.push_back(column_item, thd->mem_root);
if (!(table= create_tmp_table(thd, &sjm_table_param,
sjm_table_cols, (ORDER*) 0,

15
sql/select_handler.cc
View file

@ -51,18 +51,19 @@ select_handler::~select_handler()
TABLE *select_handler::create_tmp_table(THD *thd, SELECT_LEX *select)
{
DBUG_ENTER("select_handler::create_tmp_table");
List<Item> types;
TMP_TABLE_PARAM tmp_table_param;
TABLE *table;
DBUG_ENTER("select_handler::create_tmp_table");
if (select->master_unit()->join_union_item_types(thd, types, 1))
DBUG_RETURN(NULL);
tmp_table_param.init();
tmp_table_param.field_count= types.elements;
TABLE *table= ::create_tmp_table(thd, &tmp_table_param, types,
(ORDER *) 0, false, 0,
TMP_TABLE_ALL_COLUMNS, 1,
&empty_clex_str, true, false);
tmp_table_param.field_count= tmp_table_param.func_count= types.elements;
table= ::create_tmp_table(thd, &tmp_table_param, types,
(ORDER *) 0, false, 0,
TMP_TABLE_ALL_COLUMNS, 1,
&empty_clex_str, true, false);
DBUG_RETURN(table);
}

1
sql/sql_class.cc
View file

@ -4310,6 +4310,7 @@ create_result_table(THD *thd_arg, List<Item> *column_types,
{
DBUG_ASSERT(table == 0);
tmp_table_param.field_count= column_types->elements;
tmp_table_param.func_count= tmp_table_param.field_count;
tmp_table_param.bit_fields_as_long= bit_fields_as_long;
if (! (table= create_tmp_table(thd_arg, &tmp_table_param, *column_types,

1
sql/sql_class.h
View file

@ -5986,6 +5986,7 @@ public:
@see opt_sum_query, count_field_types
*/
uint sum_func_count;
uint copy_func_count; // Allocated copy fields
uint hidden_field_count;
uint group_parts,group_length,group_null_parts;

2
sql/sql_expression_cache.cc
View file

@ -114,7 +114,7 @@ void Expression_cache_tmptable::init()
cache_table_param.init();
/* dependent items and result */
cache_table_param.field_count= items.elements;
cache_table_param.field_count= cache_table_param.func_count= items.elements;
/* postpone table creation to index description */
cache_table_param.skip_create_table= 1;

34
sql/sql_select.cc
View file

@ -3325,7 +3325,9 @@ bool JOIN::make_aggr_tables_info()
if (gbh)
{
if (!(pushdown_query= new (thd->mem_root) Pushdown_query(select_lex, gbh)))
TABLE *table;
if (!(pushdown_query= new (thd->mem_root) Pushdown_query(select_lex,
gbh)))
DBUG_RETURN(1);
/*
We must store rows in the tmp table if we need to do an ORDER BY
@ -3345,13 +3347,13 @@ bool JOIN::make_aggr_tables_info()
if (!(curr_tab->tmp_table_param= new TMP_TABLE_PARAM(tmp_table_param)))
DBUG_RETURN(1);
TABLE* table= create_tmp_table(thd, curr_tab->tmp_table_param,
all_fields,
NULL, query.distinct,
TRUE, select_options, HA_POS_ERROR,
&empty_clex_str, !need_tmp,
query.order_by || query.group_by);
if (!table)
curr_tab->tmp_table_param->func_count= all_fields.elements;
if (!(table= create_tmp_table(thd, curr_tab->tmp_table_param,
all_fields,
NULL, query.distinct,
TRUE, select_options, HA_POS_ERROR,
&empty_clex_str, !need_tmp,
query.order_by || query.group_by)))
DBUG_RETURN(1);
if (!(curr_tab->aggr= new (thd->mem_root) AGGR_OP(curr_tab)))
@ -18805,6 +18807,7 @@ TABLE *Create_tmp_table::start(THD *thd,
*/
if (param->precomputed_group_by)
copy_func_count+= param->sum_func_count;
param->copy_func_count= copy_func_count;
init_sql_alloc(key_memory_TABLE, &own_root, TABLE_ALLOC_BLOCK_SIZE, 0,
MYF(MY_THREAD_SPECIFIC));
@ -19010,8 +19013,9 @@ bool Create_tmp_table::add_fields(THD *thd,
We here distinguish between UNION and multi-table-updates by the fact
that in the later case group is set to the row pointer.
The test for item->marker == 4 is ensure we don't create a group-by
key over a bit field as heap tables can't handle that.
The test for item->marker == MARKER_NULL_KEY is ensure we
don't create a group-by key over a bit field as heap tables
can't handle that.
*/
DBUG_ASSERT(!param->schema_table);
Field *new_field=
@ -19076,6 +19080,7 @@ bool Create_tmp_table::add_fields(THD *thd,
new_field->flags|= FIELD_PART_OF_TMP_UNIQUE;
}
}
DBUG_ASSERT(fieldnr == m_field_count[other] + m_field_count[distinct]);
DBUG_ASSERT(m_blob_count == m_blobs_count[other] + m_blobs_count[distinct]);
share->fields= fieldnr;
@ -19083,7 +19088,9 @@ bool Create_tmp_table::add_fields(THD *thd,
table->field[fieldnr]= 0; // End marker
share->blob_field[m_blob_count]= 0; // End marker
copy_func[0]= 0; // End marker
DBUG_ASSERT((copy_func - param->items_to_copy) <= param->copy_func_count);
param->func_count= (uint) (copy_func - param->items_to_copy);
share->column_bitmap_size= bitmap_buffer_size(share->fields);
thd->mem_root= mem_root_save;
@ -26461,6 +26468,11 @@ bool JOIN::rollup_init()
Item **ref_array;
tmp_table_param.quick_group= 0; // Can't create groups in tmp table
/*
Each group can potentially be replaced with Item_func_rollup_const() which
needs a copy_func placeholder.
*/
tmp_table_param.func_count+= send_group_parts;
rollup.state= ROLLUP::STATE_INITED;
/*
@ -26485,7 +26497,6 @@ bool JOIN::rollup_init()
ref_array= (Item**) (rollup.ref_pointer_arrays+send_group_parts);
/*
Prepare space for field list for the different levels
These will be filled up in rollup_make_fields()
@ -26650,7 +26661,6 @@ bool JOIN::rollup_make_fields(List<Item> &fields_arg, List<Item> &sel_fields,
/* Point to first hidden field */
uint ref_array_ix= fields_arg.elements-1;
/* Remember where the sum functions ends for the previous level */
sum_funcs_end[pos+1]= *func;

28
sql/sql_show.cc
View file

@ -8216,28 +8216,34 @@ TABLE *create_schema_table(THD *thd, TABLE_LIST *table_list)
TABLE *table;
ST_SCHEMA_TABLE *schema_table= table_list->schema_table;
ST_FIELD_INFO *fields= schema_table->fields_info;
bool need_all_fieds= table_list->schema_table_reformed || // SHOW command
bool need_all_fields= table_list->schema_table_reformed || // SHOW command
thd->lex->only_view_structure(); // need table structure
bool keep_row_order;
TMP_TABLE_PARAM *tmp_table_param;
SELECT_LEX *select_lex;
my_bitmap_map *bitmaps;
DBUG_ENTER("create_schema_table");
for (; !fields->end_marker(); fields++)
field_count++;
TMP_TABLE_PARAM *tmp_table_param = new (thd->mem_root) TMP_TABLE_PARAM;
tmp_table_param = new (thd->mem_root) TMP_TABLE_PARAM;
tmp_table_param->init();
tmp_table_param->table_charset= system_charset_info;
tmp_table_param->field_count= field_count;
tmp_table_param->schema_table= 1;
SELECT_LEX *select_lex= table_list->select_lex;
bool keep_row_order= is_show_command(thd);
if (!(table= create_tmp_table_for_schema(thd, tmp_table_param, *schema_table,
(select_lex->options | thd->variables.option_bits | TMP_TABLE_ALL_COLUMNS),
table_list->alias, !need_all_fieds, keep_row_order)))
select_lex= table_list->select_lex;
keep_row_order= is_show_command(thd);
if (!(table=
create_tmp_table_for_schema(thd, tmp_table_param, *schema_table,
(select_lex->options |
thd->variables.option_bits |
TMP_TABLE_ALL_COLUMNS),
table_list->alias, !need_all_fields,
keep_row_order)))
DBUG_RETURN(0);
my_bitmap_map* bitmaps=
(my_bitmap_map*) thd->alloc(bitmap_buffer_size(field_count));
my_bitmap_init(&table->def_read_set, (my_bitmap_map*) bitmaps, field_count,
FALSE);
bitmaps= (my_bitmap_map*) thd->alloc(bitmap_buffer_size(field_count));
my_bitmap_init(&table->def_read_set, bitmaps, field_count, FALSE);
table->read_set= &table->def_read_set;
bitmap_clear_all(table->read_set);
table_list->schema_table_param= tmp_table_param;

4
sql/sql_union.cc
View file

@ -344,6 +344,7 @@ select_unit::create_result_table(THD *thd_arg, List<Item> *column_types,
DBUG_ASSERT(table == 0);
tmp_table_param.init();
tmp_table_param.field_count= column_types->elements;
tmp_table_param.func_count= tmp_table_param.field_count;
tmp_table_param.bit_fields_as_long= bit_fields_as_long;
tmp_table_param.hidden_field_count= hidden;
@ -384,7 +385,8 @@ select_union_recursive::create_result_table(THD *thd_arg,
return true;
incr_table_param.init();
incr_table_param.field_count= column_types->elements;
incr_table_param.field_count= incr_table_param.func_count=
column_types->elements;
incr_table_param.bit_fields_as_long= bit_fields_as_long;
if (! (incr_table= create_tmp_table(thd_arg, &incr_table_param, *column_types,
(ORDER*) 0, false, 1,

6
sql/temporary_tables.cc
View file

@ -912,9 +912,8 @@ bool THD::has_temporary_tables()
uint THD::create_tmp_table_def_key(char *key, const char *db,
const char *table_name)
{
DBUG_ENTER("THD::create_tmp_table_def_key");
uint key_length;
DBUG_ENTER("THD::create_tmp_table_def_key");
key_length= tdc_create_key(key, db, table_name);
int4store(key + key_length, variables.server_id);
@ -1163,11 +1162,10 @@ TABLE *THD::open_temporary_table(TMP_TABLE_SHARE *share,
*/
bool THD::find_and_use_tmp_table(const TABLE_LIST *tl, TABLE **out_table)
{
DBUG_ENTER("THD::find_and_use_tmp_table");
char key[MAX_DBKEY_LENGTH];
uint key_length;
bool result;
DBUG_ENTER("THD::find_and_use_tmp_table");
key_length= create_tmp_table_def_key(key, tl->get_db_name(),
tl->get_table_name());

18
storage/maria/ha_maria.cc
View file

@ -2630,12 +2630,22 @@ int ha_maria::info(uint flag)
share->db_record_offset= maria_info.record_offset;
if (share->key_parts)
{
ulong *to= table->key_info[0].rec_per_key, *end;
double *from= maria_info.rec_per_key;
for (end= to+ share->key_parts ; to < end ; to++, from++)
*to= (ulong) (*from + 0.5);
KEY *key, *key_end;
for (key= table->key_info, key_end= key + share->keys;
key < key_end ; key++)
{
ulong *to= key->rec_per_key;
/* Some temporary tables does not allocate rec_per_key */
if (to)
{
for (ulong *end= to+ key->user_defined_key_parts ;
to < end ;
to++, from++)
*to= (ulong) (*from + 0.5);
}
}
}
/*
Set data_file_name and index_file_name to point at the symlink value
if table is symlinked (Ie; Real name is not same as generated name)

25
storage/maria/ma_blockrec.c
View file

@ -2774,7 +2774,8 @@ static my_bool write_block_record(MARIA_HA *info,
const uchar *field_pos;
ulong length;
if ((record[column->null_pos] & column->null_bit) ||
(row->empty_bits[column->empty_pos] & column->empty_bit))
(column->empty_bit &&
(row->empty_bits[column->empty_pos] & column->empty_bit)))
continue;
field_pos= record + column->offset;
@ -4872,7 +4873,8 @@ int _ma_read_block_record2(MARIA_HA *info, uchar *record,
uchar *field_pos= record + column->offset;
/* First check if field is present in record */
if ((record[column->null_pos] & column->null_bit) ||
(cur_row->empty_bits[column->empty_pos] & column->empty_bit))
(column->empty_bit &&
(cur_row->empty_bits[column->empty_pos] & column->empty_bit)))
{
bfill(record + column->offset, column->fill_length,
type == FIELD_SKIP_ENDSPACE ? ' ' : 0);
@ -4951,8 +4953,9 @@ int _ma_read_block_record2(MARIA_HA *info, uchar *record,
{
uint size_length;
if ((record[blob_field->null_pos] & blob_field->null_bit) ||
(cur_row->empty_bits[blob_field->empty_pos] &
blob_field->empty_bit))
(blob_field->empty_bit &
(cur_row->empty_bits[blob_field->empty_pos] &
blob_field->empty_bit)))
continue;
size_length= blob_field->length - portable_sizeof_char_ptr;
blob_lengths+= _ma_calc_blob_length(size_length, length_data);
@ -5806,7 +5809,8 @@ static size_t fill_insert_undo_parts(MARIA_HA *info, const uchar *record,
const uchar *column_pos;
size_t column_length;
if ((record[column->null_pos] & column->null_bit) ||
cur_row->empty_bits[column->empty_pos] & column->empty_bit)
(column->empty_bit &&
cur_row->empty_bits[column->empty_pos] & column->empty_bit))
continue;
column_pos= record+ column->offset;
@ -5987,7 +5991,8 @@ static size_t fill_update_undo_parts(MARIA_HA *info, const uchar *oldrec,
*/
continue;
}
if (old_row->empty_bits[column->empty_pos] & column->empty_bit)
if (column->empty_bit &&
(old_row->empty_bits[column->empty_pos] & column->empty_bit))
{
if (new_row->empty_bits[column->empty_pos] & column->empty_bit)
continue; /* Both are empty; skip */
@ -6003,8 +6008,9 @@ static size_t fill_update_undo_parts(MARIA_HA *info, const uchar *oldrec,
log the original value
*/
new_column_is_empty= ((newrec[column->null_pos] & column->null_bit) ||
(new_row->empty_bits[column->empty_pos] &
column->empty_bit));
(column->empty_bit &&
(new_row->empty_bits[column->empty_pos] &
column->empty_bit)));
old_column_pos= oldrec + column->offset;
new_column_pos= newrec + column->offset;
@ -7177,7 +7183,8 @@ my_bool _ma_apply_undo_row_delete(MARIA_HA *info, LSN undo_lsn,
column++, null_field_lengths++)
{
if ((record[column->null_pos] & column->null_bit) ||
row.empty_bits[column->empty_pos] & column->empty_bit)
(column->empty_bit &&
row.empty_bits[column->empty_pos] & column->empty_bit))
{
if (column->type != FIELD_BLOB)
*null_field_lengths= 0;

2
storage/maria/ma_open.c
View file

@ -121,7 +121,7 @@ static MARIA_HA *maria_clone_internal(MARIA_SHARE *share,
&info.blobs,sizeof(MARIA_BLOB)*share->base.blobs,
&info.buff,(share->base.max_key_block_length*2+
share->base.max_key_length),
&info.lastkey_buff,share->base.max_key_length*2+1,
&info.lastkey_buff,share->base.max_key_length*3,
&info.first_mbr_key, share->base.max_key_length,
&info.maria_rtree_recursion_state,
share->have_rtree ? 1024 : 0,

14
storage/myisam/ha_myisam.cc
View file

@ -2143,9 +2143,17 @@ int ha_myisam::info(uint flag)
share->keys_for_keyread.intersect(share->keys_in_use);
share->db_record_offset= misam_info.record_offset;
if (share->key_parts)
memcpy((char*) table->key_info[0].rec_per_key,
(char*) misam_info.rec_per_key,
sizeof(table->key_info[0].rec_per_key[0])*share->key_parts);
{
ulong *from= misam_info.rec_per_key;
KEY *key, *key_end;
for (key= table->key_info, key_end= key + share->keys;
key < key_end ; key++)
{
memcpy(key->rec_per_key, from,
key->user_defined_key_parts * sizeof(*from));
from+= key->user_defined_key_parts;
}
}
if (table_share->tmp_table == NO_TMP_TABLE)
mysql_mutex_unlock(&table_share->LOCK_share);
}