BUG#31277 - myisamchk --unpack corrupts a table

With certain data sets (when compressed record length gets bigger than
uncompressed) myisamchk --unpack may corrupt data file.

Fixed that record length was wrongly restored from compressed table.


myisam/mi_check.c:
  With compressed tables compressed record length may be bigger than
  pack_reclength, thus we may allocate insufficient memory for record
  buffer.
  
  Let single function allocate record buffer, performing needed record
  length calculations.
  
  Still, it is not doable with parallel repair, as it allocates needed
  record buffers at once. For parellel repair added better record length
  calculation.
myisam/mi_open.c:
  When calculating record buffer size, take into account that compressed
  record length may be bigger than uncompressed.
myisam/mi_packrec.c:
  With certain data set share->max_pack_length (compressed record length)
  may be bigger than share->base.pack_reclength (packed record length).
  
  set_if_bigger(pack_reclength, max_pack_length) in this case causes
  myisamchk --unpack to write extra garbage, whereas pack_reclength
  remains the same in new index file. As a result we get unreadable
  table.
myisam/myisamchk.c:
  With compressed tables compressed record length may be bigger than
  pack_reclength, thus we may allocate insufficient memory for record
  buffer.
  
  Let single function allocate record buffer, performing needed record
  length calculations.
mysql-test/mysql-test-run.pl:
  Environment variables to execute myisamchk and myisampack.
mysql-test/r/myisampack.result:
  New BitKeeper file ``mysql-test/r/myisampack.result''
mysql-test/t/myisampack.test:
  New BitKeeper file ``mysql-test/t/myisampack.test''

4 files changed, 30 insertions, 24 deletions

diff --git a/myisam/mi_check.c b/myisam/mi_check.c
index ce8fb04874e..0b786ca5332 100644
--- a/myisam/mi_check.c
+++ b/myisam/mi_check.c
@@ -940,7 +940,7 @@ int chk_data_link(MI_CHECK *param, MI_INFO *info,int extend)
   ha_rows records,del_blocks;
   my_off_t used,empty,pos,splits,start_recpos,
 	   del_length,link_used,start_block;
-  byte	*record,*to;
+  byte	*record= 0, *to;
   char llbuff[22],llbuff2[22],llbuff3[22];
   ha_checksum intern_record_checksum;
   ha_checksum key_checksum[MI_MAX_POSSIBLE_KEY];
@@ -957,7 +957,7 @@ int chk_data_link(MI_CHECK *param, MI_INFO *info,int extend)
       puts("- check record links");
   }
 
-  if (!(record= (byte*) my_malloc(info->s->base.pack_reclength,MYF(0))))
+  if (!mi_alloc_rec_buff(info, -1, &record))
   {
     mi_check_print_error(param,"Not enough memory for record");
     DBUG_RETURN(-1);
@@ -1364,12 +1364,12 @@ int chk_data_link(MI_CHECK *param, MI_INFO *info,int extend)
     printf("Lost space:   %12s    Linkdata:     %10s\n",
 	   llstr(empty,llbuff),llstr(link_used,llbuff2));
   }
-  my_free((gptr) record,MYF(0));
+  my_free(mi_get_rec_buff_ptr(info, record), MYF(0));
   DBUG_RETURN (error);
  err:
   mi_check_print_error(param,"got error: %d when reading datafile at record: %s",my_errno, llstr(records,llbuff));
  err2:
-  my_free((gptr) record,MYF(0));
+  my_free(mi_get_rec_buff_ptr(info, record), MYF(0));
   param->testflag|=T_RETRY_WITHOUT_QUICK;
   DBUG_RETURN(1);
 } /* chk_data_link */
@@ -1428,8 +1428,7 @@ int mi_repair(MI_CHECK *param, register MI_INFO *info,
 		      MYF(MY_WME | MY_WAIT_IF_FULL)))
       goto err;
   info->opt_flag|=WRITE_CACHE_USED;
-  if (!(sort_param.record=(byte*) my_malloc((uint) share->base.pack_reclength,
-					   MYF(0))) ||
+  if (!mi_alloc_rec_buff(info, -1, &sort_param.record) ||
       !mi_alloc_rec_buff(info, -1, &sort_param.rec_buff))
   {
     mi_check_print_error(param, "Not enough memory for extra record");
@@ -1631,7 +1630,8 @@ err:
   }
   my_free(mi_get_rec_buff_ptr(info, sort_param.rec_buff),
                             MYF(MY_ALLOW_ZERO_PTR));
-  my_free(sort_param.record,MYF(MY_ALLOW_ZERO_PTR));
+  my_free(mi_get_rec_buff_ptr(info, sort_param.record),
+          MYF(MY_ALLOW_ZERO_PTR));
   my_free(sort_info.buff,MYF(MY_ALLOW_ZERO_PTR));
   VOID(end_io_cache(&param->read_cache));
   info->opt_flag&= ~(READ_CACHE_USED | WRITE_CACHE_USED);
@@ -2129,8 +2129,7 @@ int mi_repair_by_sort(MI_CHECK *param, register MI_INFO *info,
   info->opt_flag|=WRITE_CACHE_USED;
   info->rec_cache.file=info->dfile;		/* for sort_delete_record */
 
-  if (!(sort_param.record=(byte*) my_malloc((uint) share->base.pack_reclength,
-					   MYF(0))) ||
+  if (!mi_alloc_rec_buff(info, -1, &sort_param.record) ||
       !mi_alloc_rec_buff(info, -1, &sort_param.rec_buff))
   {
     mi_check_print_error(param, "Not enough memory for extra record");
@@ -2415,7 +2414,8 @@ err:
 
   my_free(mi_get_rec_buff_ptr(info, sort_param.rec_buff),
                             MYF(MY_ALLOW_ZERO_PTR));
-  my_free(sort_param.record,MYF(MY_ALLOW_ZERO_PTR));
+  my_free(mi_get_rec_buff_ptr(info, sort_param.record),
+          MYF(MY_ALLOW_ZERO_PTR));
   my_free((gptr) sort_info.key_block,MYF(MY_ALLOW_ZERO_PTR));
   my_free((gptr) sort_info.ft_buf, MYF(MY_ALLOW_ZERO_PTR));
   my_free(sort_info.buff,MYF(MY_ALLOW_ZERO_PTR));
@@ -2493,6 +2493,7 @@ int mi_repair_parallel(MI_CHECK *param, register MI_INFO *info,
   SORT_INFO sort_info;
   ulonglong key_map=share->state.key_map;
   pthread_attr_t thr_attr;
+  ulong max_pack_reclength;
   DBUG_ENTER("mi_repair_parallel");
 
   start_records=info->state->records;
@@ -2649,10 +2650,13 @@ int mi_repair_parallel(MI_CHECK *param, register MI_INFO *info,
 
   del=info->state->del;
   param->glob_crc=0;
-
+  /* for compressed tables */
+  max_pack_reclength= share->base.pack_reclength;
+  if (share->options & HA_OPTION_COMPRESS_RECORD)
+    set_if_bigger(max_pack_reclength, share->max_pack_length);
   if (!(sort_param=(MI_SORT_PARAM *)
         my_malloc((uint) share->base.keys *
-		  (sizeof(MI_SORT_PARAM) + share->base.pack_reclength),
+		  (sizeof(MI_SORT_PARAM) + max_pack_reclength),
 		  MYF(MY_ZEROFILL))))
   {
     mi_check_print_error(param,"Not enough memory for key!");
@@ -2704,7 +2708,7 @@ int mi_repair_parallel(MI_CHECK *param, register MI_INFO *info,
     sort_param[i].max_pos=sort_param[i].pos=share->pack.header_length;
 
     sort_param[i].record= (((char *)(sort_param+share->base.keys))+
-			   (share->base.pack_reclength * i));
+			   (max_pack_reclength * i));
     if (!mi_alloc_rec_buff(info, -1, &sort_param[i].rec_buff))
     {
       mi_check_print_error(param,"Not enough memory!");
@@ -4320,7 +4324,7 @@ err:
 void update_auto_increment_key(MI_CHECK *param, MI_INFO *info,
 			       my_bool repair_only)
 {
-  byte *record;
+  byte *record= 0;
   DBUG_ENTER("update_auto_increment_key");
 
   if (!info->s->base.auto_key ||
@@ -4340,8 +4344,7 @@ void update_auto_increment_key(MI_CHECK *param, MI_INFO *info,
     We have to use an allocated buffer instead of info->rec_buff as 
     _mi_put_key_in_record() may use info->rec_buff
   */
-  if (!(record= (byte*) my_malloc((uint) info->s->base.pack_reclength,
-				  MYF(0))))
+  if (!mi_alloc_rec_buff(info, -1, &record))
   {
     mi_check_print_error(param,"Not enough memory for extra record");
     DBUG_VOID_RETURN;
@@ -4353,7 +4356,7 @@ void update_auto_increment_key(MI_CHECK *param, MI_INFO *info,
     if (my_errno != HA_ERR_END_OF_FILE)
     {
       mi_extra(info,HA_EXTRA_NO_KEYREAD,0);
-      my_free((char*) record, MYF(0));
+      my_free(mi_get_rec_buff_ptr(info, record), MYF(0));
       mi_check_print_error(param,"%d when reading last record",my_errno);
       DBUG_VOID_RETURN;
     }
@@ -4369,7 +4372,7 @@ void update_auto_increment_key(MI_CHECK *param, MI_INFO *info,
     set_if_bigger(info->s->state.auto_increment,auto_increment);
   }
   mi_extra(info,HA_EXTRA_NO_KEYREAD,0);
-  my_free((char*) record, MYF(0));
+  my_free(mi_get_rec_buff_ptr(info, record), MYF(0));
   update_state_info(param, info, UPDATE_AUTO_INC);
   DBUG_VOID_RETURN;
 }
diff --git a/myisam/mi_open.c b/myisam/mi_open.c
index b007eb63e63..7543bdf2bf1 100644
--- a/myisam/mi_open.c
+++ b/myisam/mi_open.c
@@ -659,8 +659,11 @@ byte *mi_alloc_rec_buff(MI_INFO *info, ulong length, byte **buf)
     /* to simplify initial init of info->rec_buf in mi_open and mi_extra */
     if (length == (ulong) -1)
     {
-      length= max(info->s->base.pack_reclength,
-                  info->s->base.max_key_length);
+      if (info->s->options & HA_OPTION_COMPRESS_RECORD)
+        length= max(info->s->base.pack_reclength, info->s->max_pack_length);
+      else
+        length= info->s->base.pack_reclength;
+      length= max(length, info->s->base.max_key_length);
       /* Avoid unnecessary realloc */
       if (newptr && length == old_length)
 	return newptr;
diff --git a/myisam/mi_packrec.c b/myisam/mi_packrec.c
index 51b0222e876..e931cc770ed 100644
--- a/myisam/mi_packrec.c
+++ b/myisam/mi_packrec.c
@@ -164,7 +164,6 @@ my_bool _mi_read_pack_info(MI_INFO *info, pbool fix_keys)
   share->pack.header_length=	uint4korr(header+4);
   share->min_pack_length=(uint) uint4korr(header+8);
   share->max_pack_length=(uint) uint4korr(header+12);
-  set_if_bigger(share->base.pack_reclength,share->max_pack_length);
   elements=uint4korr(header+16);
   intervall_length=uint4korr(header+20);
   trees=uint2korr(header+24);
diff --git a/myisam/myisamchk.c b/myisam/myisamchk.c
index a259fbf2c19..34b89967ca7 100644
--- a/myisam/myisamchk.c
+++ b/myisam/myisamchk.c
@@ -1543,8 +1543,8 @@ static int mi_sort_records(MI_CHECK *param,
     mi_check_print_error(param,"Not enough memory for key block");
     goto err;
   }
-  if (!(sort_param.record=(byte*) my_malloc((uint) share->base.pack_reclength,
-					   MYF(0))))
+
+  if (!mi_alloc_rec_buff(info, -1, &sort_param.record))
   {
     mi_check_print_error(param,"Not enough memory for record");
     goto err;
@@ -1639,7 +1639,8 @@ err:
   {
     my_afree((gptr) temp_buff);
   }
-  my_free(sort_param.record,MYF(MY_ALLOW_ZERO_PTR));
+  my_free(mi_get_rec_buff_ptr(info, sort_param.record),
+          MYF(MY_ALLOW_ZERO_PTR));
   info->opt_flag&= ~(READ_CACHE_USED | WRITE_CACHE_USED);
   VOID(end_io_cache(&info->rec_cache));
   my_free(sort_info.buff,MYF(MY_ALLOW_ZERO_PTR));