maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Varun,

So I am debugging the testcase from the bug report. I reach these lines:

>     if(i)
>     ext_key_length+= key_part->length;

(Why there is no indentation btw?)

and I see:

(gdb) p key_part->length
 $40 = 3072

3072=1024*3, that is, the NULL-byte and length bytes are NOT counted.
The code then proceeds to check if

  pk_length + ext_key_length  <= MAX_KEY_LENGTH

but that check could produce a wrong result if NULL- and length- bytes are not
counted.

The second issue is that I don't see where ext_key_length is zero'ed.  
I try debugging this query:

CREATE TABLE t2 (
  pk INT,    
  f1 VARCHAR(3),    
  f2 VARCHAR(1024),    
  f2a VARCHAR(1024),    
  PRIMARY KEY (pk),    
  KEY(f2),
  KEY(f2a) 
) ENGINE=InnoDB CHARSET utf8;

I put a breakpoint at this line:

>    if (share->use_ext_keys && i && !(keyinfo->flags & HA_NOSAME)
>                                 && (pk_length + ext_key_length  <= MAX_KEY_LENGTH))

(84 chars! where's the coding style again?)
and I observe:

(gdb) print ext_key_length
  $45 = 6144

This is clearly more than any [possible] key length for the table.

So, this particular patch is not a solution (even if the testcase happens to
pass with it).

On Mon, May 08, 2017 at 04:23:11PM +0530, Varun wrote:
> revision-id: e6dabcd3a7e994feaa1a1dc5d0c1a0d621272f72 (mariadb-10.2.2-387-ge6dabcd)
> parent(s): 39bce67b7aa27fc7e5458f36838148f6e0f4c3af
> author: Varun Gupta
> committer: Varun Gupta
> timestamp: 2017-05-08 16:21:32 +0530
> message:
> 
> MDEV-11196: Error:Run-Time Check Failure #2 - Stack around the variable 'key_buff'
>             was corrupted, server crashes in opt_sum_query
> 
> extended keys feature disabled if the length of extended key is longer than MAX_KEY_LEN
> 
> ---
>  sql/table.cc | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/sql/table.cc b/sql/table.cc
> index 398383e..554fc78 100644
> --- a/sql/table.cc
> +++ b/sql/table.cc
> @@ -695,7 +695,7 @@ static bool create_key_infos(const uchar *strpos, const uchar *frm_image_end,
>    KEY_PART_INFO *key_part= NULL;
>    ulong *rec_per_key= NULL;
>    KEY_PART_INFO *first_key_part= NULL;
> -  uint first_key_parts= 0;
> +  uint first_key_parts= 0, pk_length=0, ext_key_length=0;
>  
>    if (!keys)
>    {  
> @@ -765,6 +765,7 @@ static bool create_key_infos(const uchar *strpos, const uchar *frm_image_end,
>        keyinfo->algorithm= first_keyinfo->algorithm;
>        if (new_frm_ver >= 3)
>          keyinfo->block_size= first_keyinfo->block_size;
> +      pk_length= keyinfo->key_length;
>      }
>  
>      keyinfo->key_part=	 key_part;
> @@ -796,6 +797,8 @@ static bool create_key_infos(const uchar *strpos, const uchar *frm_image_end,
>  	strpos+=7;
>        }
>        key_part->store_length=key_part->length;
> +      if(i)
> +      ext_key_length+= key_part->length;
>      }
>  
>      /*
> @@ -805,7 +808,8 @@ static bool create_key_infos(const uchar *strpos, const uchar *frm_image_end,
>      keyinfo->ext_key_parts= keyinfo->user_defined_key_parts;
>      keyinfo->ext_key_flags= keyinfo->flags;
>      keyinfo->ext_key_part_map= 0;
> -    if (share->use_ext_keys && i && !(keyinfo->flags & HA_NOSAME))
> +    if (share->use_ext_keys && i && !(keyinfo->flags & HA_NOSAME)
> +                                 && (pk_length + ext_key_length  <= MAX_KEY_LENGTH))
>      {
>        for (j= 0; 
>             j < first_key_parts && keyinfo->ext_key_parts < MAX_REF_PARTS;
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

-- 
BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog