MDEV-27562 MSAN use-of-uninitialized-value in cmp_buffer_with_ref

[arcivanov]

cmp_buffer_with_ref() decides whether an eq_ref lookup can reuse the row the previous lookup fetched, and it decides it by comparing the whole ref key buffer with memcmp(). A key part over a VARCHAR is a fixed width image, so the bytes that follow the value are compared as well, although they hold no information.

store_key fills that buffer through field_conv(), which for two identical VARCHAR fields takes the field_conv_memcpy() shortcut and copies pack_length() bytes. The bytes after the value are therefore whatever the source row carried there, and a storage engine is entitled to leave that part of a record undefined. InnoDB marks it so in row_sel_field_store_in_mysql_format_func() and Aria in _ma_read_block_record2(), and a MemorySanitizer build then aborts the server inside the comparison.

The sibling path is already clean: store_key_field copies through Copy_field, which for the same pair of fields picks do_varstring2_no_truncation() and writes only the length prefix and the value, leaving the rest as the preceding bzero() left it. cmp_buffer_with_ref() is the only consumer that treats a key buffer as an opaque byte range rather than parsing it by key part, which is why one call site is enough.

The fix

The comparison is a shortcut rather than a correctness decision: a difference in those bytes only causes an already fetched row to be fetched again. The change marks them defined, which is what Count_distinct_field::add() already does for the same reason. Field::mark_unused_memory_as_defined() is an empty inline unless the server is built with memory instrumentation, so a production build is unchanged.

The marked range is the key part's slot, not the column's width. copy_keys_from_share() gives a key part built over a prefix its own field narrowed to the key part's length, so store_key's field already describes the slot; a KEY k (a(10)) over a VARCHAR(256) reports field_length 10, not 256.

Measured

Measured on an MSAN build of this branch, using the amd64-msan-clang-20 builder's cmake line inside quay.io/mariadb-foundation/bb-worker:debian12-msan-clang-20, with only this change differing. MSAN halts on the first report, so each block below is a separate test file.

block	without the change	with it
multi-table update, InnoDB, no row qualifying	abort, use-of-uninitialized-value in bcmp, offset 6 inside a 259 byte buffer, poisoned by row_sel_field_store_in_mysql_format_func()	pass
the same with three rows qualifying	abort, same frames	pass
correlated subquery, InnoDB	abort, same frames, offset 4 inside the same buffer	pass
multi-table update, Aria	abort, same frames, poisoned by _ma_read_block_record2()	pass
main,heap,maria under MSAN	-	1192/1192, no sanitizer reports
the test under ps, view, cursor and sp protocols	-	pass

The two offsets match the offsets in the two reports.

No build without memory instrumentation can fail on the read, so each block also checks that the subquery cache counters moved; without that the test passes with subquery_cache=off and covers nothing. Only whether they moved is checked, because a prepared statement executes the subquery twice and doubles them. The column has to be wider than 255 bytes: a key field always carries a two byte length prefix and Field_varstring::memcpy_field_possible() requires the source to agree, so a narrower column takes a conversion path that never copies the bytes in question.

Scope

This is the MSAN half of MDEV-32436 as well. The Valgrind errors that ticket also reports come from InnoDB's insert path (rec_convert_dtuple_to_rec_* <- row_ins_* <- ha_innobase::write_row) and are a separate matter.

Two neighbouring sites build key buffers the same way and are unaffected. In opt_subselect.cc the field created for the weedout key is assigned and never read, so nothing is copied through it. The GROUP BY group_buff really is filled by the same full width copy, but it is handed to ha_index_read_map(), and both engines parse the two byte length prefix and read no further - HEAP in hp_hashnr(), Aria in _ma_pack_key().

Targeted at 10.11 because the code is identical there and reachability was measured on it, not taken from the ticket's Affects Versions.

[gkodinov]

LGTM. Thank you!

Please stay tuned for the final review.

[gkodinov]

FYI: According to our development cycle we work on bugs In the following periods 15 Mar-30 Apr, 15 Jun-30 Jul, 15 Sep-30 Oct and 15 Dec-31 Jan. So, please, expect to get a review somewhere between these two dates and the goal is to have your PR merged before the second date