← Back to team overview

maria-developers team mailing list archive

Re: c34533186ab: MDEV-18734 ASAN heap-use-after-free upon sorting by blob column from partitioned table

 

Hi, Aleksey!

On Jul 23, Aleksey Midenkov wrote:
> Now updated the patch to
>
> git diff dfa2d0bc13..9b564832e3

Thanks, no new comments about it. Just one question below still

> > > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc
> > > index c53732a2b51..70ed14448ea 100644
> > > --- a/sql/ha_partition.cc
> > > +++ b/sql/ha_partition.cc
> > > @@ -5130,7 +5135,18 @@ bool ha_partition::init_record_priority_queue()
> > >          i= bitmap_get_next_set(&m_part_info->read_partitions, i))
> > >    {
> > >      DBUG_PRINT("info", ("init rec-buf for part %u", i));
> > > -    int2store(ptr, i);
> > > +    if (table->s->blob_fields)
> > > +    {
> > > +      for (uint j= 0; j < table->s->blob_fields; ++j, ++objs)
> > > +      {
> > > +        blob_storage[j]= new (objs) Ordered_blob_storage;
> > > +        if (!blob_storage[j])
> > > +          DBUG_RETURN(true);
> >
> > is this possible?
> 
> If we don't use exceptions we should handle OOM this way. If we can't
> use -fno-exceptions we should make a wrapper by overloading default
> new() and catching std::badalloc.

I mean, it's a placement new, it doesn't allocate anything.
How can it return NULL if it simply returns objs?

...
> > > +    else
> > > +    {
> > > +      bool set_read_value;
> > > +      String *cached= blob->cached(set_read_value);
> > > +      if (cached)
> > > +      {
> > > +        cached->swap(s.blob);
> > > +        s.set_read_value= set_read_value;
> > > +      }
> > > +    }
> > > +  }
> > > +  table->move_fields(table->field, table->record[0], rec_buf);
> >
> > On swap_blobs() you always move all fields from rec_buf to table->record[0]?
> > Why?
> > This looks very suspicious.
> 
> Have you seen the first move_fields() call? We work on rec_buf and if
> it is the same as record[0] move_fields() does nothing.

Oops, no. I missed the first one. And it was very suspicious that you
always move in one direction.

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx


Follow ups

References