From 68353dc92a7b933fe4229337256c98b298115e45 Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Tue, 19 Sep 2023 08:57:36 +0700 Subject: [PATCH] MDEV-23902: MariaDB crash on calling function On creation of a VIEW that depends on a stored routine an instance of the class Item_func_sp is allocated on a memory root of SP statement. It happens since mysql_make_view() calls the method THD::activate_stmt_arena_if_needed() before parsing definition of the view. On the other hand, when sp_head's rcontext is created an instance of the class Field referenced by the data member Item_func_sp::result_field is allocated on the Item_func_sp's Query_arena (call arena) that set up inside the method Item_sp::execute_impl just before calling the method sp_head::execute_function() On return from the method sp_head::execute_function() all items allocated on the Item_func_sp's Query_arena are released and its memory root is freed (see implementation of the method Item_sp::execute_impl). As a consequence, the pointer Item_func_sp::result_field references to the deallocated memory. Later, when the method sp_head::execute cleans up items allocated for just executed SP instruction the method Item_func_sp::cleanup is invoked and tries to delete an object referenced by data member Item_func_sp::result_field that points to already deallocated memory, that results in a server abnormal termination. To fix the issue the current active arena shouldn't be switched to a statement arena inside the function mysql_make_view() that invoked indirectly by the method sp_head::rcontext_create. It is implemented by introducing the new Query_arena's state STMT_SP_QUERY_ARGUMENTS that is set when explicit Query_arena is created for placing SP arguments and other caller's side items used during SP execution. Then the method THD::activate_stmt_arena_if_needed() checks Query_arena's state and returns immediately without switching to statement's arena. --- mysql-test/main/sp.result | 25 +++++++++++++++++++++++++ mysql-test/main/sp.test | 32 ++++++++++++++++++++++++++++++++ sql/item.cc | 2 +- sql/sql_class.h | 23 ++++++++++++++++++++++- 4 files changed, 80 insertions(+), 2 deletions(-) diff --git a/mysql-test/main/sp.result b/mysql-test/main/sp.result index ea8bdecabd4..567597383bb 100644 --- a/mysql-test/main/sp.result +++ b/mysql-test/main/sp.result @@ -8935,5 +8935,30 @@ CREATE TABLE t1 (a INT); CREATE PROCEDURE p1() SELECT 1 FROM t1 PROCEDURE ANALYSE( 10, (SELECT a FROM t1)); ERROR 42000: PROCEDURE does not support subqueries or stored functions DROP TABLE t1; +# +# MDEV-23902: MariaDB crash on calling function +# +CREATE FUNCTION f2 () RETURNS VARCHAR(1) +BEGIN +DECLARE rec1 ROW TYPE OF v1; +SELECT z INTO rec1 FROM v1; +RETURN 1; +END| +CREATE FUNCTION f1 () RETURNS VARCHAR(1) RETURN f2() ; +CREATE FUNCTION f3 () RETURNS VARCHAR(1) RETURN f_not_exist(); +CREATE VIEW v1 AS SELECT f3() z; +SELECT f1(); +ERROR HY000: View 'test.v1' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them +# Check that crash doen't happen in case f3 completes with success. +DROP FUNCTION f3; +CREATE FUNCTION f3 () RETURNS VARCHAR(1) RETURN '!'; +SELECT f1(); +f1() +1 +# Clean up +DROP FUNCTION f1; +DROP FUNCTION f2; +DROP FUNCTION f3; +DROP VIEW v1; # End of 10.4 tests # diff --git a/mysql-test/main/sp.test b/mysql-test/main/sp.test index 84ad4ea7453..c11ea23080b 100644 --- a/mysql-test/main/sp.test +++ b/mysql-test/main/sp.test @@ -10535,5 +10535,37 @@ CREATE PROCEDURE p1() SELECT 1 FROM t1 PROCEDURE ANALYSE( 10, (SELECT a FROM t1) DROP TABLE t1; +--echo # +--echo # MDEV-23902: MariaDB crash on calling function +--echo # + +--delimiter | +CREATE FUNCTION f2 () RETURNS VARCHAR(1) +BEGIN + DECLARE rec1 ROW TYPE OF v1; + SELECT z INTO rec1 FROM v1; + RETURN 1; +END| +--delimiter ; + +CREATE FUNCTION f1 () RETURNS VARCHAR(1) RETURN f2() ; +CREATE FUNCTION f3 () RETURNS VARCHAR(1) RETURN f_not_exist(); +CREATE VIEW v1 AS SELECT f3() z; + +--error ER_VIEW_INVALID +SELECT f1(); + +--echo # Check that crash doen't happen in case f3 completes with success. +DROP FUNCTION f3; +CREATE FUNCTION f3 () RETURNS VARCHAR(1) RETURN '!'; + +SELECT f1(); + +--echo # Clean up +DROP FUNCTION f1; +DROP FUNCTION f2; +DROP FUNCTION f3; +DROP VIEW v1; + --echo # End of 10.4 tests --echo # diff --git a/sql/item.cc b/sql/item.cc index 685fdc2b69f..472cf3ce2fc 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -2840,7 +2840,7 @@ Item_sp::execute_impl(THD *thd, Item **args, uint arg_count) { init_sql_alloc(&sp_mem_root, "Item_sp", MEM_ROOT_BLOCK_SIZE, 0, MYF(0)); *sp_query_arena= Query_arena(&sp_mem_root, - Query_arena::STMT_INITIALIZED_FOR_SP); + Query_arena::STMT_SP_QUERY_ARGUMENTS); } bool err_status= m_sp->execute_function(thd, args, arg_count, diff --git a/sql/sql_class.h b/sql/sql_class.h index 4c172ba8e2a..e96fe300eba 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1070,11 +1070,21 @@ public: Prepared statement: STMT_INITIALIZED -> STMT_PREPARED -> STMT_EXECUTED. Stored procedure: STMT_INITIALIZED_FOR_SP -> STMT_EXECUTED. Other statements: STMT_CONVENTIONAL_EXECUTION never changes. + + Special case for stored procedure arguments: STMT_SP_QUERY_ARGUMENTS + This state never changes and used for objects + whose lifetime is whole duration of function call + (sp_rcontext, it's tables and items. etc). Such objects + should be deallocated after every execution of a stored + routine. Caller's arena/memroot can't be used for + placing such objects since memory allocated on caller's + arena not freed until termination of user's session. */ enum enum_state { STMT_INITIALIZED= 0, STMT_INITIALIZED_FOR_SP= 1, STMT_PREPARED= 2, - STMT_CONVENTIONAL_EXECUTION= 3, STMT_EXECUTED= 4, STMT_ERROR= -1 + STMT_CONVENTIONAL_EXECUTION= 3, STMT_EXECUTED= 4, + STMT_SP_QUERY_ARGUMENTS= 5, STMT_ERROR= -1 }; enum_state state; @@ -4046,6 +4056,17 @@ public: inline Query_arena *activate_stmt_arena_if_needed(Query_arena *backup) { + if (state == Query_arena::STMT_SP_QUERY_ARGUMENTS) + /* + Caller uses the arena with state STMT_SP_QUERY_ARGUMENTS for stored + routine's parameters. Lifetime of these objects spans a lifetime of + stored routine call and freed every time the stored routine execution + has been completed. That is the reason why switching to statement's + arena is not performed for arguments, else we would observe increasing + of memory usage while a stored routine be called over and over again. + */ + return NULL; + /* Use the persistent arena if we are in a prepared statement or a stored procedure statement and we have not already changed to use this arena.