← Back to team overview

maria-developers team mailing list archive

Re: cc0bc69: MDEV-15813 ASAN use-after-poison in hp_hashnr upon HANDLER READ on a

 

Hi, Alexey!

On May 06, Alexey Botchkov wrote:
> revision-id: cc0bc69f2d28aace12487f930677abcdffa5ce97 (mariadb-10.3.6-102-gcc0bc69)
> parent(s): 73a10cbcc5178ae5378abb821428d35d3276b4da
> committer: Alexey Botchkov
> timestamp: 2018-05-06 02:37:20 +0400
> message:
> 
> MDEV-15813 ASAN use-after-poison in hp_hashnr upon HANDLER READ on a
> versioned HEAP table.
> 
> hp_rkey now returns an error if we try to use HASH key where it doesn't
> work.
> 
> diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt
> index c55ac4f..fcd20d8 100644
> --- a/sql/share/errmsg-utf8.txt
> +++ b/sql/share/errmsg-utf8.txt
> @@ -7915,3 +7915,5 @@ ER_UPDATED_COLUMN_ONLY_ONCE
>          eng "The column %`s.%`s cannot be changed more than once in a single UPDATE statement"
>  ER_EMPTY_ROW_IN_TVC
>          eng "Row with no elements is not allowed in table value constructor in this context"
> +ER_KEY_DOESNT_SUPPORT
> +        eng "HASH index %`s does not support this operation"

Use "%s index %`s does not support this operation" please. First
parameter could be taken from handler::index_type().

By the way, check what happens when the index is a fulltext or rtree. Is
it also a crash or this case is handled somehow? If it does, the same
check should take care of HASH indexes, no need to do a separate
solution for them.

> diff --git a/storage/heap/hp_rkey.c b/storage/heap/hp_rkey.c
> index 692e1a0..a7b81ae 100644
> --- a/storage/heap/hp_rkey.c
> +++ b/storage/heap/hp_rkey.c
> @@ -60,6 +60,14 @@ int heap_rkey(HP_INFO *info, uchar *record, int inx, const uchar *key,
>    }
>    else
>    {
> +    /*
> +      HASH key doesn't support search other than equality,
> +      and doesn't support prefix search.
> +    */
> +    if (find_flag != HA_READ_KEY_EXACT ||
> +        (1UL << share->keydef[inx].keysegs) != keypart_map + 1)
> +      DBUG_RETURN(my_errno= HA_ERR_KEY_DOESNT_SUPPORT);
> +

No, please, not here. The parent handler object knows index types, it
knows index flags and it can return an error if the corresponding index
flag is missing. No need to push this check down to the engine if the
server can do it in, say, ha_index_read.

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx