maria-developers team mailing list archive

[Oleksandr Byelkin <sanja@xxxxxxxxxxx>]

Hi, Sergei!

On Wed, Oct 19, 2022 at 5:09 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> Hi, Oleksandr,
>
> Looks good.
> Just one question below:
>
> On Oct 19, Oleksandr Byelkin wrote:
> > revision-id: 39e20ca7e28 (mariadb-10.3.36-107-g39e20ca7e28)
> > parent(s): d6707ab11f6
> > author: Oleksandr Byelkin
> > committer: Oleksandr Byelkin
> > timestamp: 2022-10-18 16:38:09 +0200
> > message:
> >
> > MDEV-29748 ASAN errors or server crash in File_parser::parse upon
> concurrent view operations
> >
> > Read the version of the view share when we read definition to prevent
> > simultaniouse access to a view table SHARE (and so its MEM_ROOT)
> > from different threads.
> >
> > diff --git a/sql/sql_view.cc b/sql/sql_view.cc
> > index 68a8410f559..1e4a740f871 100644
> > --- a/sql/sql_view.cc
> > +++ b/sql/sql_view.cc
> > @@ -1175,19 +1175,24 @@ static int mysql_register_view(THD *thd,
> TABLE_LIST *view,
> >  bool mariadb_view_version_get(TABLE_SHARE *share)
> >  {
> >    DBUG_ASSERT(share->is_view);
> > +  DBUG_ASSERT(share->tabledef_version.length == 0);
> >
> >    if (!(share->tabledef_version.str=
> >          (uchar*) alloc_root(&share->mem_root,
> >                              MICROSECOND_TIMESTAMP_BUFFER_SIZE)))
> >      return TRUE;
> > -  share->tabledef_version.length= 0; // safety if the drfinition file
> is brocken
> >
> >    DBUG_ASSERT(share->view_def != NULL);
> >    if (share->view_def->parse((uchar *) &share->tabledef_version, NULL,
> >                               view_timestamp_parameters, 1,
> >                               &file_parser_dummy_hook))
> > +  {
> > +    // safety if the definition file is brocken
> > +    share->tabledef_version.length= 0;
>
> when can view_def->parse() fail? _Only_ if the definition is invalid?
> Can it succeed for one TABLE and fail for another, both of the same
> TABLE_SHARE?
>

Also if there is no memory.

If for the other table there will be more memory or file has changed it can
succeed.


>
> >      return TRUE;
> > +  }
> >    DBUG_ASSERT(share->tabledef_version.length ==
> MICROSECOND_TIMESTAMP_BUFFER_SIZE-1);
> > +
> >    return FALSE;
> >  }
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx
>