← Back to team overview

maria-developers team mailing list archive

Re: b1b38b64598: MDEV-29181 Potential corruption on FK update on a table with vcol index

 

Hi, Nikita,

On Aug 30, Nikita Malyavin wrote:
> revision-id: b1b38b64598 (mariadb-10.5.17-4-gb1b38b64598)
> parent(s): 3b656ac8c17
> author: Nikita Malyavin
> committer: Nikita Malyavin
> timestamp: 2022-08-23 00:27:39 +0300
> message:
> 
> MDEV-29181 Potential corruption on FK update on a table with vcol
> index
> 
> vc_templ->mysql_table concept is completely broken. This table pointer
> persists with a global access and can be overwritten by arbitrary
> thread. Like it wasn't enough, it also could become invalid after
> eviction from tc cache.

this is a bit misleading. I agree it's completely broken, though :)

But not "this table pointer", because, of course it persists with a
global access and can be overwritten and so on. This is by design, it's
protected by mysql_table_query_id.

The problem is that table (dict_table_t) argument of
innodb_find_table_for_vc() points to a shared data structure, not thread
local, as I believed. So vc_templ is shared too.

I suggest to rewrite the comment like

  vc_templ->mysql_table concept is completely broken.
  table argument of innodb_find_table_for_vc() points to a shared data
  structure dict_table_t, and thus vc_templ is shared too and cannot
  be used for a thread-local cache.

> This new solution simply does the following:
> * Sets up a referenced table list in TABLE instance (sql_base.cc)
> * Iterates through it along with dict_table_t::referenced_set
>   (row_upd_check_references_constraints)
> * Passes corresponding prebuilt through a call chain to
>   row_ins_foreign_check_on_constraint
> * Sets up newly created upd_node_t::prebuilt field and uses it
> accordingly is cascade update.

Is this upd_node_t::prebuilt used anywhere? As a prebuilt, I mean.
I couldn't find it (it's a complex patch, so I couldn've missed it).
As far as I can see, it's only used to store a pointer to TABLE.

So it seems to me than a simpler fix for this bug could be:
* remove vc_templ caching (mysql_table and mysql_table_query_id)
* store TABLE* in upd_node_t.
 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx


Follow ups