summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlexander Barkov <bar@mariadb.com>2022-04-09 23:01:26 +0400
committerAlexander Barkov <bar@mariadb.com>2022-04-09 23:01:26 +0400
commit2ae92e89817b1eceed134fcf98e21f99ce26a22d (patch)
tree2a4f0c0e916adc01a0587dfd57a043fac5699a75
parentd623b5a1dd9196fdeb60d933a51f48967c9d4196 (diff)
downloadmariadb-git-2ae92e89817b1eceed134fcf98e21f99ce26a22d.tar.gz
MDEV-28267 ASAN heap-use-after-free in Item_sp::func_name_cstring
This crash happens on a combination of multiple conditions: - There is a thead#1 running an "ANALYZE FORMAT=JSON" query for a "SELECT .. FROM INFORMATION_SCHEMA.COLUMNS WHERE .. " - The WHERE clause contains a stored function call, say f1(). - The WHERE clause is built in the way so that the function f1() is never actually called, e.g. WHERE .. AND (TRUE OR f1()=expr) - The database contains multiple VIEWs that have the function f1() call, e.g. in their <select list> - The WHERE clause is built in the way so that these VIEWs match the condition. - There is a parallel thread#2 running. It creates or drops or recreates some other stored routine, say f2(), which is not used in the ANALYZE query. It effectively invalidates the stored routine cache for thread#1 without locking. Note, it is important that f2() is NOT used by ANALYZE query. Otherwise, thread#2 would be locked until the ANALYZE query finishes. When all of the above conditions are met, the following happens: 1. thread#1 starts the ANALYZE query. It notices a call for the stored function f1() in the WHERE condition. The function f1() gets parsed and cached to the SP cache. Its address also gets assigned to Item_func_sp::m_sp. 2. thread#1 starts iterating through all tables that match the WHERE condition to find the information about their columns. 3. thread#1 processes columns of the VIEW v1. It notices a call for f1() in the VIEW v1 definition. But f1() is already cached in the step#1 and it is up to date. So nothing happens with the SP cache. 4. thread#2 re-creates f2() in a non-locking mode. It effectively invalidates the SP cache in thread#1. 5. thread#1 processes columns of the VIEW v2. It notices a call for f1() in the VIEW v2 definition. It also notices that the cached version of f1() is not up to date. It frees the old definition of f1(), parses it again, and puts a new version of f1() to the SP cache. 6. thread#1 finishes processing rows and generates the JSON output. When printing the "attached_condition" value, it calls Item_func_sp::print() for f1(). But this Item_func_sp links to the old (freed) version of f1(). The above scenario demonstrates that Item_func_sp::m_sp can point to an alredy freed instance when Item_func_sp::func_name() is called, so accessing to Item_sp::m_sp->m_handler is not safe. This patch rewrites the code to use Item_func_sp::m_handler instead, which is always reliable. Note, this patch is only a cleanup for MDEV-28166 to quickly fix the regression. It fixes MDEV-28267. But it does not fix the core problem: The code behind I_S does not take into account that the SP cache can be updated while evaluating rows of the COLUMNS table. This is a corner case and it never happens with any other tables. I_S.COLUMNS is very special. Another example of the core problem is reported in MDEV-25243. The code accesses to Item_sp::m_sp->m_chistics of an already freed m_sp, again. It will be addressed separately.
-rw-r--r--mysql-test/main/sp-i_s_columns.result34
-rw-r--r--mysql-test/main/sp-i_s_columns.test49
-rw-r--r--sql/item.cc4
-rw-r--r--sql/item.h2
-rw-r--r--sql/item_func.cc2
-rw-r--r--sql/item_sum.cc2
6 files changed, 88 insertions, 5 deletions
diff --git a/mysql-test/main/sp-i_s_columns.result b/mysql-test/main/sp-i_s_columns.result
new file mode 100644
index 00000000000..707fd76fd77
--- /dev/null
+++ b/mysql-test/main/sp-i_s_columns.result
@@ -0,0 +1,34 @@
+#
+# MDEV-28267 ASAN heap-use-after-free in Item_sp::func_name_cstring
+#
+CREATE TABLE t1 (a INT);
+CREATE FUNCTION test.f1(a INT) RETURNS TEXT RETURN '';
+CREATE FUNCTION test.f2(a INT) RETURNS TEXT RETURN '';
+CREATE VIEW v1 AS SELECT f1(a) AS v1 FROM t1;
+CREATE VIEW v2 AS SELECT f1(a) AS v2 FROM t1;
+CREATE VIEW v3 AS SELECT f1(a) AS v3 FROM t1;
+CREATE VIEW v4 AS SELECT f1(a) AS v4 FROM t1;
+BEGIN NOT ATOMIC
+FOR i IN 1..10
+DO
+ANALYZE FORMAT=JSON SELECT *
+FROM INFORMATION_SCHEMA.COLUMNS
+WHERE
+TABLE_SCHEMA='test'
+ AND
+TABLE_NAME LIKE 'v%'
+ AND
+(SLEEP(0.01)=0 OR f1(ordinal_position) >'')
+ORDER BY TABLE_NAME;
+END FOR;
+END;
+$$
+connect con1,localhost,root,,test;
+CREATE OR REPLACE FUNCTION f2(a INT) RETURNS TEXT RETURN '';
+connection default;
+disconnect con1;
+connection default;
+DROP FUNCTION f1;
+DROP FUNCTION f2;
+DROP TABLE t1;
+DROP VIEW v1, v2, v3, v4;
diff --git a/mysql-test/main/sp-i_s_columns.test b/mysql-test/main/sp-i_s_columns.test
new file mode 100644
index 00000000000..0782d06d1cc
--- /dev/null
+++ b/mysql-test/main/sp-i_s_columns.test
@@ -0,0 +1,49 @@
+--echo #
+--echo # MDEV-28267 ASAN heap-use-after-free in Item_sp::func_name_cstring
+--echo #
+
+CREATE TABLE t1 (a INT);
+CREATE FUNCTION test.f1(a INT) RETURNS TEXT RETURN '';
+CREATE FUNCTION test.f2(a INT) RETURNS TEXT RETURN '';
+CREATE VIEW v1 AS SELECT f1(a) AS v1 FROM t1;
+CREATE VIEW v2 AS SELECT f1(a) AS v2 FROM t1;
+CREATE VIEW v3 AS SELECT f1(a) AS v3 FROM t1;
+CREATE VIEW v4 AS SELECT f1(a) AS v4 FROM t1;
+
+--disable_result_log
+
+DELIMITER $$;
+--send
+BEGIN NOT ATOMIC
+ FOR i IN 1..10
+ DO
+ ANALYZE FORMAT=JSON SELECT *
+ FROM INFORMATION_SCHEMA.COLUMNS
+ WHERE
+ TABLE_SCHEMA='test'
+ AND
+ TABLE_NAME LIKE 'v%'
+ AND
+ (SLEEP(0.01)=0 OR f1(ordinal_position) >'')
+ ORDER BY TABLE_NAME;
+ END FOR;
+END;
+$$
+DELIMITER ;$$
+
+--connect (con1,localhost,root,,test)
+CREATE OR REPLACE FUNCTION f2(a INT) RETURNS TEXT RETURN '';
+--connection default
+--reap
+
+--enable_result_log
+
+
+# Cleanup
+--disconnect con1
+--connection default
+
+DROP FUNCTION f1;
+DROP FUNCTION f2;
+DROP TABLE t1;
+DROP VIEW v1, v2, v3, v4;
diff --git a/sql/item.cc b/sql/item.cc
index f06055f0a08..8117199f01a 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -2893,7 +2893,7 @@ Item_sp::Item_sp(THD *thd, Item_sp *item):
}
const char *
-Item_sp::func_name(THD *thd) const
+Item_sp::func_name(THD *thd, bool is_package_function) const
{
/* Calculate length to avoid reallocation of string for sure */
size_t len= (((m_name->m_explicit_name ? m_name->m_db.length : 0) +
@@ -2914,7 +2914,7 @@ Item_sp::func_name(THD *thd) const
append_identifier(thd, &qname, &m_name->m_db);
qname.append('.');
}
- if (m_sp && m_sp->m_handler == &sp_handler_package_function)
+ if (is_package_function)
{
/*
In case of a package function split `pkg.func` and print
diff --git a/sql/item.h b/sql/item.h
index 44717a520d6..e48a2c1d9fd 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -4871,7 +4871,7 @@ public:
Field *sp_result_field;
Item_sp(THD *thd, Name_resolution_context *context_arg, sp_name *name_arg);
Item_sp(THD *thd, Item_sp *item);
- const char *func_name(THD *thd) const;
+ const char *func_name(THD *thd, bool is_package_function) const;
void cleanup();
bool sp_check_access(THD *thd);
bool execute(THD *thd, bool *null_value, Item **args, uint arg_count);
diff --git a/sql/item_func.cc b/sql/item_func.cc
index cb6fee34d48..b8355a38701 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -6362,7 +6362,7 @@ const char *
Item_func_sp::func_name() const
{
THD *thd= current_thd;
- return Item_sp::func_name(thd);
+ return Item_sp::func_name(thd, m_handler == &sp_handler_package_function);
}
diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index 034c4d2a515..4c6c3258d11 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -1458,7 +1458,7 @@ const char *
Item_sum_sp::func_name() const
{
THD *thd= current_thd;
- return Item_sp::func_name(thd);
+ return Item_sp::func_name(thd, false);
}
Item* Item_sum_sp::copy_or_same(THD *thd)