maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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?

>      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