Skip to content

Commit 680f83b

Browse files
committed
Bug#36562979: MySQL crash at VarintParse64()
The hash join buffer could in some cases have too little space for a row to be written, so that data could be written outside of the allocated buffer and cause erroneous behaviour. This happened if the hash join buffer stored row ids from a table in the MEMORY engine. The reason was that the maximum size per row was calculated before the row id size (that is, handler::ref_length) had been initialized in the MEMORY engine handler, so the default ref_length for the handler class was used instead, which is sizeof(my_off_t). This is usually half the correct size, which is sizeof(HP_HEAP_POSITION). Fixed by initializing ref_length already when the MEMORY engine handler is constructed, so that it always has the correct value. Also add more assertions to detect the problem earlier. Change-Id: I6a7a675f2117b78c5b279bbba1abd8da641fad6e (cherry picked from commit 05b23e971e0e779406043c15cc8b4b417a246c91)
1 parent 57b6d0d commit 680f83b

File tree

3 files changed

+13
-12
lines changed

3 files changed

+13
-12
lines changed

sql/iterators/hash_join_buffer.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,15 @@ HashJoinRowBuffer::StoreLinkedImmutableStringFromTableBuffers(
139139
}
140140

141141
ret = LinkedImmutableString::EncodeHeader(next_ptr, &dptr);
142+
143+
const char *const start_of_row [[maybe_unused]] = dptr;
142144
dptr = pointer_cast<char *>(
143145
StoreFromTableBuffersRaw(m_tables, pointer_cast<uchar *>(dptr)));
146+
assert(dptr <= start_of_row + row_size_upper_bound);
144147

148+
const size_t actual_length = dptr - start_of_value;
149+
assert(actual_length <= required_value_bytes);
145150
if (!committed) {
146-
const size_t actual_length = dptr - pointer_cast<char *>(start_of_value);
147151
m_mem_root.RawCommit(actual_length);
148152
}
149153
return ret;

storage/heap/ha_heap.cc

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,9 @@ static handler *heap_create_handler(handlerton *hton, TABLE_SHARE *table, bool,
7777
*****************************************************************************/
7878

7979
ha_heap::ha_heap(handlerton *hton, TABLE_SHARE *table_arg)
80-
: handler(hton, table_arg),
81-
file(nullptr),
82-
records_changed(0),
83-
key_stat_version(0),
84-
single_instance(false) {}
80+
: handler(hton, table_arg) {
81+
ref_length = sizeof(HP_HEAP_POSITION);
82+
}
8583

8684
/*
8785
Hash index statistics is updated (copied from HP_KEYDEF::hash_buckets to
@@ -131,7 +129,6 @@ int ha_heap::open(const char *name, int mode, uint test_if_locked,
131129
}
132130
}
133131

134-
ref_length = sizeof(HP_HEAP_POSITION);
135132
/*
136133
We cannot run update_key_stats() here because we do not have a
137134
lock on the table. The 'records' count might just be changed

storage/heap/ha_heap.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@
3232
#include "sql/table.h"
3333

3434
class ha_heap : public handler {
35-
HP_INFO *file;
36-
HP_SHARE *internal_share;
35+
HP_INFO *file{nullptr};
36+
HP_SHARE *internal_share{nullptr};
3737
/* number of records changed since last statistics update */
38-
uint records_changed;
39-
uint key_stat_version;
38+
uint records_changed{0};
39+
uint key_stat_version{0};
4040
/// True if only one ha_heap is to exist for the table.
41-
bool single_instance;
41+
bool single_instance{false};
4242

4343
public:
4444
ha_heap(handlerton *hton, TABLE_SHARE *table);

0 commit comments

Comments
 (0)