Hi, Sergey!
On Oct 19, Sergey Vojtovich wrote:
revision-id: cf9bafc8cb0943c914d118525888140ea15e6b55 (mariadb-10.0.36-50-gcf9bafc)
parent(s): 8e716138cef2d9be603cdf71701d82bcb72dfd69
author: Sergey Vojtovich
committer: Sergey Vojtovich
timestamp: 2018-10-18 23:50:50 +0400
message:
MDEV-14815 - Server crash or AddressSanitizer errors or valgrind warnings
in thr_lock / has_old_lock upon FLUSH TABLES
Explicit partition access of partitioned MEMORY table under LOCK TABLES
may cause subsequent statements to crash the server, deadlock, trigger
valgrind warnings or ASAN errors. Freed memory was being used due to
incorrect cleanup.
At least MyISAM and InnoDB don't seem to be affected, since their
THR_LOCK structures don't survive FLUSH TABLES. MEMORY keeps table shared
data (including THR_LOCK) even if there're no open instances.
There's partition_info::lock_partitions bitmap, which holds bits of
partitions allowed to be accessed after pruning. This bitmap is
updated for each individual statement.
This bitmap was abused in ha_partition::store_lock() such that when we
need to unlock a table, locked by LOCK TABLES, only locks for partitions
that were accessed by previous statement were released.
Eventually FLUSH TABLES frees THR_LOCK_DATA objects, which are still
linked into THR_LOCK lists. When such THR_LOCK gets reused we end up with
freed memory access.
Fixed by using ha_partition::m_locked_partitions bitmap similarly to
ha_partition::external_lock().
Thanks, good comment.
Some unused code removed.
Generally, it's better to do that in a separate commit.
It's very easy to do with git citool, and doesn't requite any advance
planning, you just edit whatever you like and then run 'git citool'
twice, selecting what lines you want to commit each time.
diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc
index 0488ebf..02de92a 100644
--- a/sql/ha_partition.cc
+++ b/sql/ha_partition.cc
@@ -3907,9 +3907,14 @@ THR_LOCK_DATA **ha_partition::store_lock(THD *thd,
}
else
{
- for (i= bitmap_get_first_set(&(m_part_info->lock_partitions));
+ MY_BITMAP *used_partitions= lock_type == TL_UNLOCK ||
+ lock_type == TL_IGNORE ?
why TL_IGNORE too? external_lock only checks for TL_UNLOCK.
+ &m_locked_partitions :
+ &m_part_info->lock_partitions;
+
+ for (i= bitmap_get_first_set(used_partitions);
i < m_tot_parts;
- i= bitmap_get_next_set(&m_part_info->lock_partitions, i))
+ i= bitmap_get_next_set(used_partitions, i))
{
DBUG_PRINT("info", ("store lock %d iteration", i));
to= m_file[i]->store_lock(thd, to, lock_type);
Regards,
Sergei
Chief Architect MariaDB
and security@mariadb.org
Sergei Golubchik
Sergey Vojtovich