Commit 6487b8e3 authored by Alexander Barkov's avatar Alexander Barkov

MDEV-27307 main.ctype_utf8mb4_uca_allkeys tests fail with Valgrind/MSAN

In case when filesort does not use addon field packing (because of
too small potential savings) and uses fixed width addon fields instead,
the field->pack() call can store less bytes when the field maximum
possible field length, e.g. in case of VARCHAR().
The memory between the packed length and addonf->length (the maximum length)
stayed uninitialized, which was reported by Valgrind/MSAN.

The problem was introduced by f52bf920 in 10.5,
which removed the tail initialization (probably unintentionally).

Restoring the bzero() in the fixed length branch,
so in case when pack() stores less bytes than addonf->length says,
the trailing bytes gets initialized.

Note, before f52bf920, the bzero()
was under HAVE_valgrind conditional compilation. Now it's being added
unconditionally:
- MSAN also reported the problem, so it's not only Valgrind specific.
- As Serg proposed, conditional initialization is bad - it can have
  potentional security problems as the non-initialized memory fragments
  can store various pieces of essential information, e.g. passwords.
parent 2776635c
#
# Start of 10.5 tests
#
#
# MDEV-27307 main.ctype_utf8mb4_uca_allkeys tests fail with Valgrind/MSAN
#
# In this scenario packing is fully disabled:
# - The sortkey is of a fixed width data type
# - The addon fields have too small potential savings
SET NAMES latin1;
CREATE TABLE t1 (
code INT NOT NULL,
str VARCHAR(5) CHARACTER SET latin1 NOT NULL
) ENGINE=MyISAM;
FOR i IN 0..50
DO
INSERT INTO t1 VALUES (i, REPEAT('a',1));
END FOR;
$$
SELECT COUNT(*) FROM t1;
COUNT(*)
51
SET sort_buffer_size=1024;
SELECT HEX(code), HEX(str) FROM t1 ORDER BY HEX(str);
SET sort_buffer_size=DEFAULT;
DROP TABLE t1;
#
# End of 10.5 tests
#
#
# Tests related to pack modes of the sortkey and addon fields.
# Fields can be either packed or fixed length.
#
# See the comment in filesort.cc:
#
# Heuristic: skip packing if potential savings are less than 10 bytes.
#
--echo #
--echo # Start of 10.5 tests
--echo #
--echo #
--echo # MDEV-27307 main.ctype_utf8mb4_uca_allkeys tests fail with Valgrind/MSAN
--echo #
--echo # In this scenario packing is fully disabled:
--echo # - The sortkey is of a fixed width data type
--echo # - The addon fields have too small potential savings
SET NAMES latin1;
CREATE TABLE t1 (
code INT NOT NULL,
str VARCHAR(5) CHARACTER SET latin1 NOT NULL
) ENGINE=MyISAM;
DELIMITER $$;
FOR i IN 0..50
DO
INSERT INTO t1 VALUES (i, REPEAT('a',1));
END FOR;
$$
DELIMITER ;$$
SELECT COUNT(*) FROM t1;
# The result is not very interesting, let's suppress it.
# We just make sure there are no Valgrind/MSAN warnings about
# not initialized memory being written to disk.
SET sort_buffer_size=1024;
--disable_result_log
SELECT HEX(code), HEX(str) FROM t1 ORDER BY HEX(str);
--enable_result_log
SET sort_buffer_size=DEFAULT;
DROP TABLE t1;
--echo #
--echo # End of 10.5 tests
--echo #
......@@ -1376,14 +1376,19 @@ static uint make_sortkey(Sort_param *param, uchar *to, uchar *ref_pos,
else
{
uchar *end= field->pack(to, field->ptr);
int sz= static_cast<int>(end - to);
DBUG_ASSERT(end >= to);
uint sz= static_cast<uint>(end - to);
res_len += sz;
if (packed_addon_fields)
to+= sz;
else
{
if (addonf->length > sz)
bzero(end, addonf->length - sz); // Make Valgrind/MSAN happy
to+= addonf->length;
}
}
}
if (packed_addon_fields)
Addon_fields::store_addon_length(p_len, res_len);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment