diff options
author | Sergei Golubchik <serg@mariadb.org> | 2022-11-17 19:23:08 +0100 |
---|---|---|
committer | Sergei Golubchik <serg@mariadb.org> | 2023-01-09 18:06:06 +0100 |
commit | 6cb84346e1bde63ec79dd5e3a7d80f69bb106ead (patch) | |
tree | f11ecc392600892e8d8fe7132f838831a72b5737 | |
parent | df82d68421699f5d51df15f0e5c1a6aa78bced63 (diff) | |
download | mariadb-git-6cb84346e1bde63ec79dd5e3a7d80f69bb106ead.tar.gz |
MDEV-17869 AddressSanitizer: use-after-poison in Item_change_list::rollback_item_tree_changes
it's incorrect to use change_item_tree() to replace arguments
of top-level AND/OR, because they (arguments) are stored in a List,
so a pointer to an argument is in the list_node, and individual
list_node's of top-level AND/OR can be deleted in Item_cond::build_equal_items().
In that case rollback_item_tree_changes() will modify the deleted object.
Luckily, it's not needed to use change_item_tree() for top-level
AND/OR, because the whole top-level item is copied and preserved
in prep_where and prep_on, and restored from there.
So, just don't.
Additionally to the test case in the commit it fixes
* ASAN failure of main.opt_tvc --ps
* ASAN failure of main.having_cond_pushdown --ps
-rw-r--r-- | mysql-test/main/prepare.result | 16 | ||||
-rw-r--r-- | mysql-test/main/prepare.test | 17 | ||||
-rw-r--r-- | sql/item.h | 10 | ||||
-rw-r--r-- | sql/item_cmpfunc.cc | 20 | ||||
-rw-r--r-- | sql/item_cmpfunc.h | 22 | ||||
-rw-r--r-- | sql/sql_lex.cc | 5 | ||||
-rw-r--r-- | sql/sql_select.cc | 7 | ||||
-rw-r--r-- | sql/sql_tvc.cc | 13 |
8 files changed, 86 insertions, 24 deletions
diff --git a/mysql-test/main/prepare.result b/mysql-test/main/prepare.result index cfe6603dbbe..7c730bff0c5 100644 --- a/mysql-test/main/prepare.result +++ b/mysql-test/main/prepare.result @@ -64,3 +64,19 @@ SQRT(?) is not null # # End of 10.3 tests # +# +# MDEV-17869 AddressSanitizer: use-after-poison in Item_change_list::rollback_item_tree_changes +# +create table t1 (pk int, v1 varchar(1)); +insert t1 values (1,'v'),(2,'v'),(3,'c'); +create table t2 (pk int, v1 varchar(1)); +insert t2 values (1,'x'); +create table t3 (pk int, i1 int, v1 varchar(1)); +insert t3 values (10,8,9); +execute immediate 'select straight_join 1 from (t1 join t2 on (t1.v1 = t2.v1)) +where (3, 6) in (select tc.pk, t3.i1 from (t3 join t1 as tc on (tc.v1 = t3.v1)) having tc.pk > 1 );'; +1 +drop table t1, t2, t3; +# +# End of 10.4 tests +# diff --git a/mysql-test/main/prepare.test b/mysql-test/main/prepare.test index 4d1573eb0c8..bf37f6dc8d1 100644 --- a/mysql-test/main/prepare.test +++ b/mysql-test/main/prepare.test @@ -52,3 +52,20 @@ execute p1 using 17864960750176564435; --echo # --echo # End of 10.3 tests --echo # + +--echo # +--echo # MDEV-17869 AddressSanitizer: use-after-poison in Item_change_list::rollback_item_tree_changes +--echo # +create table t1 (pk int, v1 varchar(1)); +insert t1 values (1,'v'),(2,'v'),(3,'c'); +create table t2 (pk int, v1 varchar(1)); +insert t2 values (1,'x'); +create table t3 (pk int, i1 int, v1 varchar(1)); +insert t3 values (10,8,9); +execute immediate 'select straight_join 1 from (t1 join t2 on (t1.v1 = t2.v1)) +where (3, 6) in (select tc.pk, t3.i1 from (t3 join t1 as tc on (tc.v1 = t3.v1)) having tc.pk > 1 );'; +drop table t1, t2, t3; + +--echo # +--echo # End of 10.4 tests +--echo # diff --git a/sql/item.h b/sql/item.h index 9389250d6ec..89d47f0026b 100644 --- a/sql/item.h +++ b/sql/item.h @@ -1868,6 +1868,11 @@ public: } virtual Item* transform(THD *thd, Item_transformer transformer, uchar *arg); + virtual Item* top_level_transform(THD *thd, Item_transformer transformer, + uchar *arg) + { + return transform(thd, transformer, arg); + } /* This function performs a generic "compilation" of the Item tree. @@ -1892,6 +1897,11 @@ public: return ((this->*transformer) (thd, arg_t)); return 0; } + virtual Item* top_level_compile(THD *thd, Item_analyzer analyzer, uchar **arg_p, + Item_transformer transformer, uchar *arg_t) + { + return compile(thd, analyzer, arg_p, transformer, arg_t); + } virtual void traverse_cond(Cond_traverser traverser, void *arg, traverse_order order) diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index b6509fa8b25..278e11f4002 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -5032,7 +5032,8 @@ bool Item_cond::walk(Item_processor processor, bool walk_subquery, void *arg) Item returned as the result of transformation of the root node */ -Item *Item_cond::transform(THD *thd, Item_transformer transformer, uchar *arg) +Item *Item_cond::do_transform(THD *thd, Item_transformer transformer, uchar *arg, + bool toplevel) { DBUG_ASSERT(!thd->stmt_arena->is_stmt_prepare()); @@ -5040,7 +5041,8 @@ Item *Item_cond::transform(THD *thd, Item_transformer transformer, uchar *arg) Item *item; while ((item= li++)) { - Item *new_item= item->transform(thd, transformer, arg); + Item *new_item= toplevel ? item->top_level_transform(thd, transformer, arg) + : item->transform(thd, transformer, arg); if (!new_item) return 0; @@ -5050,7 +5052,9 @@ Item *Item_cond::transform(THD *thd, Item_transformer transformer, uchar *arg) Otherwise we'll be allocating a lot of unnecessary memory for change records at each execution. */ - if (new_item != item) + if (toplevel) + *li.ref()= new_item; + else if (new_item != item) thd->change_item_tree(li.ref(), new_item); } return Item_func::transform(thd, transformer, arg); @@ -5081,8 +5085,8 @@ Item *Item_cond::transform(THD *thd, Item_transformer transformer, uchar *arg) Item returned as the result of transformation of the root node */ -Item *Item_cond::compile(THD *thd, Item_analyzer analyzer, uchar **arg_p, - Item_transformer transformer, uchar *arg_t) +Item *Item_cond::do_compile(THD *thd, Item_analyzer analyzer, uchar **arg_p, + Item_transformer transformer, uchar *arg_t, bool toplevel) { if (!(this->*analyzer)(arg_p)) return 0; @@ -5097,7 +5101,11 @@ Item *Item_cond::compile(THD *thd, Item_analyzer analyzer, uchar **arg_p, */ uchar *arg_v= *arg_p; Item *new_item= item->compile(thd, analyzer, &arg_v, transformer, arg_t); - if (new_item && new_item != item) + if (!new_item || new_item == item) + continue; + if (toplevel) + *li.ref()= new_item; + else thd->change_item_tree(li.ref(), new_item); } return Item_func::transform(thd, transformer, arg_t); diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 05b1f95ff36..01834fe06d7 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -3018,12 +3018,30 @@ public: bool top_level() { return abort_on_null; } void copy_andor_arguments(THD *thd, Item_cond *item); bool walk(Item_processor processor, bool walk_subquery, void *arg); - Item *transform(THD *thd, Item_transformer transformer, uchar *arg); + Item *do_transform(THD *thd, Item_transformer transformer, uchar *arg, bool toplevel); + Item *transform(THD *thd, Item_transformer transformer, uchar *arg) + { + return do_transform(thd, transformer, arg, 0); + } + Item *top_level_transform(THD *thd, Item_transformer transformer, uchar *arg) + { + return do_transform(thd, transformer, arg, 1); + } void traverse_cond(Cond_traverser, void *arg, traverse_order order); void neg_arguments(THD *thd); Item* propagate_equal_fields(THD *, const Context &, COND_EQUAL *); + Item *do_compile(THD *thd, Item_analyzer analyzer, uchar **arg_p, + Item_transformer transformer, uchar *arg_t, bool toplevel); Item *compile(THD *thd, Item_analyzer analyzer, uchar **arg_p, - Item_transformer transformer, uchar *arg_t); + Item_transformer transformer, uchar *arg_t) + { + return do_compile(thd, analyzer, arg_p, transformer, arg_t, 0); + } + Item* top_level_compile(THD *thd, Item_analyzer analyzer, uchar **arg_p, + Item_transformer transformer, uchar *arg_t) + { + return do_compile(thd, analyzer, arg_p, transformer, arg_t, 1); + } bool eval_not_null_tables(void *opt_arg); Item *build_clone(THD *thd); bool excl_dep_on_table(table_map tab_map); diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index f31a128832b..ac570be78aa 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -10042,9 +10042,8 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond) */ if (cond->get_extraction_flag() == FULL_EXTRACTION_FL) { - Item *result= cond->transform(thd, - &Item::multiple_equality_transformer, - (uchar *)this); + Item *result= cond->top_level_transform(thd, + &Item::multiple_equality_transformer, (uchar *)this); if (!result) return true; if (result->type() == Item::COND_ITEM && diff --git a/sql/sql_select.cc b/sql/sql_select.cc index d9922fddb8f..eb54484fa51 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -28295,11 +28295,11 @@ void JOIN::cache_const_exprs() return; if (conds) - conds->compile(thd, &Item::cache_const_expr_analyzer, &analyzer_arg, + conds->top_level_compile(thd, &Item::cache_const_expr_analyzer, &analyzer_arg, &Item::cache_const_expr_transformer, &cache_flag); cache_flag= FALSE; if (having) - having->compile(thd, &Item::cache_const_expr_analyzer, + having->top_level_compile(thd, &Item::cache_const_expr_analyzer, &analyzer_arg, &Item::cache_const_expr_transformer, &cache_flag); for (JOIN_TAB *tab= first_depth_first_tab(this); tab; @@ -28308,7 +28308,7 @@ void JOIN::cache_const_exprs() if (*tab->on_expr_ref) { cache_flag= FALSE; - (*tab->on_expr_ref)->compile(thd, &Item::cache_const_expr_analyzer, + (*tab->on_expr_ref)->top_level_compile(thd, &Item::cache_const_expr_analyzer, &analyzer_arg, &Item::cache_const_expr_transformer, &cache_flag); } } @@ -29365,7 +29365,6 @@ select_handler *SELECT_LEX::find_select_handler(THD *thd) } - /** @} (end of group Query_Optimizer) */ diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc index a25f6522bd9..d3d20ef7a9a 100644 --- a/sql/sql_tvc.cc +++ b/sql/sql_tvc.cc @@ -1121,12 +1121,10 @@ bool JOIN::transform_in_predicates_into_in_subq(THD *thd) { select_lex->parsing_place= IN_WHERE; conds= - conds->transform(thd, - &Item::in_predicate_to_in_subs_transformer, - (uchar*) 0); + conds->top_level_transform(thd, + &Item::in_predicate_to_in_subs_transformer, 0); if (!conds) DBUG_RETURN(true); - select_lex->prep_where= conds ? conds->copy_andor_structure(thd) : 0; select_lex->where= conds; } @@ -1141,13 +1139,10 @@ bool JOIN::transform_in_predicates_into_in_subq(THD *thd) if (table->on_expr) { table->on_expr= - table->on_expr->transform(thd, - &Item::in_predicate_to_in_subs_transformer, - (uchar*) 0); + table->on_expr->top_level_transform(thd, + &Item::in_predicate_to_in_subs_transformer, 0); if (!table->on_expr) DBUG_RETURN(true); - table->prep_on_expr= table->on_expr ? - table->on_expr->copy_andor_structure(thd) : 0; } } } |