Skip to content

Commit

Permalink
MDEV-23902: MariaDB crash on calling function
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dmitryshulga committed Sep 19, 2023
1 parent 8bbe3a3 commit 68353dc
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 2 deletions.
25 changes: 25 additions & 0 deletions mysql-test/main/sp.result
Expand Up @@ -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
#
32 changes: 32 additions & 0 deletions mysql-test/main/sp.test
Expand Up @@ -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 #
2 changes: 1 addition & 1 deletion sql/item.cc
Expand Up @@ -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,
Expand Down
23 changes: 22 additions & 1 deletion sql/sql_class.h
Expand Up @@ -1070,11 +1070,21 @@ class Query_arena
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;
Expand Down Expand Up @@ -4046,6 +4056,17 @@ class THD: public THD_count, /* this must be first */

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.
Expand Down

0 comments on commit 68353dc

Please sign in to comment.