Skip to content

Conversation

@bsrikanth-mariadb
Copy link
Contributor

@bsrikanth-mariadb bsrikanth-mariadb commented Sep 26, 2025

MDEV-35815: use-after-poison_in_get_hash_symbol

When a PREPARED statment is executed twice, it is crashing during the
second execution.

For the following query: -
PREPARE stmt FROM 'SELECT tbl.subject AS fld FROM v1 AS tbl
GROUP BY fld HAVING 0 AND fld != 1';

Here v1 is a view on top of a table having a single longtext column.
The column "subject" in v1 is defined as an expression using "ifnull"
function.

During the first execution:
The Item "fld" in the HAVING clause, is an Item_ref instance with name
"fld", where it has a ref to another Item_ref instance with the same name "fld"
which in turn has a reference to another Item with name "subject".
In the join prepare stage, while in the call to setup_copy_fields() from
make_aggr_tables_info(), the name field of the last Item instance
is replaced with the name in second Item_ref instance. However, after
the first execution is done, the meory for the second Item_ref is freed.

Now, during the second execution of the same statement, when trying to
access name field Item_ref instance, it was getting crashed, because,
that memory was already freed earlier.

The fix is to allocate a new memory in the stmt_arena for the second
Item_ref instance's name->str field.

@CLAassistant
Copy link

CLAassistant commented Sep 26, 2025

CLA assistant check
All committers have signed the CLA.

@bsrikanth-mariadb bsrikanth-mariadb changed the base branch from main to 10.11 September 26, 2025 06:43
@spetrunia
Copy link
Member

@bsrikanth-mariadb , did you see the tests failing with:

CURRENT_TEST: main.view
--- /home/buildbot/amd64-debian-12/build/mysql-test/main/view.result	2025-09-26 06:47:13.000000000 +0000
+++ /home/buildbot/amd64-debian-12/build/mysql-test/main/view.reject	2025-09-26 06:54:03.068000000 +0000
@@ -6891,7 +6891,7 @@
 SELECT v.id, v.foo AS bar  FROM v1 v
 WHERE id = 2
 GROUP BY v.id;
-id	bar
+id	foo
 2	2
 Drop View v1;
 Drop table t1;

@spetrunia spetrunia self-requested a review October 6, 2025 08:18
@spetrunia
Copy link
Member

Notes from discussion:

One Item_ref refers to another, not sure if this is ok or not.

Copying of Item's name was introduced by:

commit 82da98556cf58f0fbb43c82e9c6ae1a887b6cf3d
Author:	Oleksandr Byelkin <[email protected]>  Mon Feb 25 16:57:08 2019
Committer:	Oleksandr Byelkin <[email protected]>  Tue Feb 26 09:32:02 2019

MDEV-18605: Loss of column aliases by using view and group

Preserv column name with copy fields even if it is function and Co.

@bsrikanth-mariadb
Copy link
Contributor Author

@bsrikanth-mariadb , did you see the tests failing with:

CURRENT_TEST: main.view
--- /home/buildbot/amd64-debian-12/build/mysql-test/main/view.result	2025-09-26 06:47:13.000000000 +0000
+++ /home/buildbot/amd64-debian-12/build/mysql-test/main/view.reject	2025-09-26 06:54:03.068000000 +0000
@@ -6891,7 +6891,7 @@
 SELECT v.id, v.foo AS bar  FROM v1 v
 WHERE id = 2
 GROUP BY v.id;
-id	bar
+id	foo
 2	2
 Drop View v1;
 Drop table t1;

Yes Sergei, I am aware of this. Hence I haven't added any reviewers yet. Still it is WIP. Have to sync up with Sanja on this.

@bsrikanth-mariadb bsrikanth-mariadb force-pushed the 10.11-MDEV-35815-use-after-poison_in_get_hash_symbol branch 2 times, most recently from 6f36639 to f28809b Compare October 8, 2025 09:31
@spetrunia
Copy link
Member

  • Did you discuss this with @sanja-byelkin ?
  • Please move away the example from using mysql.user to using your own table and VIEW. Using a table from mysql.* makes one think that is somehow special. System tables are sometimes treated in a special way but as far as this bug concerned this is just a table and view.
  • You enable optimizer trace in the test. Please restore its setting back.
  • Is checking for ROOT_FLAG_READ_ONLY reliable? AFAIU ROOT_FLAG_READ_ONLY is only set and checked in debug builds.

@bsrikanth-mariadb bsrikanth-mariadb force-pushed the 10.11-MDEV-35815-use-after-poison_in_get_hash_symbol branch from f28809b to f70d18d Compare October 9, 2025 05:22
@bsrikanth-mariadb
Copy link
Contributor Author

  • Did you discuss this with @sanja-byelkin ?
  • Please move away the example from using mysql.user to using your own table and VIEW. Using a table from mysql.* makes one think that is somehow special. System tables are sometimes treated in a special way but as far as this bug concerned this is just a table and view.
  • You enable optimizer trace in the test. Please restore its setting back.
  • Is checking for ROOT_FLAG_READ_ONLY reliable? AFAIU ROOT_FLAG_READ_ONLY is only set and checked in debug builds.

Sure, made the suggested changes. Also, refined the logic in sql_select.cc with inputs from @sanja-byelkin

@bsrikanth-mariadb bsrikanth-mariadb force-pushed the 10.11-MDEV-35815-use-after-poison_in_get_hash_symbol branch 2 times, most recently from 6e87c47 to d1074a1 Compare October 14, 2025 09:59
When a PREPARED statment is executed twice, it is crashing during the
second execution.

For the following query: -
PREPARE stmt FROM 'SELECT tbl.subject AS fld FROM v1 AS tbl
GROUP BY fld HAVING 0 AND fld != 1';

Here v1 is a view on top of a table having a single longtext column.
The column "subject" in v1 is defined as an expression using "ifnull"
function.

During the first execution:
The Item "fld" in the HAVING clause, is an Item_ref instance with name
"fld", where it has a ref to another Item_ref instance with the same name "fld"
which in turn has a reference to another Item with name "subject".
In the join prepare stage, while in the call to setup_copy_fields() from
make_aggr_tables_info(), the name field of the last Item instance
is replaced with the name in second Item_ref instance. However, after
the first execution is done, the meory for the second Item_ref is freed.

Now, during the second execution of the same statement, when trying to
access name field Item_ref instance, it was getting crashed, because,
that memory was already freed earlier.

The fix is to allocate a new memory in the stmt_arena for the second
Item_ref instance's name->str field.
@bsrikanth-mariadb bsrikanth-mariadb force-pushed the 10.11-MDEV-35815-use-after-poison_in_get_hash_symbol branch from d1074a1 to cc56eeb Compare October 14, 2025 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants