Skip to content

Commit 96d7294

Browse files
montywimontywi
committed
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()
1 parent dfb41fd commit 96d7294

File tree

8 files changed

+51
-30
lines changed

8 files changed

+51
-30
lines changed

‎mysql-test/main/myisampack.result

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
DROP TABLE IF EXISTS t1,t2,t3;
22
CREATE TABLE t1(c1 DOUBLE, c2 DOUBLE, c3 DOUBLE, c4 DOUBLE, c5 DOUBLE,
3-
c6 DOUBLE, c7 DOUBLE, c8 DOUBLE, c9 DOUBLE, a INT PRIMARY KEY);
3+
c6 DOUBLE, c7 DOUBLE, c8 DOUBLE, c9 DOUBLE, a INT PRIMARY KEY) checksum=1;
44
INSERT INTO t1 VALUES
55
(-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),
66
(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),

‎mysql-test/main/myisampack.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ DROP TABLE IF EXISTS t1,t2,t3;
55
# BUG#31277 - myisamchk --unpack corrupts a table
66
#
77
CREATE TABLE t1(c1 DOUBLE, c2 DOUBLE, c3 DOUBLE, c4 DOUBLE, c5 DOUBLE,
8-
c6 DOUBLE, c7 DOUBLE, c8 DOUBLE, c9 DOUBLE, a INT PRIMARY KEY);
8+
c6 DOUBLE, c7 DOUBLE, c8 DOUBLE, c9 DOUBLE, a INT PRIMARY KEY) checksum=1;
99
INSERT INTO t1 VALUES
1010
(-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),
1111
(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),

‎storage/maria/aria_chk.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1773,21 +1773,26 @@ static void descript(HA_CHECK *param, register MARIA_HA *info, char *name)
17731773
type=share->columndef[field].base_type;
17741774
else
17751775
type=(enum en_fieldtype) share->columndef[field].type;
1776-
end=strmov(buff,field_pack[type]);
1776+
end= strmov(buff, field_pack[type]);
1777+
if (end != buff)
1778+
{
1779+
*(end++)=',';
1780+
*(end++)=' ';
1781+
}
17771782
if (share->options & HA_OPTION_COMPRESS_RECORD)
17781783
{
17791784
if (share->columndef[field].pack_type & PACK_TYPE_SELECTED)
1780-
end=strmov(end,", not_always");
1785+
end=strmov(end,"not_always, ");
17811786
if (share->columndef[field].pack_type & PACK_TYPE_SPACE_FIELDS)
1782-
end=strmov(end,", no empty");
1787+
end=strmov(end,"no empty, ");
17831788
if (share->columndef[field].pack_type & PACK_TYPE_ZERO_FILL)
17841789
{
1785-
sprintf(end,", zerofill(%d)",share->columndef[field].space_length_bits);
1790+
sprintf(end,"zerofill(%d), ",share->columndef[field].space_length_bits);
17861791
end=strend(end);
17871792
}
17881793
}
1789-
if (buff[0] == ',')
1790-
strmov(buff,buff+2);
1794+
if (end != buff)
1795+
end[-2]= 0;
17911796
int10_to_str((long) share->columndef[field].length,length,10);
17921797
null_bit[0]=null_pos[0]=0;
17931798
if (share->columndef[field].null_bit)

