[MDEV-33209] Stack overflow in main.json_debug_nonembedded due to incorrect debug injection Created: 2024-01-10  Updated: 2024-01-22

Status: Confirmed
Project: MariaDB Server
Component/s: JSON, Tests
Affects Version/s: 10.4, 10.5, 10.6, 10.11, 11.0, 11.1, 11.2, 11.3, 11.4
Fix Version/s: 10.4, 10.5, 10.6, 10.11, 11.0, 11.1, 11.2, 11.3, 11.4

Type: Bug Priority: Critical
Reporter: Marko Mäkelä Assignee: Rucha Deodhar
Resolution: Unresolved Votes: 0
Labels: ASAN, affects-tests, debug

Issue Links:
Relates
relates to MDEV-28762 recursive call of some json functions... Closed
relates to MDEV-31605 cmake/stack_direction.c does not work... Closed

 Description   

A cmake -DWITH_ASAN=ON build on Clang 16.0.6 crashes for me in a test:

10.6 c6c2a2b8d463e0a103997aba81f18c37fcdc0597

CURRENT_TEST: main.json_debug_nonembedded
mysqltest: At line 16: query 'SELECT * from JSON_TABLE('[{"a": 1, "b": [11,111]}, {"a": 2, "b": [22,222]}]', '$[*]' COLUMNS( a INT PATH '$.a')) as tt' failed with wrong errno <Unknown> (2013): 'Lost connection to server during query', instead of ER_STACK_OVERRUN_NEED_MORE (1436)...

I debugged it with

./mtr --rr main.json_debug_nonembedded

and was able to determine that the crash occurs around the following:

10.6 c6c2a2b8d463e0a103997aba81f18c37fcdc0597

#0  0x0000555c2343cd9e in json_path_parts_compare (a=<optimized out>, a_end=<optimized out>, b=<optimized out>, b_end=<optimized out>, vt=<optimized out>) at /mariadb/10.6/sql/item_jsonfunc.cc:153
#1  0x0000555c2358f6ac in Json_table_nested_path::scan_next (this=0x62b000085b78) at /mariadb/10.6/sql/json_table.cc:342
#2  0x0000555c2358fb76 in ha_json_table::rnd_next (this=0x61b00003aab8, buf=0x619000077c90 "\377\377") at /mariadb/10.6/sql/json_table.cc:447
#3  0x0000555c225406d9 in handler::ha_rnd_next (this=0x61b00003aab8, buf=<optimized out>) at /mariadb/10.6/sql/handler.cc:3463

The problematic code is as follows:

static
int get_disallowed_table_deps_for_list(MEM_ROOT *mem_root,
                                       TABLE_LIST *table_func,
                                       List<TABLE_LIST> *join_list,
                                       List<TABLE_LIST> *disallowed_tables)
{
  TABLE_LIST *table;
  NESTED_JOIN *nested_join;
  List_iterator<TABLE_LIST> li(*join_list);
 
  DBUG_EXECUTE_IF("json_check_min_stack_requirement",
                  {
                    long arbitrary_var;
                    long stack_used_up= (available_stack_size(current_thd->thread_stack, &arbitrary_var));
                    ALLOCATE_MEM_ON_STACK(my_thread_stack_size-stack_used_up-STACK_MIN_SIZE);
                  });
  if (check_stack_overrun(current_thd, STACK_MIN_SIZE , NULL))
    return 1;

In my execution, the local variables had been optimized away. On the call to __asan_alloca_poison(), I observed that $rsp-thd->thread_stack (thd from a much earlier frame) is 13260824. This should roughly be the value of stack_used_up above. The value of my_thread_stack_size would be less than that (11534336), and therefore the parameter to ALLOCATE_MEM_ON_STACK would wrap around, to -1742488. STACK_MIN_SIZE is 16000.

I tried to fix the incorrect check, but ultimately gave up:

diff --git a/sql/json_table.cc b/sql/json_table.cc
index 65fe3c9a659..62475b4c45c 100644
--- a/sql/json_table.cc
+++ b/sql/json_table.cc
@@ -120,9 +120,13 @@ int get_disallowed_table_deps_for_list(MEM_ROOT *mem_root,
 
   DBUG_EXECUTE_IF("json_check_min_stack_requirement",
                   {
-                    long arbitrary_var;
-                    long stack_used_up= (available_stack_size(current_thd->thread_stack, &arbitrary_var));
-                    ALLOCATE_MEM_ON_STACK(my_thread_stack_size-stack_used_up-STACK_MIN_SIZE);
+                    ssize_t stack_used_up=
+                      (available_stack_size(current_thd->thread_stack,
+                                            &stack_used_up));
+                    if (ssize_t(my_thread_stack_size) <=
+                        stack_used_up - STACK_MIN_SIZE)
+                      ALLOCATE_MEM_ON_STACK(my_thread_stack_size -
+                                            stack_used_up - STACK_MIN_SIZE);
                   });
   if (check_stack_overrun(current_thd, STACK_MIN_SIZE , NULL))
     return 1;

Why do we use this fragile code? Why not just get rid of all the alloca() black magic and inject an error, like this:

diff --git a/sql/json_table.cc b/sql/json_table.cc
index 65fe3c9a659..9e7caf7f05f 100644
--- a/sql/json_table.cc
+++ b/sql/json_table.cc
@@ -29,21 +29,14 @@
 
 #define HA_ERR_JSON_TABLE (HA_ERR_LAST+1)
 
-/*
-  Allocating memory and *also* using it (reading and
-  writing from it) because some build instructions cause
-  compiler to optimize out stack_used_up. Since alloca()
-  here depends on stack_used_up, it doesnt get executed
-  correctly and causes json_debug_nonembedded to fail
-  ( --error ER_STACK_OVERRUN_NEED_MORE does not occur).
-*/
-#define ALLOCATE_MEM_ON_STACK(A) do \
-                              { \
-                                uchar *array= (uchar*)alloca(A); \
-                                array[0]= 1; \
-                                array[0]++; \
-                                array[0] ? array[0]++ : array[0]--; \
-                              } while(0)
+#ifndef DBUG_OFF
+static int dbug_json_check_min_stack_requirement()
+{
+  my_error(ER_STACK_OVERRUN_NEED_MORE, MYF(ME_FATAL),
+           my_thread_stack_size, my_thread_stack_size, STACK_MIN_SIZE);
+  return 1;
+}
+#endif
 
 class table_function_handlerton
 {
@@ -118,15 +111,12 @@ int get_disallowed_table_deps_for_list(MEM_ROOT *mem_root,
   NESTED_JOIN *nested_join;
   List_iterator<TABLE_LIST> li(*join_list);
 
-  DBUG_EXECUTE_IF("json_check_min_stack_requirement",
-                  {
-                    long arbitrary_var;
-                    long stack_used_up= (available_stack_size(current_thd->thread_stack, &arbitrary_var));
-                    ALLOCATE_MEM_ON_STACK(my_thread_stack_size-stack_used_up-STACK_MIN_SIZE);
-                  });
   if (check_stack_overrun(current_thd, STACK_MIN_SIZE , NULL))
     return 1;
 
+  DBUG_EXECUTE_IF("json_check_min_stack_requirement",
+                  return dbug_json_check_min_stack_requirement(););
+
   while ((table= li++))
   {
     if ((nested_join= table->nested_join))
@@ -1331,15 +1321,12 @@ static void add_extra_deps(List<TABLE_LIST> *join_list, table_map deps)
   TABLE_LIST *table;
   List_iterator<TABLE_LIST> li(*join_list);
 
-  DBUG_EXECUTE_IF("json_check_min_stack_requirement",
-                  {
-                    long arbitrary_var;
-                    long stack_used_up= (available_stack_size(current_thd->thread_stack, &arbitrary_var));
-                    ALLOCATE_MEM_ON_STACK(my_thread_stack_size-stack_used_up-STACK_MIN_SIZE);
-                  });
   if (check_stack_overrun(current_thd, STACK_MIN_SIZE , NULL))
     return;
 
+  DBUG_EXECUTE_IF("json_check_min_stack_requirement",
+                  dbug_json_check_min_stack_requirement(); return;);
+
   while ((table= li++))
   {
     table->dep_tables |= deps;
@@ -1428,14 +1415,10 @@ table_map add_table_function_dependencies(List<TABLE_LIST> *join_list,
   table_map res= 0;
   List_iterator<TABLE_LIST> li(*join_list);
 
-  DBUG_EXECUTE_IF("json_check_min_stack_requirement",
-                  {
-                    long arbitrary_var;
-                    long stack_used_up= (available_stack_size(current_thd->thread_stack, &arbitrary_var));
-                    ALLOCATE_MEM_ON_STACK(my_thread_stack_size-stack_used_up-STACK_MIN_SIZE);
-                  });
   if ((res=check_stack_overrun(current_thd, STACK_MIN_SIZE , NULL)))
     return res;
+  DBUG_EXECUTE_IF("json_check_min_stack_requirement",
+                  return dbug_json_check_min_stack_requirement(););
 
   // Recursively compute extra dependencies
   while ((table= li++))



 Comments   
Comment by Rex Johnston [ 2024-01-22 ]

Also crashes using gcc 12.3.0.

Generated at Thu Feb 08 10:37:11 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.