Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug#104121: Ensure that PREPAREd statements are shown correctly in PERFORMANCE_SCHEMA.THREADS table #5060

Open
wants to merge 1 commit into
base: 8.0
Choose a base branch
from

Conversation

uoysip
Copy link

@uoysip uoysip commented May 31, 2023

This reverts the deletion of several lines from sql/sql_class.cc in afcaac4#diff-65c68b03ca220f14939f817d3c9cfe7767580750f9ef47876b383780a0fd82c3L4357.

That deletion caused a regression: PERFORMANCE_SCHEMA.THREADS is no longer correctly updated while executing a PREPAREd statement. Detailed example in https://bugs.mysql.com/bug.php?id=104121

The one-line patch proposed as a fix in https://bugs.mysql.com/bug.php?id=104121#c513825 has the unwanted side effect of overwriting performance_schema.events_statements_history.SQL_TEXT (causing it to appear as NULL).

We verified that this change does not undo the intended effect of the original commit. Passwords (still) do not leak into the PERFORMANCE_SCHEMA.THREADS table with this change:

-- In one session (connected as `root`@`localhost`):
mysql> CREATE USER alice IDENTIFIED BY 'thisisapassword';
Query OK, 0 rows affected (0.01 sec)

-- In another session (connected as `root`@`localhost`)
mysql> select PROCESSLIST_INFO from performance_schema.threads;
+-----------------------------------------------------------------+
| PROCESSLIST_INFO                                                |
+-----------------------------------------------------------------+
...
| CREATE USER 'alice'@'%' IDENTIFIED BY <secret>                   |
...
| select PROCESSLIST_INFO from performance_schema.threads         |
+-----------------------------------------------------------------+
53 rows in set (0.00 sec)

…RFORMANCE_SCHEMA.THREADS table

This reverts the deletion of several lines from `sql/sql_class.cc` in
percona@afcaac4#diff-65c68b03ca220f14939f817d3c9cfe7767580750f9ef47876b383780a0fd82c3L4357.

That deletion caused a regression: `PERFORMANCE_SCHEMA.THREADS` is no longer
correctly updated while executing a `PREPARE`d statement.  Detailed example
in https://bugs.mysql.com/bug.php?id=104121

The one-line patch proposed as a fix in
https://bugs.mysql.com/bug.php?id=104121#c513825 has the unwanted side
effect of overwriting `performance_schema.events_statements_history.SQL_TEXT`
(causing it to appear as `NULL`).

We verified that this change *does not* undo the intended effect of the
original commit. Passwords (still) do not leak into the
`PERFORMANCE_SCHEMA.THREADS` table with this change:

    -- In one session (connected as `root`@`localhost`):
    mysql> CREATE USER alice IDENTIFIED BY 'thisisapassword';
    Query OK, 0 rows affected (0.01 sec)

    -- In another session (connected as `root`@`localhost`)
    mysql> select PROCESSLIST_INFO from performance_schema.threads;
    +-----------------------------------------------------------------+
    | PROCESSLIST_INFO                                                |
    +-----------------------------------------------------------------+
    ...
    | CREATE USER 'alice'@'%' IDENTIFIED BY <secret>                   |
    ...
    | select PROCESSLIST_INFO from performance_schema.threads         |
    +-----------------------------------------------------------------+
    53 rows in set (0.00 sec)

All new code of the whole pull request, including one or several files that are
either new files or modified ones, are contributed under the BSD-new license. I
am contributing on behalf of my employer Amazon Web Services, Inc.
@uoysip uoysip force-pushed the bug-1104121-fix-prepared-statements-in-PERFORMANCE_SCHEMA.THREADS branch from 617e127 to a11734d Compare June 1, 2023 18:07
@robinnewhouse
Copy link

Could we request a review of this PR? Thank you.

@VarunNagaraju
Copy link
Contributor

Hi @uoysip, Thanks a lot for reporting this issue. We were able to reproduce the issue using tpcc-mysql as described in the upstream bug. A JIRA ticket has been created to track it's progress. https://perconadev.atlassian.net/browse/PS-9183
As far as https://bugs.mysql.com/bug.php?id=99039 is concerned. It seems to be fixed by WL#9090 Reimplement SHOW PROCESSLIST(mysql/mysql-server@4095e087545769c193bd0dc17b5449e5fbe17606). So, The 'commit' in PROCESSLIST_INFO is NOT the previously executed statement on the same thread. Perhaps, it is the statement currently being executed on the thread.

I have a few questions regarding the PR

  • After Bug#99039 is fixed, performance_schema.threads doesn't show the last executed query in PROCESSLIST_INFO. Instead, it shows the query being executed currently. So, how is CREATE USER 'alice'@'%' IDENTIFIED BY query visible in the example shown in the description?
-- In one session (connected as `root`@`localhost`):
mysql> CREATE USER alice IDENTIFIED BY 'thisisapassword';
Query OK, 0 rows affected (0.01 sec)

-- In another session (connected as `root`@`localhost`)
mysql> select PROCESSLIST_INFO from performance_schema.threads;
+-----------------------------------------------------------------+
| PROCESSLIST_INFO                                                |
+-----------------------------------------------------------------+
...
| CREATE USER 'alice'@'%' IDENTIFIED BY <secret>                   |
...
| select PROCESSLIST_INFO from performance_schema.threads         |
+-----------------------------------------------------------------+
53 rows in set (0.00 sec)
  • In the patches I can see submitted on the upstream ticket, I don't see the part where upstream complains the password is being leaked to stderr. Was a different patch contributed privately?
+  if (info != nullptr) {
+    fprintf(stderr, "THREAD.PROCESSLIST_INFO = %.*s\n", info_len, info);
+  }
+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants