← Back to team overview

maria-developers team mailing list archive

Re: MDEV-14929 - AddressSanitizer: memcpy-param-overlap in Field_longstr::compress

 

Hi, Sergey,

> From 6de643a580b4e706e0201927e43a033f5cb98970 Mon Sep 17 00:00:00 2001
> From: Sergey Vojtovich <svoj@xxxxxxxxxxx>
> Date: Wed, 21 Mar 2018 13:54:15 +0400
> Subject: [PATCH] MDEV-14929 - AddressSanitizer: memcpy-param-overlap in
>  Field_longstr::compress
> 
> Handle overlaping "from" and Field_blob_compressed::value for compressed
> blobs similarily to regular blobs.
> ---
>  mysql-test/r/column_compression.result | 13 +++++++++++++
>  mysql-test/t/column_compression.test   | 12 ++++++++++++
>  sql/field.cc                           | 16 +++++++++++-----
>  3 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/sql/field.cc b/sql/field.cc
> index 9da65526e145..1b044ec740e2 100644
> --- a/sql/field.cc
> +++ b/sql/field.cc
> @@ -8718,17 +8718,23 @@ int Field_blob_compressed::store(const char *from, size_t length,
>  {
>    ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED;
>    uint to_length= (uint)MY_MIN(max_data_length(), field_charset->mbmaxlen * length + 1);
> +  String tmp(from, length, cs);
>    int rc;
>  
> +  if (from >= value.ptr() && from <= value.ptr() + value.length() &&
> +      tmp.copy(from, length, cs))

minor style comment: this could be 

     if (from >= value.ptr() && from <= value.end() && tmp.copy())

Bigger question: Field_blob::store() also checks whether charset
conversion is necessary. Why is it not applicable here?

And, it'd be good to have a test case where from > value.ptr()
(with a substring() function).

> +    goto oom;
> +
>    if (value.alloc(to_length))
> -  {
> -    set_ptr((uint32) 0, NULL);
> -    return -1;
> -  }
> +    goto oom;
>  
> -  rc= compress((char*) value.ptr(), &to_length, from, (uint)length, cs);
> +  rc= compress((char*) value.ptr(), &to_length, tmp.ptr(), (uint) length, cs);
>    set_ptr(to_length, (uchar*) value.ptr());
>    return rc;
> +
> +oom:
> +  set_ptr((uint32) 0, NULL);
> +  return -1;
>  }
 
Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx


Follow ups