Commit 4bd94e7d authored by Chaithra Gopalareddy's avatar Chaithra Gopalareddy

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.


sql/item_func.h:
  Add constructors.
sql/sql_select.cc:
  Change user variable assignment functions to read from fields after
  tables have been unlocked.
parent 0c903fb5
......@@ -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();
......
......@@ -15779,20 +15779,44 @@ 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
item_field= item;
}
else if ((field= item->get_tmp_table_field()))
{
......@@ -15801,7 +15825,7 @@ change_to_use_tmp_fields(THD *thd, Item **ref_pointer_array,
else
item_field= (Item*) new Item_field(field);
if (!item_field)
DBUG_RETURN(TRUE); // Fatal error
DBUG_RETURN(true); // Fatal error
if (item->real_item()->type() != Item::FIELD_ITEM)
field->orig_table= 0;
......@@ -15826,17 +15850,17 @@ change_to_use_tmp_fields(THD *thd, Item **ref_pointer_array,
}
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);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment