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()

8 files changed, 51 insertions, 30 deletions

diff --git a/mysql-test/main/myisampack.result b/mysql-test/main/myisampack.result
index 13321695360..61ad21a6c9f 100644
--- a/mysql-test/main/myisampack.result
+++ b/mysql-test/main/myisampack.result
@@ -1,6 +1,6 @@
 DROP TABLE IF EXISTS t1,t2,t3;
 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
 (-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),
diff --git a/mysql-test/main/myisampack.test b/mysql-test/main/myisampack.test
index 1f97a28e6fd..a63f0bcd484 100644
--- a/mysql-test/main/myisampack.test
+++ b/mysql-test/main/myisampack.test
@@ -5,7 +5,7 @@ DROP TABLE IF EXISTS t1,t2,t3;
 # BUG#31277 - myisamchk --unpack corrupts a table
 #
 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
 (-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),
diff --git a/storage/maria/aria_chk.c b/storage/maria/aria_chk.c
index 6f0ab98dcbc..f46fe7490d8 100644
--- a/storage/maria/aria_chk.c
+++ b/storage/maria/aria_chk.c
@@ -1773,21 +1773,26 @@ static void descript(HA_CHECK *param, register MARIA_HA *info, char *name)
 	type=share->columndef[field].base_type;
       else
 	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->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)
-	  end=strmov(end,", no empty");
+	  end=strmov(end,"no empty, ");
 	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);
 	}
       }
-      if (buff[0] == ',')
-	strmov(buff,buff+2);
+      if (end != buff)
+        end[-2]= 0;
       int10_to_str((long) share->columndef[field].length,length,10);
       null_bit[0]=null_pos[0]=0;
       if (share->columndef[field].null_bit)
diff --git a/storage/maria/ma_check.c b/storage/maria/ma_check.c
index 39337098e3a..553c04bb602 100644
--- a/storage/maria/ma_check.c
+++ b/storage/maria/ma_check.c
@@ -1545,6 +1545,7 @@ static int check_compressed_record(HA_CHECK *param, MARIA_HA *info, int extend,
                             my_errno, llstr(block_info.filepos, llbuff));
       DBUG_RETURN(1);
     }
+    info->rec_buff[block_info.rec_len]= 0;  /* Keep valgrind happy */
     if (_ma_pack_rec_unpack(info, &info->bit_buff, record,
                             info->rec_buff, block_info.rec_len))
     {
@@ -5327,10 +5328,7 @@ static int sort_get_next_record(MARIA_SORT_PARAM *sort_param)
 			      llstr(sort_param->pos,llbuff));
 	continue;
       }
-#ifdef HAVE_valgrind
-      bzero(sort_param->rec_buff + block_info.rec_len,
-            share->base.extra_rec_buff_size);
-#endif
+      sort_param->rec_buff[block_info.rec_len]= 0;  /* Keep valgrind happy */
       if (_ma_pack_rec_unpack(info, &sort_param->bit_buff, sort_param->record,
                               sort_param->rec_buff, block_info.rec_len))
       {
diff --git a/storage/maria/ma_packrec.c b/storage/maria/ma_packrec.c
index dd4ac28bbac..5eec3faa88f 100644
--- a/storage/maria/ma_packrec.c
+++ b/storage/maria/ma_packrec.c
@@ -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)))
     goto panic;
   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,
                                   info->rec_buff, block_info.rec_len));
 panic:
@@ -1397,8 +1399,9 @@ int _ma_read_rnd_pack_record(MARIA_HA *info,
   info->cur_row.nextpos= block_info.filepos+block_info.rec_len;
   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));
+  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));
  err:
   DBUG_RETURN(my_errno);
 }
diff --git a/storage/myisam/mi_check.c b/storage/myisam/mi_check.c
index b2605cf5d2f..b97c9656ab1 100644
--- a/storage/myisam/mi_check.c
+++ b/storage/myisam/mi_check.c
@@ -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,
 			block_info.filepos, block_info.rec_len, READING_NEXT))
 	goto err;
+      info->rec_buff[block_info.rec_len]= 0;  /* Keep valgrind happy */
       if (_mi_pack_rec_unpack(info, &info->bit_buff, record,
                               info->rec_buff, block_info.rec_len))
       {
@@ -3632,6 +3633,7 @@ static int sort_get_next_record(MI_SORT_PARAM *sort_param)
 			      llstr(sort_param->pos,llbuff));
 	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,
                               sort_param->rec_buff, block_info.rec_len))
       {
diff --git a/storage/myisam/mi_packrec.c b/storage/myisam/mi_packrec.c
index d5bfdac3008..ca8a8ef06c7 100644
--- a/storage/myisam/mi_packrec.c
+++ b/storage/myisam/mi_packrec.c
@@ -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)))
     goto panic;
   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,
                                   info->rec_buff, block_info.rec_len));
 panic:
@@ -1352,8 +1354,9 @@ int _mi_read_rnd_pack_record(MI_INFO *info, uchar *buf,
   info->nextpos=block_info.filepos+block_info.rec_len;
   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));
+  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));
  err:
   DBUG_RETURN(my_errno);
 }
@@ -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;
     /*
-      We can't use mysql_file_pread() here because mi_read_rnd_pack_record assumes
-      position is ok
+      We can't use mysql_file_pread() here because mi_read_rnd_pack_record
+      assumes position is ok
     */
     mysql_file_seek(file, filepos, MY_SEEK_SET, MYF(0));
     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,
 }
 
 
-	/* 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)
 {
   bit_buff->pos=buffer;
   bit_buff->end=buffer+length;
   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)
@@ -1562,9 +1567,11 @@ void _mi_unmap_file(MI_INFO *info)
 }
 
 
-static uchar *_mi_mempack_get_block_info(MI_INFO *myisam, MI_BIT_BUFF *bit_buff,
-                                         MI_BLOCK_INFO *info, uchar **rec_buff_p,
-					 uchar *header)
+static uchar *_mi_mempack_get_block_info(MI_INFO *myisam,
+                                         MI_BIT_BUFF *bit_buff,
+                                         MI_BLOCK_INFO *info,
+                                         uchar **rec_buff_p,
+                                         uchar *header)
 {
   header+= read_pack_length((uint) myisam->s->pack.version, header,
                             &info->rec_len);
@@ -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,
                               &info->blob_len);
     /* 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)))
       return 0;				/* not enough memory */
     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)
 						(uchar*) share->file_map+
 						filepos)))
     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,
                                   pos, block_info.rec_len));
 }
diff --git a/storage/myisam/myisamchk.c b/storage/myisam/myisamchk.c
index c4b274b3fe9..10d4987e1cb 100644
--- a/storage/myisam/myisamchk.c
+++ b/storage/myisam/myisamchk.c
@@ -1427,20 +1427,25 @@ static void descript(HA_CHECK *param, register MI_INFO *info, char * name)
       else
 	type=(enum en_fieldtype) share->rec[field].type;
       end=strmov(buff,field_pack[type]);
+      if (end != buff)
+      {
+        *(end++)=',';
+        *(end++)=' ';
+      }
       if (share->options & HA_OPTION_COMPRESS_RECORD)
       {
 	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)
-	  end=strmov(end,", no empty");
+	  end=strmov(end,"no empty, ");
 	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);
 	}
       }
-      if (buff[0] == ',')
-	strmov(buff,buff+2);
+      if (end != buff)
+        end[-2]= 0;                               /* Remove ", " */
       int10_to_str((long) share->rec[field].length,length,10);
       null_bit[0]=null_pos[0]=0;
       if (share->rec[field].null_bit)