Skip to content

corec: fix dataheaphead layout so Size is read where datahead reads it - #220

Open
rgreen83 wants to merge 1 commit into
Matroska-Org:masterfrom
rgreen83:corec-array-fix
Open

rgreen83 wants to merge 1 commit into
Matroska-Org:masterfrom
rgreen83:corec-array-fix

Conversation

@rgreen83

Copy link
Copy Markdown

corec: fix memory corruption from misaligned dataheaphead layout

The 2025 array refactor split the array header into two distinct layouts:
datahead puts Size first, while dataheaphead put Heap first and
Size second.

Because Data_Head() always assumes the size field is the first member
before the array data, it incorrectly interpreted the Heap pointer of
heap-tracked arrays as the array size. This garbage size caused
ArrayResize to skip proper reallocation and hand back the static
placeholder in MemHeap_Default as if it were writable storage.

Consequently, writes into memheap-backed empty arrays (such as mkclean's
block SizeList) went past the static placeholder and corrupted memory.

This fix moves Size to the first position in dataheaphead to align
with datahead.

This also fixes mkvalidator, which segfaults on any input with the broken
corec; verified before and after the fix.

Data_Head() assumes the size field is the first member before the array
data. dataheaphead had Heap first, so heap-flag checks and size reads
got the heap pointer instead, and writes into memheap-backed empty
arrays (e.g. mkclean's block SizeList) went past the static placeholder
and corrupted memory. Put Size first to match datahead.
@rgreen83

rgreen83 commented Nov 5, 2026

Copy link
Copy Markdown
Author

This is an independent fix for the same underlying bug as #219, with the same diagnosis: the 2025 refactor left datahead and dataheaphead with Size at different offsets relative to data, so Data_Head() reads a memheap-backed array's Heap self-pointer as its size and ArrayResize() skips allocation. The fix goes the other direction from #219: rather than giving datahead an unused leading pointer to match dataheaphead, this reorders MEMHEAD_POINTER_HOLDER to Size-first so dataheaphead matches the layout Data_Head() has always read on plain arrays.

One extra detail this approach needs and #219 does not: MEMHEAD_POINTER_HOLDER also defines the tail of cc_memheap, so reordering it means the static initializer of MemHeap_Default in corec/array/array.c has to be reordered as well, or the default heap's function pointers and flags land in the wrong slots. The second commit here covers that.

Verified with ASan: clean runs on mkclean and mkvalidator on files that segfault plain master. Happy to withdraw this in favor of #219 if the maintainers prefer that approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant