BUG#18037: Fix stack corruption in THD::rollback_item_tree_changes().

  
Stored procedure execution sometimes placed the address of auto variables
in the list of Item changes to undo in THD::rollback_item_tree_changes().
This could cause stack corruption.

5 files changed, 28 insertions, 25 deletions

diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index 6a7676c7bf2..d1da6f7d3b3 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -310,11 +310,13 @@ sp_prepare_func_item(THD* thd, Item **it_addr)
 */
 
 bool
-sp_eval_expr(THD *thd, Field *result_field, Item *expr_item)
+sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr)
 {
+  Item *expr_item;
+
   DBUG_ENTER("sp_eval_expr");
 
-  if (!(expr_item= sp_prepare_func_item(thd, &expr_item)))
+  if (!(expr_item= sp_prepare_func_item(thd, expr_item_ptr)))
     DBUG_RETURN(TRUE);
 
   bool err_status= FALSE;
@@ -1269,7 +1271,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount,
       param_values[i]= Item_cache::get_cache(argp[i]->result_type());
       param_values[i]->store(argp[i]);
 
-      if (nctx->set_variable(thd, i, param_values[i]))
+      if (nctx->set_variable(thd, i, (struct Item **)&(param_values[i])))
       {
         err_status= TRUE;
         break;
@@ -1467,7 +1469,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
         Item_null *null_item= new Item_null();
 
         if (!null_item ||
-            nctx->set_variable(thd, i, null_item))
+            nctx->set_variable(thd, i, (struct Item **)&null_item))
         {
           err_status= TRUE;
           break;
@@ -1475,7 +1477,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
       }
       else
       {
-        if (nctx->set_variable(thd, i, *it_args.ref()))
+        if (nctx->set_variable(thd, i, it_args.ref()))
         {
           err_status= TRUE;
           break;
@@ -1531,7 +1533,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
       {
         if (octx->set_variable(thd,
                                ((Item_splocal*) arg_item)->get_var_idx(),
-                               nctx->get_item(i)))
+                               nctx->get_item_addr(i)))
         {
           err_status= TRUE;
           break;
@@ -1543,15 +1545,15 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
 
 	if (guv)
 	{
-	  Item *item= nctx->get_item(i);
+	  Item **item= nctx->get_item_addr(i);
 	  Item_func_set_user_var *suv;
 
-	  suv= new Item_func_set_user_var(guv->get_name(), item);
+	  suv= new Item_func_set_user_var(guv->get_name(), *item);
 	  /*
             Item_func_set_user_var is not fixed after construction,
             call fix_fields().
 	  */
-          if ((err_status= test(!suv || suv->fix_fields(thd, &item) ||
+          if ((err_status= test(!suv || suv->fix_fields(thd, item) ||
                                 suv->check() || suv->update())))
             break;
 	}
@@ -2328,7 +2330,7 @@ sp_instr_set::execute(THD *thd, uint *nextp)
 int
 sp_instr_set::exec_core(THD *thd, uint *nextp)
 {
-  int res= thd->spcont->set_variable(thd, m_offset, m_value);
+  int res= thd->spcont->set_variable(thd, m_offset, &m_value);
 
   if (res && thd->spcont->found_handler_here())
   {
@@ -2603,7 +2605,7 @@ sp_instr_freturn::exec_core(THD *thd, uint *nextp)
     do it in scope of execution the current context/block.
   */
 
-  return thd->spcont->set_return_value(thd, m_value);
+  return thd->spcont->set_return_value(thd, &m_value);
 }
 
 void
@@ -3047,7 +3049,7 @@ sp_instr_set_case_expr::execute(THD *thd, uint *nextp)
 int
 sp_instr_set_case_expr::exec_core(THD *thd, uint *nextp)
 {
-  int res= thd->spcont->set_case_expr(thd, m_case_expr_id, m_case_expr);
+  int res= thd->spcont->set_case_expr(thd, m_case_expr_id, &m_case_expr);
 
   if (res &&
       !thd->spcont->get_case_expr(m_case_expr_id) &&
@@ -3061,7 +3063,7 @@ sp_instr_set_case_expr::exec_core(THD *thd, uint *nextp)
     Item *null_item= new Item_null();
     
     if (!null_item ||
-        thd->spcont->set_case_expr(thd, m_case_expr_id, null_item))
+        thd->spcont->set_case_expr(thd, m_case_expr_id, &null_item))
     {
       /* If this also failed, we have to abort. */
 
diff --git a/sql/sp_head.h b/sql/sp_head.h
index 6a9cf97d739..d5f49d8a964 100644
--- a/sql/sp_head.h
+++ b/sql/sp_head.h
@@ -1169,6 +1169,6 @@ Item *
 sp_prepare_func_item(THD* thd, Item **it_addr);
 
 bool
-sp_eval_expr(THD *thd, Field *result_field, Item *expr_item);
+sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr);
 
 #endif /* _SP_HEAD_H_ */
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc
index 38b6de0e75a..3bc27a029d0 100644
--- a/sql/sp_rcontext.cc
+++ b/sql/sp_rcontext.cc
@@ -150,7 +150,7 @@ sp_rcontext::init_var_items()
 
 
 bool
-sp_rcontext::set_return_value(THD *thd, Item *return_value_item)
+sp_rcontext::set_return_value(THD *thd, Item **return_value_item)
 {
   DBUG_ASSERT(m_return_value_fld);
 
@@ -279,14 +279,14 @@ sp_rcontext::pop_cursors(uint count)
 
 
 int
-sp_rcontext::set_variable(THD *thd, uint var_idx, Item *value)
+sp_rcontext::set_variable(THD *thd, uint var_idx, Item **value)
 {
   return set_variable(thd, m_var_table->field[var_idx], value);
 }
 
 
 int
-sp_rcontext::set_variable(THD *thd, Field *field, Item *value)
+sp_rcontext::set_variable(THD *thd, Field *field, Item **value)
 {
   if (!value)
   {
@@ -478,9 +478,10 @@ sp_rcontext::create_case_expr_holder(THD *thd, Item_result result_type)
 */
 
 int
-sp_rcontext::set_case_expr(THD *thd, int case_expr_id, Item *case_expr_item)
+sp_rcontext::set_case_expr(THD *thd, int case_expr_id, Item **case_expr_item_ptr)
 {
-  if (!(case_expr_item= sp_prepare_func_item(thd, &case_expr_item)))
+  Item *case_expr_item= sp_prepare_func_item(thd, case_expr_item_ptr);
+  if (!case_expr_item)
     return TRUE;
 
   if (!m_case_expr_holders[case_expr_id] ||
@@ -542,7 +543,7 @@ bool Select_fetch_into_spvars::send_data(List<Item> &items)
   */
   for (; spvar= spvar_iter++, item= item_iter++; )
   {
-    if (thd->spcont->set_variable(thd, spvar->offset, item))
+    if (thd->spcont->set_variable(thd, spvar->offset, &item))
       return TRUE;
   }
   return FALSE;
diff --git a/sql/sp_rcontext.h b/sql/sp_rcontext.h
index 20aaea3b7c1..30521f6da84 100644
--- a/sql/sp_rcontext.h
+++ b/sql/sp_rcontext.h
@@ -91,7 +91,7 @@ class sp_rcontext : public Sql_alloc
   ~sp_rcontext();
 
   int
-  set_variable(THD *thd, uint var_idx, Item *value);
+  set_variable(THD *thd, uint var_idx, Item **value);
 
   Item *
   get_item(uint var_idx);
@@ -100,7 +100,7 @@ class sp_rcontext : public Sql_alloc
   get_item_addr(uint var_idx);
 
   bool
-  set_return_value(THD *thd, Item *return_value_item);
+  set_return_value(THD *thd, Item **return_value_item);
 
   inline bool
   is_return_value_set() const
@@ -200,7 +200,7 @@ class sp_rcontext : public Sql_alloc
   */
 
   int
-  set_case_expr(THD *thd, int case_expr_id, Item *case_expr_item);
+  set_case_expr(THD *thd, int case_expr_id, Item **case_expr_item_ptr);
 
   Item *
   get_case_expr(int case_expr_id);
@@ -254,7 +254,7 @@ private:
 
   Item_cache *create_case_expr_holder(THD *thd, Item_result result_type);
 
-  int set_variable(THD *thd, Field *field, Item *value);
+  int set_variable(THD *thd, Field *field, Item **value);
 }; // class sp_rcontext : public Sql_alloc
 
 
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 026c3e0d515..65fd4d3ac19 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -1877,7 +1877,7 @@ bool select_dumpvar::send_data(List<Item> &items)
       if ((yy=var_li++)) 
       {
 	if (thd->spcont->set_variable(current_thd, yy->get_var_idx(),
-                                      *it.ref()))
+                                      it.ref()))
 	  DBUG_RETURN(1);
       }
     }