[MDEV-24281] Reading from freed memory when running main.view with --ps-protocol Created: 2020-11-25 Updated: 2022-03-29 Resolved: 2022-03-24 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Prepared Statements, Tests |
| Affects Version/s: | 10.3, 10.4, 10.5, 10.6, 10.7 |
| Fix Version/s: | 10.3.35, 10.4.25, 10.5.16, 10.6.8, 10.7.4 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Alice Sherepa | Assignee: | Igor Babaev |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | affects-tests | ||
| Description |
|
test case from main.view +ps-protocol
|
| Comments |
| Comment by Igor Babaev [ 2022-03-18 ] | |||||||||||||||||||||||||||||||||||
|
Let's consider the test case
and see how we come to the reported problem when the server executes the statement
1. When the server executes the statement
it parses the command
Doing so it calls the function st_select_lex::add_item_to_list() for all 3 elements of the select list
For the second item we have:
For the third item we have:
Yet for the third item we additionally call Item::set_name() that changes item->name
All these items and names are allocated in lex->thd->mem_root used for the query
In my case the address of this mem_root was:
| |||||||||||||||||||||||||||||||||||
| Comment by Igor Babaev [ 2022-03-18 ] | |||||||||||||||||||||||||||||||||||
|
2. We come to execution of the statement:
that brings us to a call of the function Prepared_statement::execute(). The latter brings us to a call of mysql_create_view(). This function calls check_duplicate_names() for the specification of the view:
Here the function sees that the name for the first item is the name for the second item and it generates the new name for the first item as 'My_exp_s1'. At the same time the orig_name for the first item is set to 's1'. Then the function notices that the new name for the first item coincides with the name for the third item and it renames the first item for 'My_exp_1_s1' setting orig_name for the first to ''My_exp_s1'.
At the end of the above mentioned invocation of Prepared_statement::execute() the function Prepared_statement::cleanup_stmt() is called that brings us to the statement
First it cleans up the item
Here nothing is actually done as item->orig_name == 0.
the function Item_field::cleanup() is called.
Item_basic_constant::cleanup() is called that sets item->name to item->orig_name. | |||||||||||||||||||||||||||||||||||
| Comment by Igor Babaev [ 2022-03-21 ] | |||||||||||||||||||||||||||||||||||
|
3. At the end of execution of the command
the mem_root used for this execution is freed. This is the root
After we have done with the execution of the statement
we move to the execution of the statement
The function Prepared_statement::deallocate() is called that calls Statement_map::erase() that calls indirectly Prepared_statement::~Prepared_statement(). The latter calls the function Query_arena::free_items() that frees the item allocated when the CREATE VIEW statement was parsed.
Item_basic_constant::cleanup() is called. As orig_name is 0 for it we do nothing.
Item_field::cleanup() works fine for it.
Item_basic_constant::cleanup() is called. As orig_name is
the code
is executed. Yet the value of the orig_name was allocated in the mem_root that has been already freed. | |||||||||||||||||||||||||||||||||||
| Comment by Igor Babaev [ 2022-03-21 ] | |||||||||||||||||||||||||||||||||||
|
A fix would be not to reset the value of orig_name in the function make_unique_view_field_name()
In this case we know that orig_name was set only once and it set to the value of name_str that is allocated in the mem_root used for the prepared statement.
is freed in the destructor of the prepared statement the value of the orig_name is
and this value is allocated in the mem_root that is used when CREATE VIEW is parsed. | |||||||||||||||||||||||||||||||||||
| Comment by Igor Babaev [ 2022-03-22 ] | |||||||||||||||||||||||||||||||||||
|
If I add another another 'execute stmt' to the test case:
then I have additionally problems of reading from freed memory when executing this second 'execute stmt'. For example I have additionally this invalid reading:
| |||||||||||||||||||||||||||||||||||
| Comment by Oleksandr Byelkin [ 2022-03-23 ] | |||||||||||||||||||||||||||||||||||
|
OK to push | |||||||||||||||||||||||||||||||||||
| Comment by Igor Babaev [ 2022-03-23 ] | |||||||||||||||||||||||||||||||||||
|
Let's figure out whether we need a similar change in the following code of make_valid_column_names()
Consider the following test case:
See what happens the item
is processed in make_valid_column_names(). | |||||||||||||||||||||||||||||||||||
| Comment by Igor Babaev [ 2022-03-24 ] | |||||||||||||||||||||||||||||||||||
|
A fix for this bug was pushed into 10.3 |