← Back to team overview

maria-developers team mailing list archive

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

 

Sergei,

On Sat, Mar 31, 2018 at 01:15:02PM +0200, Sergei Golubchik wrote:
> 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())
Sure, thanks!

> 
> Bigger question: Field_blob::store() also checks whether charset
> conversion is necessary. Why is it not applicable here?
Character set conversion is done in Field_longstr::compress().
But generally this optimisation is not applicable indeed, we can't just
return bmove()'d string here.

> 
> And, it'd be good to have a test case where from > value.ptr()
> (with a substring() function).
I'll try to come up with something.

Thanks,
Sergey


References