MDEV-40498 KEY_OP_CHANGE copies length without checking header_end - #5568
Open
KhaledR57 wants to merge 1 commit into
Open
MDEV-40498 KEY_OP_CHANGE copies length without checking header_end#5568KhaledR57 wants to merge 1 commit into
KhaledR57 wants to merge 1 commit into
Conversation
The KEY_OP_CHANGE branch of _ma_apply_redo_index() passed the length from the redo record to memcpy() without checking it against the end of that record, and bounded the destination with a DBUG_ASSERT, which is compiled out when DBUG_OFF is set. A record claiming more data than it carries could therefore copy bytes of earlier records into the page, and an unchecked page offset could put that copy outside the page. Turn the assert into a runtime check. The logged length and its two operand bytes must fit in what is left of the record, the page offset must be set, and offset plus length must stay inside the used page. The page bound matters on its own: page_offset comes from KEY_OP_OFFSET, which is still guarded only by a DBUG_ASSERT, and records carrying no KEY_OP_SHIFT reach the copy with no bound on it at all. The test forges the logged length with corrupt_change_length and the logged offset with corrupt_change_offset, then crashes the server so recovery has to replay the records. Each keyword uses the largest value the earlier checks accept, so the two cases trip different halves of the new check.
KhaledR57
force-pushed
the
10.11-MDEV-40498
branch
from
August 18, 2026 14:46
14a06fb to
e7cfdd8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
KEY_OP_CHANGEbranch of_ma_apply_redo_index()passed the length from the redo record tomemcpy()without checking it against the end of that record, and bounded the destination with aDBUG_ASSERT, which is compiled out whenDBUG_OFFis set. A record claiming more data than it carries could therefore copy bytes of earlier records into the page, and an unchecked page offset could put that copy outside the page.Turn the assert into a runtime check. The logged length and its two operand bytes must fit in what is left of the record, the page offset must be set, and offset plus length must stay inside the used page. The page bound matters on its own:
page_offsetcomes fromKEY_OP_OFFSET, which is still guarded only by aDBUG_ASSERT, and records carrying noKEY_OP_SHIFTreach the copy with no bound on it at all.The test forges the logged length with
corrupt_change_lengthand the logged offset withcorrupt_change_offset, then crashes the server so recovery has to replay the records. Each keyword uses the largest value the earlier checks accept, so the two cases trip different halves of the new check.