Skip to content

Commit

Permalink
Fix crashing when DocStore indexes contain long document paths
Browse files Browse the repository at this point in the history
Summary: System crashes while DocStore indexes contain long document paths. This is caused by buffer overwrite in `pack_keys()`. Although the overwrite does not throw exception in the first place, it triggers the assertion in `open_binary_frm()`. Checking the boundary condition fixes this issue.

Test Plan:
Create a mtr test with the following:
let $big_string = `select repeat('a', 2000)`;
eval create table t4(a int primary key, b blob, doc document, key(doc.$big_string as string(100))) engine=innodb;

Reviewers: santoshb, tianx

Reviewed By: tianx
  • Loading branch information
jiayisuse authored and jtolmer committed Jan 5, 2016
1 parent 6c61e7d commit 7367a3e
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 7 deletions.
6 changes: 6 additions & 0 deletions mysql-test/suite/json/r/type_document_path_indexing.result
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ primary key (a),
key (doc)) engine=innodb;
ERROR HY000: DOCUMENT column 'doc' cannot be part of a key
CREATE TABLE t1 (
a int primary key,
b blob not null,
doc document not null,
key(doc.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a as string(100))) engine=innodb;
ERROR HY000: Specified document path key was too long; max path length is 641 bytes
CREATE TABLE t1 (
a int not null,
b char (10) not null,
doc document not null,
Expand Down
9 changes: 9 additions & 0 deletions mysql-test/suite/json/t/type_document_path_indexing.test
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ CREATE TABLE t1 (
primary key (a),
key (doc)) engine=innodb;

## Specified key was too long
let $big_string = `SELECT REPEAT('.a', 2000)`;
--error ER_TOO_LONG_DOCUMENT_PATH_INDEX
eval CREATE TABLE t1 (
a int primary key,
b blob not null,
doc document not null,
key(doc$big_string as string(100))) engine=innodb;


## The prefix length must be specified when indexing on a document path as string
--error 1064
Expand Down
3 changes: 3 additions & 0 deletions sql/share/errmsg-utf8.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7194,6 +7194,9 @@ WARN_DOCUMENT_PATH_KEY_TRUNCATED
ER_ORDERBY_AS_TYPE_NOT_SUPPORTED
eng "Order by as type is not supported for non-document type."

ER_TOO_LONG_DOCUMENT_PATH_INDEX
eng "Specified document path key was too long; max path length is %d bytes"

#
# End of 5.6 error messages.
#
34 changes: 27 additions & 7 deletions sql/unireg.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ using std::max;

static uchar * pack_screens(List<Create_field> &create_fields,
uint *info_length, uint *screens, bool small_file);
static uint pack_keys(uchar *keybuff,uint key_count, KEY *key_info,
ulong data_offset);
static int64_t pack_keys(uchar *keybuff, uint key_length, uint key_count,
KEY *key_info, ulong data_offset);
static bool pack_header(uchar *forminfo,enum legacy_db_type table_type,
List<Create_field> &create_fields,
uint info_length, uint screens, uint table_options,
Expand Down Expand Up @@ -117,7 +117,8 @@ bool mysql_create_frm(THD *thd, const char *file_name,
handler *db_file)
{
LEX_STRING str_db_type;
uint reclength, info_length, screens, key_info_length, maxlength, i;
uint reclength, info_length, screens, maxlength, i;
int64_t key_info_length;
ulong key_buff_length;
File file;
ulong filepos, data_offset;
Expand Down Expand Up @@ -273,7 +274,10 @@ bool mysql_create_frm(THD *thd, const char *file_name,

key_buff_length= uint4korr(fileinfo+47);
keybuff=(uchar*) my_malloc(key_buff_length, MYF(0));
key_info_length= pack_keys(keybuff, keys, key_info, data_offset);
key_info_length= pack_keys(keybuff, key_buff_length, keys, key_info,
data_offset);
if (key_info_length < 0)
goto err;

/*
Ensure that there are no forms in this newly created form file.
Expand Down Expand Up @@ -615,10 +619,11 @@ static uchar *pack_screens(List<Create_field> &create_fields,

/* Pack keyinfo and keynames to keybuff for save in form-file. */

static uint pack_keys(uchar *keybuff, uint key_count, KEY *keyinfo,
ulong data_offset)
static int64_t pack_keys(uchar *keybuff, uint key_length, uint key_count,
KEY *keyinfo, ulong data_offset)
{
uint key_parts,length;
uint document_key_part_count= 0;
uchar *pos, *keyname_pos;
KEY *key,*end;
KEY_PART_INFO *key_part,*key_part_end;
Expand Down Expand Up @@ -649,6 +654,7 @@ static uint pack_keys(uchar *keybuff, uint key_count, KEY *keyinfo,
if (key_part->document_path_key_part)
{
document_path_keys_exist = true;
document_key_part_count++;
DBUG_ASSERT(f_is_document(key_part->key_type));
}

Expand Down Expand Up @@ -680,6 +686,14 @@ static uint pack_keys(uchar *keybuff, uint key_count, KEY *keyinfo,
/* Save document path keys */
if (document_path_keys_exist)
{
/* 10-byte metadata per document path key name will be filled
to buffer. Here we take off these 10-bytes along with the
'0' terminator to get the precise length of the reserved
space for document path key names
*/
key_length-= ((char *)pos - (char *)keybuff +
document_key_part_count * 10 + 1);
uint curr_path_key_length = 0;
for (key=keyinfo,end=keyinfo+key_count; key != end; key++)
{
for (key_part=key->key_part,
Expand Down Expand Up @@ -734,6 +748,12 @@ static uint pack_keys(uchar *keybuff, uint key_count, KEY *keyinfo,
for (uint i = 0;
i < key_part->document_path_key_part->number_of_names; i++)
{
curr_path_key_length +=
(uint)strlen(key_part->document_path_key_part->names[i]) + 1;
if (curr_path_key_length > key_length) {
my_error(ER_TOO_LONG_DOCUMENT_PATH_INDEX, MYF(0), key_length);
DBUG_RETURN(-1);
}
pos=(uchar*) strmov((char*)pos,
key_part->document_path_key_part->names[i]);
*pos++= (uchar) NAMES_SEP_CHAR;
Expand Down Expand Up @@ -772,7 +792,7 @@ static uint pack_keys(uchar *keybuff, uint key_count, KEY *keyinfo,
}
length=(uint) (pos-keyname_pos);
int2store(keybuff+4,length);
DBUG_RETURN((uint) (pos-keybuff));
DBUG_RETURN((int64_t) (pos-keybuff));
} /* pack_keys */


Expand Down

0 comments on commit 7367a3e

Please sign in to comment.