database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-07-17 12:02:09 +03:00
Code Activity

MDEV-17092 use-after-poison around lock_trx_handle_wait_low

Browse Source 
There was a race condition where the connection of the
victim of a KILL statement is disconnected while the
KILL statement is executing.

As a side effect of this fix, we will make XA PREPARE
transactions immune to KILL statements.

Starting with MariaDB 10.2, we have a pool of trx_t objects.
trx_free() would only free memory to the pool. We poison the
contents of freed objects in the pool in order to catch misuse.

trx_free(): Unpoison also trx->mysql_thd and trx->state.
This is to counter the poisoning of *trx in trx_pools->mem_free().
Unpoison only on AddressSanitizer or Valgrind, but not on MemorySanitizer.

Pool: Unpoison allocated objects only on AddressSanitizer or
Valgrind, but not on MemorySanitizer.

innobase_kill_query(): Properly protect trx, acquiring also
trx_sys_t::mutex and checking trx_t::mysql_thd and trx_t::state.
This commit is contained in:
Marko Mäkelä
2020-05-25 18:57:14 +03:00
parent e2c749380b
commit 5530a93f47
3 changed files with 60 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
storage/innobase/handler/ha_innodb.cc
View File

@ -5201,13 +5201,43 @@ static void innobase_kill_query(handlerton*, THD* thd, enum thd_kill_levels)
}
#endif /* WITH_WSREP */
if (trx_t* trx = thd_to_trx(thd)) {
ut_ad(trx->mysql_thd == thd);
/* Cancel a pending lock request if there are any */
lock_trx_handle_wait(trx);
}
if (trx_t* trx= thd_to_trx(thd))
{
lock_mutex_enter();
trx_sys_mutex_enter();
trx_mutex_enter(trx);
/* It is possible that innobase_close_connection() is concurrently
being executed on our victim. In that case, trx->mysql_thd would
be reset before invoking trx_free(). Even if the trx object is later
reused for another client connection or a background transaction,
its trx->mysql_thd will differ from our thd.
DBUG_VOID_RETURN;
If trx never performed any changes, nothing is really protecting
the trx_free() call or the changes of trx_t::state when the
transaction is being rolled back and trx_commit_low() is being
executed.
The function trx_allocate_for_mysql() acquires
trx_sys_t::mutex, but trx_allocate_for_background() will not.
Luckily, background transactions cannot be read-only, because
for read-only transactions, trx_start_low() will avoid acquiring
any of the trx_sys_t::mutex, lock_sys_t::mutex, trx_t::mutex before
assigning trx_t::state.
At this point, trx may have been reallocated for another client
connection, or for a background operation. In that case, either
trx_t::state or trx_t::mysql_thd should not match our expectations. */
bool cancel= trx->mysql_thd == thd && trx->state == TRX_STATE_ACTIVE &&
!trx->lock.was_chosen_as_deadlock_victim;
trx_sys_mutex_exit();
if (!cancel);
else if (lock_t *lock= trx->lock.wait_lock)
lock_cancel_waiting_and_release(lock);
lock_mutex_exit();
trx_mutex_exit(trx);
}
DBUG_VOID_RETURN;
}

10
storage/innobase/include/ut0pool.h
View File

@ -1,6 +1,7 @@
/*****************************************************************************
Copyright (c) 2013, 2014, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2018, 2020, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
@ -86,11 +87,15 @@ struct Pool {
for (Element* elem = m_start; elem != m_last; ++elem) {
ut_ad(elem->m_pool == this);
#ifdef __SANITIZE_ADDRESS__
/* Unpoison the memory for AddressSanitizer */
MEM_UNDEFINED(&elem->m_type, sizeof elem->m_type);
#endif
#ifdef HAVE_valgrind
/* Declare the contents as initialized for Valgrind;
we checked this in mem_free(). */
UNIV_MEM_VALID(&elem->m_type, sizeof elem->m_type);
#endif
Factory::destroy(&elem->m_type);
}
@ -127,13 +132,18 @@ struct Pool {
#if defined HAVE_valgrind || defined __SANITIZE_ADDRESS__
if (elem) {
# ifdef __SANITIZE_ADDRESS__
/* Unpoison the memory for AddressSanitizer */
MEM_UNDEFINED(&elem->m_type, sizeof elem->m_type);
# endif
# ifdef HAVE_valgrind
/* Declare the memory initialized for Valgrind.
The trx_t that are released to the pool are
actually initialized; we checked that by
UNIV_MEM_ASSERT_RW() in mem_free() below. */
UNIV_MEM_VALID(&elem->m_type, sizeof elem->m_type);
# endif
}
#endif

16
storage/innobase/trx/trx0trx.cc
View File

@ -408,14 +408,26 @@ trx_free(trx_t*& trx)
ut_ad(trx->will_lock == 0);
trx_pools->mem_free(trx);
#ifdef __SANITIZE_ADDRESS__
/* Unpoison the memory for innodb_monitor_set_option;
it is operating also on the freed transaction objects. */
MEM_UNDEFINED(&trx->mutex, sizeof trx->mutex);
MEM_UNDEFINED(&trx->undo_mutex, sizeof trx->undo_mutex);
/* Declare the contents as initialized for Valgrind;
we checked that it was initialized in trx_pools->mem_free(trx). */
/* For innobase_kill_connection() */
MEM_UNDEFINED(&trx->state, sizeof trx->state);
MEM_UNDEFINED(&trx->mysql_thd, sizeof trx->mysql_thd);
#endif
#ifdef HAVE_valgrind
/* Unpoison the memory for innodb_monitor_set_option;
it is operating also on the freed transaction objects.
We checked that these were initialized in
trx_pools->mem_free(trx). */
UNIV_MEM_VALID(&trx->mutex, sizeof trx->mutex);
UNIV_MEM_VALID(&trx->undo_mutex, sizeof trx->undo_mutex);
/* For innobase_kill_connection() */
UNIV_MEM_VALID(&trx->state, sizeof trx->state);
UNIV_MEM_VALID(&trx->mysql_thd, sizeof trx->mysql_thd);
#endif
trx = NULL;
}