MDEV-35908 Unexpected error, crash, MSAN errors, assertion failures upon CHECK

Two different problems are fixed here:
1. A bogus inet6 -> time match turned to be possible:
inet6 size is 20, while time size is 3, so the bigger is pointing to a smaller
When innodb makes a referencial check, it compares to the min(len1, len2), i.e.
in this case 3 bytes, and then checks that the rest is filled with 0x20,
that is, space-padded.

Probably, this is just a bogus hole in semantic checks, but now this reference
is possible. Yet, implementing correct checks for such cases will require a lot
of effort, so let's just skip such keys.

2. char(15) -> varchar(15) record check caused use-of-uninitialized.

This is a tough one. A from-table and a to-table store data in different
formats, both in and in key, which was a problem for key_copy.

The solution is to use save_in_field to first convert the data to the format of
the to-table's record, and then use a to-table's field to store the string in
the key.

Note that varchar(15) -> char(15) even doesn't work well in innodb, and
insertion is impossible, unless foreign_key_checks is off. However, cascade
deletions do work, though, again, with a quirk (see the tests).

mysql-test/main/check_table_references.result

@@ -231,3 +231,39 @@ load index into cache t;
 Table	Op	Msg_type	Msg_text
 test.t	preload_keys	note	The storage engine for the table doesn't support preload_keys
 drop table t;
+# MDEV-35908 Unexpected error, crash, MSAN errors, assertion failures
+# upon CHECK
+create table t (pk int primary key, a varchar(15), b char(15), key(a), key(b),
+foreign key (a) references t(b) on delete cascade)
+engine=innodb;
+insert into t values (1,'foo', 'foo');
+ERROR 23000: Cannot add or update a child row: a foreign key constraint fails (`test`.`t`, CONSTRAINT `t_ibfk_1` FOREIGN KEY (`a`) REFERENCES `t` (`b`) ON DELETE CASCADE)
+set foreign_key_checks = off;
+insert into t values (1,'foo', 'foo');
+insert into t values (2,'fee', 'qwe');
+insert into t values (3,'fii', 'fiii');
+# This one is really confusing, but it's a valid reference to 'foo'
+# See the select results after b='foo' record is deleted.
+insert into t values (4,'foo ', 'floo');
+set foreign_key_checks = on;
+check table t extended;
+Table	Op	Msg_type	Msg_text
+test.t	check	Warning	Cannot add or update a child row: a foreign key constraint fails (Key: t_ibfk_1, record: 'fee')
+test.t	check	Warning	Cannot add or update a child row: a foreign key constraint fails (Key: t_ibfk_1, record: 'fii')
+test.t	check	error	Corrupt
+drop table t;
+create table t (pk int primary key, a char(15), b varchar(15), key(b), foreign key (a) references t(b)) engine=innodb;
+insert into t values (1,'foo', 'foo');
+check table t extended;
+Table	Op	Msg_type	Msg_text
+test.t	check	status	OK
+drop table t;
+# Bogus table: length of `b` is bigger than length of `a`
+create table t (a time, b inet6, key(a)) engine=innodb;
+alter table t add foreign key (b) references t (a);
+insert into t values ('00:00:00','8000:0020:2020:2020:2020:2020:2020:2020');
+check table t extended;
+Table	Op	Msg_type	Msg_text
+test.t	check	Warning	Bogus foreign key t_ibfk_1 is skipped.
+test.t	check	error	Corrupt
+drop table t;

mysql-test/main/check_table_references.test

@@ -155,3 +155,41 @@ drop table t1;
 create table t (pk int primary key) engine=innodb;
 load index into cache t;
 drop table t;
+
+--echo # MDEV-35908 Unexpected error, crash, MSAN errors, assertion failures
+--echo # upon CHECK
+
+create table t (pk int primary key, a varchar(15), b char(15), key(a), key(b),
+                foreign key (a) references t(b) on delete cascade)
+               engine=innodb;
+
+--error ER_NO_REFERENCED_ROW_2
+insert into t values (1,'foo', 'foo');
+
+set foreign_key_checks = off;
+insert into t values (1,'foo', 'foo');
+insert into t values (2,'fee', 'qwe');
+insert into t values (3,'fii', 'fiii');
+
+--echo # This one is really confusing, but it's a valid reference to 'foo'
+--echo # See the select results after b='foo' record is deleted.
+insert into t values (4,'foo ', 'floo');
+set foreign_key_checks = on;
+
+check table t extended;
+
+drop table t;
+
+create table t (pk int primary key, a char(15), b varchar(15), key(b), foreign key (a) references t(b)) engine=innodb;
+insert into t values (1,'foo', 'foo');
+check table t extended;
+drop table t;
+
+
+--echo # Bogus table: length of `b` is bigger than length of `a`
+create table t (a time, b inet6, key(a)) engine=innodb;
+alter table t add foreign key (b) references t (a);
+insert into t values ('00:00:00','8000:0020:2020:2020:2020:2020:2020:2020');
+check table t extended;
+
+drop table t;

sql/handler.cc

@@ -7959,7 +7959,8 @@ int handler::check_record_reference(const KEY *this_key, const KEY *ref_key,
     if (this_key->key_part[kp].field->is_real_null())
       return 0;
 
-  key_copy(key_buf, this_record, this_key, ref_key, prefix_length, false);
+  key_copy(key_buf, ref_record, this_record, this_key, ref_key, prefix_length,
+           false);
 
   int error= ha_index_read_map(ref_record, key_buf,
                                make_prev_keypart_map(fk_parts),

sql/key.cc

@@ -106,6 +106,9 @@ int find_ref_key(KEY *key, uint key_count, uchar *record, Field *field,
     form a key.
 
   @param to_key      buffer that will be used as a key
+  @param extra_buff  a buffer to store extra intermediate data of a size not
+                     less than a maximum length of a field in a to-table.
+                     Can be null if from_key_info == to_key_info
   @param from_record full record to be copied from
   @param from_key_info  a descriptor of the index that corresponds a data to be
                         copied from the record
@@ -117,7 +120,8 @@ int find_ref_key(KEY *key, uint key_count, uchar *record, Field *field,
   @param with_zerofill  skipped bytes in the key buffer to be filled with 0
 */
 
-void key_copy(uchar *to_key, const uchar *from_record, const KEY *from_key_info,
+void key_copy(uchar *to_key, uchar *extra_buff,
+              const uchar *from_record, const KEY *from_key_info,
               const KEY *to_key_info, uint key_length, bool with_zerofill)
 {
   uint length;
@@ -148,17 +152,36 @@ void key_copy(uchar *to_key, const uchar *from_record, const KEY *from_key_info,
       }
     }
     auto *from_ptr= key_part->field->ptr_in_record(from_record);
-    if (key_part->key_part_flag & HA_BLOB_PART ||
-        key_part->key_part_flag & HA_VAR_LENGTH_PART)
+    if (key_part->is_var_length() || to_key_part->is_var_length())
     {
       DBUG_ASSERT(to_key_part->length == key_part->length);
-      key_length-= HA_KEY_BLOB_LENGTH;
+      Field *field= key_part->field;
+
+      if (to_key_part->is_var_length() != key_part->is_var_length())
+      {
+        key_part->field->move_field_offset(key_part->field->table->record[0]
+                                           - from_record);
+        uchar *to_ptr= to_key_part->field->ptr;
+        to_key_part->field->ptr= extra_buff;
+
+        key_part->field->save_in_field(to_key_part->field);
+        field= to_key_part->field;
+        from_ptr= extra_buff;
+
+        to_key_part->field->ptr= to_ptr;
+        key_part->field->move_field_offset(from_record
+                                           - key_part->field->table->record[0]);
+      }
+
+      if (to_key_part->is_var_length())
+        key_length-= HA_KEY_BLOB_LENGTH;
       length= MY_MIN(key_length, key_part->length);
-      uint bytes= key_part->field->get_key_image(to_key, length, from_ptr,
-                                   Field::image_type(from_key_info->algorithm));
+      uint bytes= field->get_key_image(to_key, length, from_ptr,
+                                   Field::image_type(to_key_info->algorithm));
       if (with_zerofill && bytes < length)
         bzero((char*) to_key + bytes, length - bytes);
-      to_key+= HA_KEY_BLOB_LENGTH;
+      if (to_key_part->is_var_length())
+        to_key+= HA_KEY_BLOB_LENGTH;
     }
     else
     {
@@ -178,7 +201,8 @@ void key_copy(uchar *to_key, const uchar *from_record, const KEY *from_key_info,
 void key_copy(uchar *to_key, const uchar *from_record, const KEY *key_info,
               uint key_length, bool with_zerofill)
 {
-  key_copy(to_key, from_record, key_info, key_info, key_length, with_zerofill);
+  key_copy(to_key, NULL, from_record, key_info, key_info, key_length,
+           with_zerofill);
 }
 
 /**

sql/key.h

@@ -25,9 +25,9 @@ typedef struct st_key_part_info KEY_PART_INFO;
 
 int find_ref_key(KEY *key, uint key_count, uchar *record, Field *field,
                  uint *key_length, uint *keypart);
-void key_copy(uchar *to_key, const uchar *from_record, const KEY *from_key_info,
-              const KEY *to_key_info, uint key_length,
-              bool with_zerofill= FALSE);
+void key_copy(uchar *to_key, uchar *extra_buff, const uchar *from_record,
+              const KEY *from_key_info, const KEY *to_key_info,
+              uint key_length, bool with_zerofill= FALSE);
 void key_copy(uchar *to_key, const uchar *from_record, const KEY *key_info,
               uint key_length, bool with_zerofill= FALSE);
 void key_restore(uchar *to_record, const uchar *from_key, KEY *key_info,

sql/sql_admin.cc

@@ -1829,6 +1829,37 @@ static bool check_key_description(THD *thd, const KEY *key,
   return true;
 }
 
+static
+bool check_foreign_key_description(THD *thd, const TABLE *this_table,
+                                   const TABLE *ref_table,
+                                   const KEY *this_key, const KEY *ref_key,
+                                   const FOREIGN_KEY_INFO &fk)
+{
+  if (!check_key_description(thd, this_key, this_table, fk.foreign_fields))
+  {
+    report_check_key_wrong_description(thd, this_key, this_table, fk);
+    return false;
+  }
+  if (!check_key_description(thd, ref_key, ref_table, fk.referenced_fields))
+  {
+    report_check_key_wrong_description(thd, ref_key, ref_table, fk);
+    return false;
+  }
+
+  for (uint kp = 0; kp < fk.foreign_fields.size(); kp++)
+  {
+    if(this_key->key_part[kp].length > ref_key->key_part[kp].length)
+    {
+      push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
+                          HA_ERR_INDEX_CORRUPT,
+                          "Bogus foreign key %s is skipped.",
+                          fk.foreign_id.str);
+      return false;
+    }
+  }
+  return true;
+}
+
 static
 int check_foreign_key_relation(THD *thd, const TABLE *this_table,
                                const TABLE *ref_table,
@@ -1846,16 +1877,10 @@ int check_foreign_key_relation(THD *thd, const TABLE *this_table,
   if (!this_key || !ref_key)
     return HA_ADMIN_CORRUPT;
 
-  if (!check_key_description(thd, this_key, this_table, fk.foreign_fields))
-  {
-    report_check_key_wrong_description(thd, this_key, this_table, fk);
+
+  if (!check_foreign_key_description(thd, this_table, ref_table, this_key,
+                                     ref_key, fk))
     return HA_ADMIN_CORRUPT;
-  }
-  if (!check_key_description(thd, ref_key, ref_table, fk.referenced_fields))
-  {
-    report_check_key_wrong_description(thd, ref_key, ref_table, fk);
-    return HA_ADMIN_CORRUPT;
-  }
 
   size_t kp_num= fk.foreign_fields.size();
   return check_key_referential_integrity(this_table, ref_table, this_key,

sql/structs.h

@@ -93,6 +93,10 @@ typedef struct st_key_part_info {	/* Info about a key part */
   uint16 key_part_flag;                 /* 0 or HA_REVERSE_SORT */
   uint8 type;
   uint8 null_bit;                       /* Position to null_bit */
+  bool is_var_length() const
+  {
+    return key_part_flag & HA_BLOB_PART || key_part_flag & HA_VAR_LENGTH_PART;
+  }
 } KEY_PART_INFO ;
 
 class engine_option_value;