‎storage/maria/ma_check.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1545,6 +1545,7 @@ static int check_compressed_record(HA_CHECK *param, MARIA_HA *info, int extend,
15451545
my_errno, llstr(block_info.filepos, llbuff));
15461546
DBUG_RETURN(1);
15471547
}
1548+
info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
15481549
if (_ma_pack_rec_unpack(info, &info->bit_buff, record,
15491550
info->rec_buff, block_info.rec_len))
15501551
{
@@ -5327,10 +5328,7 @@ static int sort_get_next_record(MARIA_SORT_PARAM *sort_param)
53275328
llstr(sort_param->pos,llbuff));
53285329
continue;
53295330
}
5330-
#ifdef HAVE_valgrind
5331-
bzero(sort_param->rec_buff + block_info.rec_len,
5332-
share->base.extra_rec_buff_size);
5333-
#endif
5331+
sort_param->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
53345332
if (_ma_pack_rec_unpack(info, &sort_param->bit_buff, sort_param->record,
53355333
sort_param->rec_buff, block_info.rec_len))
53365334
{

‎storage/maria/ma_packrec.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,8 @@ int _ma_read_pack_record(MARIA_HA *info, uchar *buf, MARIA_RECORD_POS filepos)
757757
block_info.rec_len - block_info.offset, MYF(MY_NABP)))
758758
goto panic;
759759
info->update|= HA_STATE_AKTIV;
760+
761+
info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
760762
DBUG_RETURN(_ma_pack_rec_unpack(info,&info->bit_buff, buf,
761763
info->rec_buff, block_info.rec_len));
762764
panic:
@@ -1397,8 +1399,9 @@ int _ma_read_rnd_pack_record(MARIA_HA *info,
13971399
info->cur_row.nextpos= block_info.filepos+block_info.rec_len;
13981400
info->update|= HA_STATE_AKTIV | HA_STATE_KEY_CHANGED;
13991401

1400-
DBUG_RETURN (_ma_pack_rec_unpack(info, &info->bit_buff, buf,
1401-
info->rec_buff, block_info.rec_len));
1402+
info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
1403+
DBUG_RETURN(_ma_pack_rec_unpack(info, &info->bit_buff, buf,
1404+
info->rec_buff, block_info.rec_len));
14021405
err:
14031406
DBUG_RETURN(my_errno);
14041407
}

‎storage/myisam/mi_check.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,6 +1175,7 @@ int chk_data_link(HA_CHECK *param, MI_INFO *info, my_bool extend)
11751175
if (_mi_read_cache(&param->read_cache,(uchar*) info->rec_buff,
11761176
block_info.filepos, block_info.rec_len, READING_NEXT))
11771177
goto err;
1178+
info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
11781179
if (_mi_pack_rec_unpack(info, &info->bit_buff, record,
11791180
info->rec_buff, block_info.rec_len))
11801181
{
@@ -3632,6 +3633,7 @@ static int sort_get_next_record(MI_SORT_PARAM *sort_param)
36323633
llstr(sort_param->pos,llbuff));
36333634
continue;
36343635
}
3636+
sort_param->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
36353637
if (_mi_pack_rec_unpack(info, &sort_param->bit_buff, sort_param->record,
36363638
sort_param->rec_buff, block_info.rec_len))
36373639
{

‎storage/myisam/mi_packrec.c

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,8 @@ int _mi_read_pack_record(MI_INFO *info, my_off_t filepos, uchar *buf)
725725
block_info.rec_len - block_info.offset, MYF(MY_NABP)))
726726
goto panic;
727727
info->update|= HA_STATE_AKTIV;
728+
729+
info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
728730
DBUG_RETURN(_mi_pack_rec_unpack(info, &info->bit_buff, buf,
729731
info->rec_buff, block_info.rec_len));
730732
panic:
@@ -1352,8 +1354,9 @@ int _mi_read_rnd_pack_record(MI_INFO *info, uchar *buf,
13521354
info->nextpos=block_info.filepos+block_info.rec_len;
13531355
info->update|= HA_STATE_AKTIV | HA_STATE_KEY_CHANGED;
13541356

1355-
DBUG_RETURN (_mi_pack_rec_unpack(info, &info->bit_buff, buf,
1356-
info->rec_buff, block_info.rec_len));
1357+
info->rec_buff[block_info.rec_len]= 0; /* Keep valgrind happy */
1358+
DBUG_RETURN(_mi_pack_rec_unpack(info, &info->bit_buff, buf,
1359+
info->rec_buff, block_info.rec_len));
13571360
err:
13581361
DBUG_RETURN(my_errno);
13591362
}
@@ -1372,8 +1375,8 @@ uint _mi_pack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff,
13721375
{
13731376
ref_length=myisam->s->pack.ref_length;
13741377
/*
1375-
We can't use mysql_file_pread() here because mi_read_rnd_pack_record assumes
1376-
position is ok
1378+
We can't use mysql_file_pread() here because mi_read_rnd_pack_record
1379+
assumes position is ok
13771380
*/
13781381
mysql_file_seek(file, filepos, MY_SEEK_SET, MYF(0));
13791382
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,
14081411
}
14091412

14101413

1411-
/* rutines for bit buffer */
1412-
/* Note buffer must be 6 byte bigger than longest row */
1414+
/*
1415+
Rutines for bit buffer
1416+
Note: buffer must be 6 byte bigger than longest row
1417+
*/
14131418

14141419
static void init_bit_buffer(MI_BIT_BUFF *bit_buff, uchar *buffer, uint length)
14151420
{
14161421
bit_buff->pos=buffer;
14171422
bit_buff->end=buffer+length;
14181423
bit_buff->bits=bit_buff->error=0;
1419-
bit_buff->current_byte=0; /* Avoid purify errors */
1424+
bit_buff->current_byte=0; /* Avoid valgrind errors */
14201425
}
14211426

14221427
static uint fill_and_get_bits(MI_BIT_BUFF *bit_buff, uint count)
@@ -1562,9 +1567,11 @@ void _mi_unmap_file(MI_INFO *info)
15621567
}
15631568

15641569

1565-
static uchar *_mi_mempack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff,
1566-
MI_BLOCK_INFO *info, uchar **rec_buff_p,
1567-
uchar *header)
1570+
static uchar *_mi_mempack_get_block_info(MI_INFO *myisam,
1571+
MI_BIT_BUFF *bit_buff,
1572+
MI_BLOCK_INFO *info,
1573+
uchar **rec_buff_p,
1574+
uchar *header)
15681575
{
15691576
header+= read_pack_length((uint) myisam->s->pack.version, header,
15701577
&info->rec_len);
@@ -1573,7 +1580,7 @@ static uchar *_mi_mempack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff,
15731580
header+= read_pack_length((uint) myisam->s->pack.version, header,
15741581
&info->blob_len);
15751582
/* mi_alloc_rec_buff sets my_errno on error */
1576-
if (!(mi_alloc_rec_buff(myisam, info->blob_len,
1583+
if (!(mi_alloc_rec_buff(myisam, info->blob_len ,
15771584
rec_buff_p)))
15781585
return 0; /* not enough memory */
15791586
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)
15981605
(uchar*) share->file_map+
15991606
filepos)))
16001607
DBUG_RETURN(-1);
1608+
/* No need to end-zero pos here for valgrind as data is memory mapped */
16011609
DBUG_RETURN(_mi_pack_rec_unpack(info, &info->bit_buff, buf,
16021610
pos, block_info.rec_len));
16031611
}

‎storage/myisam/myisamchk.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,20 +1427,25 @@ static void descript(HA_CHECK *param, register MI_INFO *info, char * name)
14271427
else
14281428
type=(enum en_fieldtype) share->rec[field].type;
14291429
end=strmov(buff,field_pack[type]);
1430+
if (end != buff)
1431+
{
1432+
*(end++)=',';
1433+
*(end++)=' ';
1434+
}
14301435
if (share->options & HA_OPTION_COMPRESS_RECORD)
14311436
{
14321437
if (share->rec[field].pack_type & PACK_TYPE_SELECTED)
1433-
end=strmov(end,", not_always");
1438+
end=strmov(end,"not_always, ");
14341439
if (share->rec[field].pack_type & PACK_TYPE_SPACE_FIELDS)
1435-
end=strmov(end,", no empty");
1440+
end=strmov(end,"no empty, ");
14361441
if (share->rec[field].pack_type & PACK_TYPE_ZERO_FILL)
14371442
{
1438-
sprintf(end,", zerofill(%d)",share->rec[field].space_length_bits);
1443+
sprintf(end,"zerofill(%d), ",share->rec[field].space_length_bits);
14391444
end=strend(end);
14401445
}
14411446
}
1442-
if (buff[0] == ',')
1443-
strmov(buff,buff+2);
1447+
if (end != buff)
1448+
end[-2]= 0; /* Remove ", " */
14441449
int10_to_str((long) share->rec[field].length,length,10);
14451450
null_bit[0]=null_pos[0]=0;
14461451
if (share->rec[field].null_bit)

0 commit comments

Comments
 (0)