Bug #16119355: PREPARED STATEMENT: READ OF FREED MEMORY WITH

                                 STRING CONVERSION FUNCTIONS
            
Problem:
While executing the prepared statement, user variable is
set to memory which would be freed at the end of
execution.
If the statement is executed again, valgrind throws
error when accessing this pointer.
                  
Analysis:
                
1. First time when Item_func_set_user_var::check is called,
   memory is allocated for "value" to store the result.
   (In the call to copy_if_not_alloced).
2. While sending the result, Item_func_set_user_var::check
   is called again. But, this time, its called with
   "use_result_field" set to true. 
   As a result, we call result_field->val_str(&value).
3. Here memory allocated for "value" gets freed. And "value"
   gets set to "result_field", with "str_length" being that of
   result_field's.
4. In the call to JOIN::cleanup, result_field's memory gets
   freed as this is allocated in a chunk as part of the
   temporary table which is needed to execute the query.
5. Next time, when execute of the same statement is called,
   "value" will be set to memory which is already freed.
   Valgrind error occurs as "str_length" is positive 
   (set at Step 3)
                  
Note that user variables list is stored as part of the Lex object
in set_var_list. Hence the persistance across executions.
            
Solution:
Patch for Bug#11764371 fixed in mysql-5.6+ fixes this problem 
as well.So backporting the same.
            
In the solution for Bug#11764371, we create another object of 
user_var and repoint it to temp_table's field. As a result while 
deleting the alloced buffer in Step 3, since the cloned object 
does not own the buffer, deletion will not happen.
So at step 5 when we execute the statement second time, the 
original object will be used and since deletion did not happen 
valgrind will not complain about dangling pointer.

2 files changed, 71 insertions, 40 deletions

diff --git a/sql/item_func.h b/sql/item_func.h
index 22fb38e176d..c3ef3d12d71 100644
--- a/sql/item_func.h
+++ b/sql/item_func.h
@@ -1391,6 +1391,13 @@ public:
     :Item_func(b), cached_result_type(INT_RESULT),
      entry(NULL), entry_thread_id(0), name(a)
   {}
+  Item_func_set_user_var(THD *thd, Item_func_set_user_var *item)
+    :Item_func(thd, item), cached_result_type(item->cached_result_type),
+    entry(item->entry), entry_thread_id(item->entry_thread_id),
+    value(item->value), decimal_buff(item->decimal_buff),
+    null_item(item->null_item), save_result(item->save_result),
+    name(item->name)
+  {}
 
   enum Functype functype() const { return SUSERVAR_FUNC; }
   double val_real();
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 7b1a8cf9e82..7507f430eb7 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -15779,64 +15779,88 @@ change_to_use_tmp_fields(THD *thd, Item **ref_pointer_array,
   res_selected_fields.empty();
   res_all_fields.empty();
 
-  uint i, border= all_fields.elements - elements;
-  for (i= 0; (item= it++); i++)
+  uint border= all_fields.elements - elements;
+  for (uint i= 0; (item= it++); i++)
   {
     Field *field;
-
-    if ((item->with_sum_func && item->type() != Item::SUM_FUNC_ITEM) ||
-        (item->type() == Item::FUNC_ITEM &&
-         ((Item_func*)item)->functype() == Item_func::SUSERVAR_FUNC))
+    if (item->with_sum_func && item->type() != Item::SUM_FUNC_ITEM)
       item_field= item;
-    else
+    else if (item->type() == Item::FIELD_ITEM)
+      item_field= item->get_tmp_table_item(thd);
+    else if (item->type() == Item::FUNC_ITEM &&
+             ((Item_func*)item)->functype() == Item_func::SUSERVAR_FUNC)
     {
-      if (item->type() == Item::FIELD_ITEM)
+      field= item->get_tmp_table_field();
+      if( field != NULL)
       {
-        item_field= item->get_tmp_table_item(thd);
+        /*
+          Replace "@:=<expression>" with "@:=<tmp table column>". Otherwise, we
+          would re-evaluate <expression>, and if expression were a subquery, this
+          would access already-unlocked tables.
+         */
+        Item_func_set_user_var* suv=
+          new Item_func_set_user_var(thd, (Item_func_set_user_var*) item);
+        Item_field *new_field= new Item_field(field);
+        if (!suv || !new_field)
+          DBUG_RETURN(true);                  // Fatal error
+        /*
+          We are replacing the argument of Item_func_set_user_var after its value
+          has been read.  The argument's null_value should be set by now, so we
+          must set it explicitly for the replacement argument since the null_value
+          may be read without any preceeding call to val_*().
+         */
+        new_field->update_null_value();
+        List<Item> list;
+        list.push_back(new_field);
+        suv->set_arguments(list);
+        item_field= suv;
       }
-      else if ((field= item->get_tmp_table_field()))
+      else
+        item_field= item;
+    }
+    else if ((field= item->get_tmp_table_field()))
+    {
+      if (item->type() == Item::SUM_FUNC_ITEM && field->table->group)
+        item_field= ((Item_sum*) item)->result_item(field);
+      else
+        item_field= (Item*) new Item_field(field);
+      if (!item_field)
+        DBUG_RETURN(true);                    // Fatal error
+
+      if (item->real_item()->type() != Item::FIELD_ITEM)
+        field->orig_table= 0;
+      item_field->name= item->name;
+      if (item->type() == Item::REF_ITEM)
       {
-        if (item->type() == Item::SUM_FUNC_ITEM && field->table->group)
-          item_field= ((Item_sum*) item)->result_item(field);
-        else
-          item_field= (Item*) new Item_field(field);
-        if (!item_field)
-          DBUG_RETURN(TRUE);                    // Fatal error
-
-        if (item->real_item()->type() != Item::FIELD_ITEM)
-          field->orig_table= 0;
-        item_field->name= item->name;
-        if (item->type() == Item::REF_ITEM)
-        {
-          Item_field *ifield= (Item_field *) item_field;
-          Item_ref *iref= (Item_ref *) item;
-          ifield->table_name= iref->table_name;
-          ifield->db_name= iref->db_name;
-        }
+        Item_field *ifield= (Item_field *) item_field;
+        Item_ref *iref= (Item_ref *) item;
+        ifield->table_name= iref->table_name;
+        ifield->db_name= iref->db_name;
+      }
 #ifndef DBUG_OFF
-        if (!item_field->name)
-        {
-          char buff[256];
-          String str(buff,sizeof(buff),&my_charset_bin);
-          str.length(0);
-          item->print(&str, QT_ORDINARY);
-          item_field->name= sql_strmake(str.ptr(),str.length());
-        }
-#endif
+      if (!item_field->name)
+      {
+        char buff[256];
+        String str(buff,sizeof(buff),&my_charset_bin);
+        str.length(0);
+        item->print(&str, QT_ORDINARY);
+        item_field->name= sql_strmake(str.ptr(),str.length());
       }
-      else
-        item_field= item;
+#endif
     }
+    else
+      item_field= item;
+
     res_all_fields.push_back(item_field);
     ref_pointer_array[((i < border)? all_fields.elements-i-1 : i-border)]=
       item_field;
   }
 
   List_iterator_fast<Item> itr(res_all_fields);
-  for (i= 0; i < border; i++)
+  for (uint i= 0; i < border; i++)
     itr++;
   itr.sublist(res_selected_fields, elements);
-  DBUG_RETURN(FALSE);
+  DBUG_RETURN(false);
 }