Commit 96d72945 authored by Monty's avatar Monty

Fixed access of undefined memory for compressed MyISAM and Aria tables

MDEV-22689 MSAN use-of-uninitialized-value in decode_bytes()

This was not a user visible issue as the huffman code lookup tables would
automatically ignore any of the unitialized bits

Fixed by adding a end-zero byte to the bit-stream buffer.

Other things:
- Fixed a (for this case) wrong assert in strmov() for myisamchk
  and aria_chk by removing the strmov()
parent dfb41fdd
DROP TABLE IF EXISTS t1,t2,t3; DROP TABLE IF EXISTS t1,t2,t3;
CREATE TABLE t1(c1 DOUBLE, c2 DOUBLE, c3 DOUBLE, c4 DOUBLE, c5 DOUBLE, CREATE TABLE t1(c1 DOUBLE, c2 DOUBLE, c3 DOUBLE, c4 DOUBLE, c5 DOUBLE,
c6 DOUBLE, c7 DOUBLE, c8 DOUBLE, c9 DOUBLE, a INT PRIMARY KEY); c6 DOUBLE, c7 DOUBLE, c8 DOUBLE, c9 DOUBLE, a INT PRIMARY KEY) checksum=1;
INSERT INTO t1 VALUES INSERT INTO t1 VALUES
(-3.31168791059336e-06,-3.19054655887874e-06,-1.06528081684847e-05,-1.227278240089e-06,-1.66718069164799e-06,-2.59038972510885e-06,-2.83145227805303e-06,-4.09678491270648e-07,-2.22610091291797e-06,6), (-3.31168791059336e-06,-3.19054655887874e-06,-1.06528081684847e-05,-1.227278240089e-06,-1.66718069164799e-06,-2.59038972510885e-06,-2.83145227805303e-06,-4.09678491270648e-07,-2.22610091291797e-06,6),
(0.0030743000272545,2.53222044316438e-05,2.78674650061845e-05,1.95914465544536e-05,1.7347572525984e-05,1.87513810069614e-05,1.69882826885005e-05,2.44449336987598e-05,1.89914629921774e-05,9), (0.0030743000272545,2.53222044316438e-05,2.78674650061845e-05,1.95914465544536e-05,1.7347572525984e-05,1.87513810069614e-05,1.69882826885005e-05,2.44449336987598e-05,1.89914629921774e-05,9),
......
...@@ -5,7 +5,7 @@ DROP TABLE IF EXISTS t1,t2,t3; ...@@ -5,7 +5,7 @@ DROP TABLE IF EXISTS t1,t2,t3;
# BUG#31277 - myisamchk --unpack corrupts a table # BUG#31277 - myisamchk --unpack corrupts a table
# #
CREATE TABLE t1(c1 DOUBLE, c2 DOUBLE, c3 DOUBLE, c4 DOUBLE, c5 DOUBLE, CREATE TABLE t1(c1 DOUBLE, c2 DOUBLE, c3 DOUBLE, c4 DOUBLE, c5 DOUBLE,
c6 DOUBLE, c7 DOUBLE, c8 DOUBLE, c9 DOUBLE, a INT PRIMARY KEY); c6 DOUBLE, c7 DOUBLE, c8 DOUBLE, c9 DOUBLE, a INT PRIMARY KEY) checksum=1;
INSERT INTO t1 VALUES INSERT INTO t1 VALUES
(-3.31168791059336e-06,-3.19054655887874e-06,-1.06528081684847e-05,-1.227278240089e-06,-1.66718069164799e-06,-2.59038972510885e-06,-2.83145227805303e-06,-4.09678491270648e-07,-2.22610091291797e-06,6), (-3.31168791059336e-06,-3.19054655887874e-06,-1.06528081684847e-05,-1.227278240089e-06,-1.66718069164799e-06,-2.59038972510885e-06,-2.83145227805303e-06,-4.09678491270648e-07,-2.22610091291797e-06,6),
(0.0030743000272545,2.53222044316438e-05,2.78674650061845e-05,1.95914465544536e-05,1.7347572525984e-05,1.87513810069614e-05,1.69882826885005e-05,2.44449336987598e-05,1.89914629921774e-05,9), (0.0030743000272545,2.53222044316438e-05,2.78674650061845e-05,1.95914465544536e-05,1.7347572525984e-05,1.87513810069614e-05,1.69882826885005e-05,2.44449336987598e-05,1.89914629921774e-05,9),
......
...@@ -1773,21 +1773,26 @@ static void descript(HA_CHECK *param, register MARIA_HA *info, char *name) ...@@ -1773,21 +1773,26 @@ static void descript(HA_CHECK *param, register MARIA_HA *info, char *name)
type=share->columndef[field].base_type; type=share->columndef[field].base_type;
else else
type=(enum en_fieldtype) share->columndef[field].type; type=(enum en_fieldtype) share->columndef[field].type;
end=strmov(buff,field_pack[type]); end= strmov(buff, field_pack[type]);
if (end != buff)
{
*(end++)=',';
*(end++)=' ';
}
if (share->options & HA_OPTION_COMPRESS_RECORD) if (share->options & HA_OPTION_COMPRESS_RECORD)
{ {
if (share->columndef[field].pack_type & PACK_TYPE_SELECTED) if (share->columndef[field].pack_type & PACK_TYPE_SELECTED)
end=strmov(end,", not_always"); end=strmov(end,"not_always, ");
if (share->columndef[field].pack_type & PACK_TYPE_SPACE_FIELDS) if (share->columndef[field].pack_type & PACK_TYPE_SPACE_FIELDS)
end=strmov(end,", no empty"); end=strmov(end,"no empty, ");
if (share->columndef[field].pack_type & PACK_TYPE_ZERO_FILL) if (share->columndef[field].pack_type & PACK_TYPE_ZERO_FILL)
{ {
sprintf(end,", zerofill(%d)",share->columndef[field].space_length_bits); sprintf(end,"zerofill(%d), ",share->columndef[field].space_length_bits);
end=strend(end); end=strend(end);
} }
} }
if (buff[0] == ',') if (end != buff)
strmov(buff,buff+2); end[-2]= 0;
int10_to_str((long) share->columndef[field].length,length,10); int10_to_str((long) share->columndef[field].length,length,10);
null_bit[0]=null_pos[0]=0; null_bit[0]=null_pos[0]=0;
if (share->columndef[field].null_bit) if (share->columndef[field].null_bit)
......
...@@ -1545,6 +1545,7 @@ static int check_compressed_record(HA_CHECK *param, MARIA_HA *info, int extend, ...@@ -1545,6 +1545,7 @@ static int check_compressed_record(HA_CHECK *param, MARIA_HA *info, int extend,
my_errno, llstr(block_info.filepos, llbuff)); my_errno, llstr(block_info.filepos, llbuff));
DBUG_RETURN(1); DBUG_RETURN(1);
} }
info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
if (_ma_pack_rec_unpack(info, &info->bit_buff, record, if (_ma_pack_rec_unpack(info, &info->bit_buff, record,
info->rec_buff, block_info.rec_len)) info->rec_buff, block_info.rec_len))
{ {
...@@ -5327,10 +5328,7 @@ static int sort_get_next_record(MARIA_SORT_PARAM *sort_param) ...@@ -5327,10 +5328,7 @@ static int sort_get_next_record(MARIA_SORT_PARAM *sort_param)
llstr(sort_param->pos,llbuff)); llstr(sort_param->pos,llbuff));
continue; continue;
} }
#ifdef HAVE_valgrind sort_param->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
bzero(sort_param->rec_buff + block_info.rec_len,
share->base.extra_rec_buff_size);
#endif
if (_ma_pack_rec_unpack(info, &sort_param->bit_buff, sort_param->record, if (_ma_pack_rec_unpack(info, &sort_param->bit_buff, sort_param->record,
sort_param->rec_buff, block_info.rec_len)) sort_param->rec_buff, block_info.rec_len))
{ {
......
...@@ -757,6 +757,8 @@ int _ma_read_pack_record(MARIA_HA *info, uchar *buf, MARIA_RECORD_POS filepos) ...@@ -757,6 +757,8 @@ int _ma_read_pack_record(MARIA_HA *info, uchar *buf, MARIA_RECORD_POS filepos)
block_info.rec_len - block_info.offset, MYF(MY_NABP))) block_info.rec_len - block_info.offset, MYF(MY_NABP)))
goto panic; goto panic;
info->update|= HA_STATE_AKTIV; info->update|= HA_STATE_AKTIV;
info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
DBUG_RETURN(_ma_pack_rec_unpack(info,&info->bit_buff, buf, DBUG_RETURN(_ma_pack_rec_unpack(info,&info->bit_buff, buf,
info->rec_buff, block_info.rec_len)); info->rec_buff, block_info.rec_len));
panic: panic:
...@@ -1397,7 +1399,8 @@ int _ma_read_rnd_pack_record(MARIA_HA *info, ...@@ -1397,7 +1399,8 @@ int _ma_read_rnd_pack_record(MARIA_HA *info,
info->cur_row.nextpos= block_info.filepos+block_info.rec_len; info->cur_row.nextpos= block_info.filepos+block_info.rec_len;
info->update|= HA_STATE_AKTIV | HA_STATE_KEY_CHANGED; info->update|= HA_STATE_AKTIV | HA_STATE_KEY_CHANGED;
DBUG_RETURN (_ma_pack_rec_unpack(info, &info->bit_buff, buf, info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
DBUG_RETURN(_ma_pack_rec_unpack(info, &info->bit_buff, buf,
info->rec_buff, block_info.rec_len)); info->rec_buff, block_info.rec_len));
err: err:
DBUG_RETURN(my_errno); DBUG_RETURN(my_errno);
......
...@@ -1175,6 +1175,7 @@ int chk_data_link(HA_CHECK *param, MI_INFO *info, my_bool extend) ...@@ -1175,6 +1175,7 @@ int chk_data_link(HA_CHECK *param, MI_INFO *info, my_bool extend)
if (_mi_read_cache(&param->read_cache,(uchar*) info->rec_buff, if (_mi_read_cache(&param->read_cache,(uchar*) info->rec_buff,
block_info.filepos, block_info.rec_len, READING_NEXT)) block_info.filepos, block_info.rec_len, READING_NEXT))
goto err; goto err;
info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
if (_mi_pack_rec_unpack(info, &info->bit_buff, record, if (_mi_pack_rec_unpack(info, &info->bit_buff, record,
info->rec_buff, block_info.rec_len)) info->rec_buff, block_info.rec_len))
{ {
...@@ -3632,6 +3633,7 @@ static int sort_get_next_record(MI_SORT_PARAM *sort_param) ...@@ -3632,6 +3633,7 @@ static int sort_get_next_record(MI_SORT_PARAM *sort_param)
llstr(sort_param->pos,llbuff)); llstr(sort_param->pos,llbuff));
continue; continue;
} }
sort_param->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
if (_mi_pack_rec_unpack(info, &sort_param->bit_buff, sort_param->record, if (_mi_pack_rec_unpack(info, &sort_param->bit_buff, sort_param->record,
sort_param->rec_buff, block_info.rec_len)) sort_param->rec_buff, block_info.rec_len))
{ {
......
...@@ -725,6 +725,8 @@ int _mi_read_pack_record(MI_INFO *info, my_off_t filepos, uchar *buf) ...@@ -725,6 +725,8 @@ int _mi_read_pack_record(MI_INFO *info, my_off_t filepos, uchar *buf)
block_info.rec_len - block_info.offset, MYF(MY_NABP))) block_info.rec_len - block_info.offset, MYF(MY_NABP)))
goto panic; goto panic;
info->update|= HA_STATE_AKTIV; info->update|= HA_STATE_AKTIV;
info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
DBUG_RETURN(_mi_pack_rec_unpack(info, &info->bit_buff, buf, DBUG_RETURN(_mi_pack_rec_unpack(info, &info->bit_buff, buf,
info->rec_buff, block_info.rec_len)); info->rec_buff, block_info.rec_len));
panic: panic:
...@@ -1352,7 +1354,8 @@ int _mi_read_rnd_pack_record(MI_INFO *info, uchar *buf, ...@@ -1352,7 +1354,8 @@ int _mi_read_rnd_pack_record(MI_INFO *info, uchar *buf,
info->nextpos=block_info.filepos+block_info.rec_len; info->nextpos=block_info.filepos+block_info.rec_len;
info->update|= HA_STATE_AKTIV | HA_STATE_KEY_CHANGED; info->update|= HA_STATE_AKTIV | HA_STATE_KEY_CHANGED;
DBUG_RETURN (_mi_pack_rec_unpack(info, &info->bit_buff, buf, info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
DBUG_RETURN(_mi_pack_rec_unpack(info, &info->bit_buff, buf,
info->rec_buff, block_info.rec_len)); info->rec_buff, block_info.rec_len));
err: err:
DBUG_RETURN(my_errno); DBUG_RETURN(my_errno);
...@@ -1372,8 +1375,8 @@ uint _mi_pack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff, ...@@ -1372,8 +1375,8 @@ uint _mi_pack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff,
{ {
ref_length=myisam->s->pack.ref_length; ref_length=myisam->s->pack.ref_length;
/* /*
We can't use mysql_file_pread() here because mi_read_rnd_pack_record assumes We can't use mysql_file_pread() here because mi_read_rnd_pack_record
position is ok assumes position is ok
*/ */
mysql_file_seek(file, filepos, MY_SEEK_SET, MYF(0)); mysql_file_seek(file, filepos, MY_SEEK_SET, MYF(0));
if (mysql_file_read(file, header, ref_length, MYF(MY_NABP))) if (mysql_file_read(file, header, ref_length, MYF(MY_NABP)))
...@@ -1408,15 +1411,17 @@ uint _mi_pack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff, ...@@ -1408,15 +1411,17 @@ uint _mi_pack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff,
} }
/* rutines for bit buffer */ /*
/* Note buffer must be 6 byte bigger than longest row */ Rutines for bit buffer
Note: buffer must be 6 byte bigger than longest row
*/
static void init_bit_buffer(MI_BIT_BUFF *bit_buff, uchar *buffer, uint length) static void init_bit_buffer(MI_BIT_BUFF *bit_buff, uchar *buffer, uint length)
{ {
bit_buff->pos=buffer; bit_buff->pos=buffer;
bit_buff->end=buffer+length; bit_buff->end=buffer+length;
bit_buff->bits=bit_buff->error=0; bit_buff->bits=bit_buff->error=0;
bit_buff->current_byte=0; /* Avoid purify errors */ bit_buff->current_byte=0; /* Avoid valgrind errors */
} }
static uint fill_and_get_bits(MI_BIT_BUFF *bit_buff, uint count) static uint fill_and_get_bits(MI_BIT_BUFF *bit_buff, uint count)
...@@ -1562,8 +1567,10 @@ void _mi_unmap_file(MI_INFO *info) ...@@ -1562,8 +1567,10 @@ void _mi_unmap_file(MI_INFO *info)
} }
static uchar *_mi_mempack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff, static uchar *_mi_mempack_get_block_info(MI_INFO *myisam,
MI_BLOCK_INFO *info, uchar **rec_buff_p, MI_BIT_BUFF *bit_buff,
MI_BLOCK_INFO *info,
uchar **rec_buff_p,
uchar *header) uchar *header)
{ {
header+= read_pack_length((uint) myisam->s->pack.version, header, header+= read_pack_length((uint) myisam->s->pack.version, header,
...@@ -1573,7 +1580,7 @@ static uchar *_mi_mempack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff, ...@@ -1573,7 +1580,7 @@ static uchar *_mi_mempack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff,
header+= read_pack_length((uint) myisam->s->pack.version, header, header+= read_pack_length((uint) myisam->s->pack.version, header,
&info->blob_len); &info->blob_len);
/* mi_alloc_rec_buff sets my_errno on error */ /* mi_alloc_rec_buff sets my_errno on error */
if (!(mi_alloc_rec_buff(myisam, info->blob_len, if (!(mi_alloc_rec_buff(myisam, info->blob_len ,
rec_buff_p))) rec_buff_p)))
return 0; /* not enough memory */ return 0; /* not enough memory */
bit_buff->blob_pos= (uchar*) *rec_buff_p; bit_buff->blob_pos= (uchar*) *rec_buff_p;
...@@ -1598,6 +1605,7 @@ static int _mi_read_mempack_record(MI_INFO *info, my_off_t filepos, uchar *buf) ...@@ -1598,6 +1605,7 @@ static int _mi_read_mempack_record(MI_INFO *info, my_off_t filepos, uchar *buf)
(uchar*) share->file_map+ (uchar*) share->file_map+
filepos))) filepos)))
DBUG_RETURN(-1); DBUG_RETURN(-1);
/* No need to end-zero pos here for valgrind as data is memory mapped */
DBUG_RETURN(_mi_pack_rec_unpack(info, &info->bit_buff, buf, DBUG_RETURN(_mi_pack_rec_unpack(info, &info->bit_buff, buf,
pos, block_info.rec_len)); pos, block_info.rec_len));
} }
......
...@@ -1427,20 +1427,25 @@ static void descript(HA_CHECK *param, register MI_INFO *info, char * name) ...@@ -1427,20 +1427,25 @@ static void descript(HA_CHECK *param, register MI_INFO *info, char * name)
else else
type=(enum en_fieldtype) share->rec[field].type; type=(enum en_fieldtype) share->rec[field].type;
end=strmov(buff,field_pack[type]); end=strmov(buff,field_pack[type]);
if (end != buff)
{
*(end++)=',';
*(end++)=' ';
}
if (share->options & HA_OPTION_COMPRESS_RECORD) if (share->options & HA_OPTION_COMPRESS_RECORD)
{ {
if (share->rec[field].pack_type & PACK_TYPE_SELECTED) if (share->rec[field].pack_type & PACK_TYPE_SELECTED)
end=strmov(end,", not_always"); end=strmov(end,"not_always, ");
if (share->rec[field].pack_type & PACK_TYPE_SPACE_FIELDS) if (share->rec[field].pack_type & PACK_TYPE_SPACE_FIELDS)
end=strmov(end,", no empty"); end=strmov(end,"no empty, ");
if (share->rec[field].pack_type & PACK_TYPE_ZERO_FILL) if (share->rec[field].pack_type & PACK_TYPE_ZERO_FILL)
{ {
sprintf(end,", zerofill(%d)",share->rec[field].space_length_bits); sprintf(end,"zerofill(%d), ",share->rec[field].space_length_bits);
end=strend(end); end=strend(end);
} }
} }
if (buff[0] == ',') if (end != buff)
strmov(buff,buff+2); end[-2]= 0; /* Remove ", " */
int10_to_str((long) share->rec[field].length,length,10); int10_to_str((long) share->rec[field].length,length,10);
null_bit[0]=null_pos[0]=0; null_bit[0]=null_pos[0]=0;
if (share->rec[field].null_bit) if (share->rec[field].null_bit)
......
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