OcAppleKernelLib: fix unchecked overflow and align-before-check in PrelinkedContext.c - #614
Conversation
albert-mueller
commented
Aug 15, 2026
albert-mueller
commented
- PrelinkedContextInit: guard the KC __LINKEDIT SegmentEndOffset and PrelinkedLastLoadAddress additions with BaseOverflowAddU64, matching the overflow-checked style already used elsewhere in this file (e.g. PrelinkedFindLastLoadAddress).
- PrelinkedInjectPrepare: compute AlignedExpansion before validating it against PrelinkedAllocSize, instead of validating the unaligned LinkedExpansion and then zeroing/committing the larger aligned value.
…elinkedContext.c - PrelinkedContextInit: guard the KC __LINKEDIT SegmentEndOffset and PrelinkedLastLoadAddress additions with BaseOverflowAddU64, matching the overflow-checked style already used elsewhere in this file (e.g. PrelinkedFindLastLoadAddress). - PrelinkedInjectPrepare: compute AlignedExpansion before validating it against PrelinkedAllocSize, instead of validating the unaligned LinkedExpansion and then zeroing/committing the larger aligned value.
vit9696
left a comment
vit9696
left a comment
There was a problem hiding this comment.
Thank you for the patch. I am not fully positive all the changes are needed. Replied inline.
| // In KC mode last load address is the __LINKEDIT address. | ||
| // | ||
| SegmentEndOffset = Context->LinkEditSegment->Segment64.FileOffset + Context->LinkEditSegment->Segment64.FileSize; | ||
| if (BaseOverflowAddU64 ( |
There was a problem hiding this comment.
This looks checked to me: https://github.com/acidanthera/OpenCorePkg/blob/164696d/Library/OcMachoLib/HeaderX.h#L1239-L1246
| } | ||
|
|
||
| Context->PrelinkedLastLoadAddress = Context->LinkEditSegment->Segment64.VirtualAddress + Context->LinkEditSegment->Segment64.Size; | ||
| if (BaseOverflowAddU64 ( |
There was a problem hiding this comment.
This also looks checked to me: https://github.com/acidanthera/OpenCorePkg/blob/164696d/Library/OcMachoLib/HeaderX.h#L1248-L1250
| // zeroed and committed below -- checking the unaligned LinkedExpansion here | ||
| // would let AlignedExpansion (rounded up) write past the validated bound. | ||
| // | ||
| AlignedExpansion = MACHO_ALIGN (LinkedExpansion); |
There was a problem hiding this comment.
Did you find an actual path to violate this? I am not yet sure this change is needed, but it will help if you confirm it.
There was a problem hiding this comment.
Yes, it is needed, just tested on my T2 MacBook Pro and for the first time on a T2 Mac it boots Tahoe via OpenCore Legacy Patcher T2 and now it is able to boot Tahoe without triggering Activation Lock or causing Needs authenticator panics. Just uploading a video showing my MacBook Pro 2020 booting OpenCore with a spoofed SMBIOS, with adding the latest changes I did in my fork on top:
https://youtu.be/_ACpaSJEt2Q?is=rXZPrSjH3SAsbL3_ - this video demonstrates my MacBook Pro 2020 4 thunderbolt 3 ports being able to boot via OpenCorePkg spoofed as MacBook Pro 2019 16 inch without much patches except the most obvious kexts and does get to the desktop.
On a Hackintosh system or a non-T2 Mac, you don’t have a T2 chip and as such this change only improves performance. But on T2 Macs trying to boot Tahoe via OpenCorePkg supported or not, this change is must-have to even boot.
There was a problem hiding this comment.
Well, hmm.
- The first two changes do nothing, they are definitely checked prior to the code you changed.
- The third change will abort booting, if the check you modified triggers. In fact, if the check triggers, you have corrupted kernel collection supplied by Apple, which has never been the case so far.
The fact your T2 system boots is dependent on something else, and these changes cannot affect it directly, I am afraid.
What is the origin of the changes?
There was a problem hiding this comment.
Fair, and thanks for checking — you're right on all three points.
Origin: these came out of an AI-assisted read-through of this file. It picked up on the overflow-checked style used elsewhere in PrelinkedContext.c and pattern-matched the same treatment onto these two additions, without tracing far enough to see they're already covered.
On 1 & 2: confirmed against MachoGetSegmentByName() → MachoGetNextSegment() directly. Context->LinkEditSegment can only ever come from there, and that path already checks FileOffset + FileSize (overflow, bounded against Context->FileSize) and VirtualAddress + Size (overflow) before a segment is ever returned. So both BaseOverflowAddU64 additions are unreachable, as you said.
On 3: I don't have a concrete path that violates the existing LinkedExpansion check, and since PrelinkedAllocSize is reserved with its own margin by the caller, I agree this is theoretical at best.
And I should retract the T2 boot claim — that's not explained by any of this. Whatever actually fixed that boot is unrelated to this file. Sorry for the noise, closing this.
There was a problem hiding this comment.
Right, thank you. Since 3 might happen, though very very unlikely, I made the change in master to make things cleaner anyway.
Might happen if aligning LinkedExpansion exceeds PrelinkedAllocSize. Thanks @albert-mueller closes #614
|
I’ve found that this error is still occurring mostly on unsupported T2 Macs: albert-mueller/OpenCore-Legacy-Patcher-T2#130 |
2 participants
