Re: [Maria-developers] 4de59fa0f92: MENT-360: ASAN heap-use-after-free in strmake_root / Query_arena::strmake

Hi, Oleksandr! On Nov 06, Oleksandr Byelkin wrote:

revision-id: 4de59fa0f92 (mariadb-10.2.27-129-g4de59fa0f92) parent(s): 4b21495343d author: Oleksandr Byelkin committer: Oleksandr Byelkin timestamp: 2019-10-18 15:14:20 +0200 message:

MENT-360: ASAN heap-use-after-free in strmake_root / Query_arena::strmake

Do not allow thd->query point to freed memory, use safe method to set query.

--- mysql-test/r/processlist.result | 32 +++++++++++++++++++++++ mysql-test/t/processlist.test | 58 +++++++++++++++++++++++++++++++++++++++++ sql/sql_prepare.cc | 7 ++++- 3 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/mysql-test/r/processlist.result b/mysql-test/r/processlist.result index ab518d961ef..066154feaf1 100644 --- a/mysql-test/r/processlist.result +++ b/mysql-test/r/processlist.result @@ -43,3 +43,35 @@ Message Incorrect string value: '\xF0\x9F\x98\x8Eyy...' for column `information_ # # End of 10.1 tests # +# +# MENT-360: ASAN heap-use-after-free in strmake_root / +# Query_arena::strmake +# +CREATE PROCEDURE pr1() +BEGIN +DECLARE CONTINUE HANDLER FOR 1146 SET @a = 1; +DECLARE CONTINUE HANDLER FOR 1243 SET @a = 1; +LOOP +PREPARE stmt FROM "CREATE TEMPORARY TABLE ps AS SELECT * FROM non_existing_table"; +EXECUTE stmt; +END LOOP; +END $ +CREATE PROCEDURE pr2() +BEGIN +DECLARE CONTINUE HANDLER FOR 1094 SET @a = 1; +LOOP +SHOW EXPLAIN FOR 1; +END LOOP; +END $ +connect con1,localhost,root,,; +CALL pr1(); +connect con2,localhost,root,,; +CALL pr2(); +connection default; +KILL 6; +KILL 7;

^^^ you forgot replace_result also, mysqltest was designed for deterministic test cases, it'd be better to rewrite this test to be deterministic, if possible.

+DROP PROCEDURE pr1; +DROP PROCEDURE pr2; +# +# end of Enterprise tests +# diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 525f09d611b..d0043979135 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -2885,6 +2885,11 @@ void mysql_sql_stmt_prepare(THD *thd)

if (stmt->prepare(query.str, (uint) query.length)) { + /* + stmt->prepare() sets thd->query_string, so we have to reset it beck, + before it will point to uninitialised memory + */ + thd->set_query(orig_query); /* Statement map deletes the statement on erase */ thd->stmt_map.erase(stmt); } @@ -2902,7 +2907,7 @@ void mysql_sql_stmt_prepare(THD *thd) But here we should restore the original query so it's mentioned in logs properly. */ - thd->set_query_inner(orig_query); + thd->set_query(orig_query); DBUG_VOID_RETURN; }

may be better to have set_query() only once, like in bool res=stmt->prepare(query.str, (uint) query.length); thd->set_query(orig_query); if (res) ... Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org