Hi, Alexey! On Aug 14, Alexey Botchkov wrote:

revision-id: e2a82094332 (mariadb-10.4.7-123-ge2a82094332) parent(s): 691654721b3 author: Alexey Botchkov <holyfoot@mariadb.com> committer: Alexey Botchkov <holyfoot@mariadb.com> timestamp: 2019-08-07 11:49:54 +0400 message:

MENT-253 Server Audit v2 does not work with PS protocol: server crashes in filter_query_type.

Set the thd->query_string where possible. server_audit2 should not crash when gets empty event->query.

--- plugin/server_audit2/server_audit.c | 15 ++++++++++++--- sql/sql_prepare.cc | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-)

Test case? With normal and error code paths?

diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 74723d5bd91..d2cb4e9b372 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -2600,6 +2600,14 @@ void mysqld_stmt_prepare(THD *thd, const char *packet, uint packet_length)

if (stmt->prepare(packet, packet_length)) { + /* + Set the thd->query_string with the current query so the + audit plugin gets the meaningful notification.

say that it's an error case. Like Prepare failed. Set the thd->query_string with the current query so the audit plugin gets the meaningful notification when it gets the error notification.

+ */ + if (alloc_query(thd, stmt->query_string.str(), stmt->query_string.length())) + { + thd->set_query(0, 0); + } /* Statement map deletes statement on erase */ thd->stmt_map.erase(stmt); thd->clear_last_stmt(); @@ -3184,6 +3192,13 @@ static void mysql_stmt_execute_common(THD *thd, char llbuf[22]; my_error(ER_UNKNOWN_STMT_HANDLER, MYF(0), static_cast<int>(sizeof(llbuf)), llstr(stmt_id, llbuf), "mysqld_stmt_execute"); + /* + Set thd->query_string with the stmt_id so the + audit plugin gets the meaningful notification.

Same here.

+ */ + if (alloc_query(thd, llbuf, strlen(llbuf))) + thd->set_query(0, 0); + DBUG_VOID_RETURN; } stmt->read_types= read_types; @@ -3939,6 +3954,13 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) DBUG_RETURN(TRUE); }

+ /* + We'd like to have thd->query to be set to the actual query + after the function ends. + This value will be sent to audit pulgins later.

1. typo: "pulgins" 2. Mention explicitly that "query string is allocated in the stmt arena, not in the thd arena. But it's safe here, because stmt always has a longer lifetime than thd arena." - or something along these lines.

+ */ + stmt_backup.query_string= thd->query_string; + old_stmt_arena= thd->stmt_arena; thd->stmt_arena= this;

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org