Commit 266dd9c0 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 2ec9dcf6
......@@ -1391,6 +1391,14 @@ 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();
longlong val_int();
......
......@@ -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);
}
......
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