From cd37e494220fa556592d8f56666a72876ee63505 Mon Sep 17 00:00:00 2001 From: Monty Date: Thu, 20 Apr 2023 14:12:48 +0300 Subject: [PATCH] MDEV-31083 ASAN use-after-poison in myrg_attach_children The reason for ASAN report was that the MERGE and MYISAM file had different key definitions, which is not allowed. Fixed by ensuring that the MERGE code is not copying more key stats than what is in the MyISAM file. Other things: - Give an error if different MyISAM files has different number of key parts. --- include/myisammrg.h | 1 + mysql-test/main/merge.result | 12 ++++++++++++ mysql-test/main/merge.test | 12 ++++++++++++ storage/myisam/mi_open.c | 1 + storage/myisam/myisamdef.h | 2 +- storage/myisammrg/myrg_open.c | 16 ++++++++++------ 6 files changed, 37 insertions(+), 7 deletions(-) diff --git a/include/myisammrg.h b/include/myisammrg.h index 1d7efbe74d6..b3bca218a44 100644 --- a/include/myisammrg.h +++ b/include/myisammrg.h @@ -71,6 +71,7 @@ typedef struct st_myrg_info ulong cache_size; uint merge_insert_method; uint tables,options,reclength,keys; + uint key_parts; my_bool cache_in_use; /* If MERGE children attached to parent. See top comment in ha_myisammrg.cc */ my_bool children_attached; diff --git a/mysql-test/main/merge.result b/mysql-test/main/merge.result index 3761ca2c150..6722dd38b9c 100644 --- a/mysql-test/main/merge.result +++ b/mysql-test/main/merge.result @@ -3919,3 +3919,15 @@ ERROR HY000: Unable to open underlying table which is differently defined or of DROP TRIGGER trg1; DROP TABLE t1; DROP TABLE m1; +# +# MDEV-31083 ASAN use-after-poison in myrg_attach_children +# +CREATE TABLE t1 (f TEXT, FULLTEXT (f)) ENGINE=MyISAM; +INSERT INTO t1 VALUES ('foo'),('bar'); +CREATE TABLE mrg (f TEXT) ENGINE=MERGE, UNION(t1); +SELECT * FROM mrg; +f +foo +bar +DROP TABLE mrg, t1; +End of 10.5 tests diff --git a/mysql-test/main/merge.test b/mysql-test/main/merge.test index 888b41b24bd..ba2ce29ade2 100644 --- a/mysql-test/main/merge.test +++ b/mysql-test/main/merge.test @@ -2919,3 +2919,15 @@ set global default_storage_engine=@save_default_storage_engine; # Check that all connections opened by test cases in this file are really # gone so execution of other tests won't be affected by their presence. --source include/wait_until_count_sessions.inc + +--echo # +--echo # MDEV-31083 ASAN use-after-poison in myrg_attach_children +--echo # + +CREATE TABLE t1 (f TEXT, FULLTEXT (f)) ENGINE=MyISAM; +INSERT INTO t1 VALUES ('foo'),('bar'); +CREATE TABLE mrg (f TEXT) ENGINE=MERGE, UNION(t1); +SELECT * FROM mrg; +DROP TABLE mrg, t1; + +--echo End of 10.5 tests diff --git a/storage/myisam/mi_open.c b/storage/myisam/mi_open.c index 3db424ea997..f47ae48ad0d 100644 --- a/storage/myisam/mi_open.c +++ b/storage/myisam/mi_open.c @@ -518,6 +518,7 @@ MI_INFO *mi_open(const char *name, int mode, uint open_flags) share->kfile=kfile; share->this_process=(ulong) getpid(); share->last_process= share->state.process; + share->base.base_key_parts= base_key_parts; share->base.key_parts=key_parts; share->base.all_key_parts=key_parts+unique_key_parts; if (!(share->last_version=share->state.version)) diff --git a/storage/myisam/myisamdef.h b/storage/myisam/myisamdef.h index c90d989c975..f84ad6fa184 100644 --- a/storage/myisam/myisamdef.h +++ b/storage/myisam/myisamdef.h @@ -132,7 +132,7 @@ typedef struct st_mi_base_info uint extra_alloc_bytes; uint extra_alloc_procent; /* The following are from the header */ - uint key_parts, all_key_parts; + uint key_parts, all_key_parts, base_key_parts; } MI_BASE_INFO; diff --git a/storage/myisammrg/myrg_open.c b/storage/myisammrg/myrg_open.c index d9ea4b754f2..4a983684394 100644 --- a/storage/myisammrg/myrg_open.c +++ b/storage/myisammrg/myrg_open.c @@ -432,17 +432,20 @@ int myrg_attach_children(MYRG_INFO *m_info, int handle_locking, first_child= FALSE; m_info->reclength= myisam->s->base.reclength; min_keys= myisam->s->base.keys; - key_parts= myisam->s->base.key_parts; + key_parts= myisam->s->base.base_key_parts; if (*need_compat_check && m_info->rec_per_key_part) { my_free(m_info->rec_per_key_part); m_info->rec_per_key_part= NULL; } - if (!m_info->rec_per_key_part) + if (!m_info->rec_per_key_part || m_info->key_parts != key_parts) { - if(!(m_info->rec_per_key_part= (ulong*) - my_malloc(rg_key_memory_MYRG_INFO, - key_parts * sizeof(long), MYF(MY_WME)))) + m_info->key_parts= key_parts; + /* The +1 is because by my_realloc() don't allow zero length */ + if (!(m_info->rec_per_key_part= (ulong*) + my_realloc(rg_key_memory_MYRG_INFO, m_info->rec_per_key_part, + key_parts * sizeof(long) +1, + MYF(MY_WME | MY_ALLOW_ZERO_PTR | MY_FREE_ON_ERROR)))) goto err; /* purecov: inspected */ errpos= 1; } @@ -457,7 +460,8 @@ int myrg_attach_children(MYRG_INFO *m_info, int handle_locking, myisam->open_flag|= HA_OPEN_MERGE_TABLE; /* Check table definition match. */ - if (m_info->reclength != myisam->s->base.reclength) + if (m_info->reclength != myisam->s->base.reclength || + key_parts != myisam->s->base.base_key_parts) { DBUG_PRINT("error", ("definition mismatch table: '%s' repair: %d", myisam->filename,