Skip to content

OcAppleKernelLib: fix unchecked overflow and align-before-check in PrelinkedContext.c #614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
albert-mueller wants to merge 1 commit into from
Closed

OcAppleKernelLib: fix unchecked overflow and align-before-check in PrelinkedContext.c #614

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions Library/OcAppleKernelLib/PrelinkedContext.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,14 +384,30 @@ PrelinkedContextInit (
//
// In KC mode last load address is the __LINKEDIT address.
//
SegmentEndOffset = Context->LinkEditSegment->Segment64.FileOffset + Context->LinkEditSegment->Segment64.FileSize;
if (BaseOverflowAddU64 (

@vit9696 vit9696 Aug 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks checked to me: https://github.com/acidanthera/OpenCorePkg/blob/164696d/Library/OcMachoLib/HeaderX.h#L1239-L1246

Context->LinkEditSegment->Segment64.FileOffset,
Context->LinkEditSegment->Segment64.FileSize,
&SegmentEndOffset
))
{
PrelinkedContextFree (Context);
return EFI_INVALID_PARAMETER;
}

if (MACHO_ALIGN (SegmentEndOffset) != Context->PrelinkedSize) {
PrelinkedContextFree (Context);
return EFI_INVALID_PARAMETER;
}

Context->PrelinkedLastLoadAddress = Context->LinkEditSegment->Segment64.VirtualAddress + Context->LinkEditSegment->Segment64.Size;
if (BaseOverflowAddU64 (

@vit9696 vit9696 Aug 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also looks checked to me: https://github.com/acidanthera/OpenCorePkg/blob/164696d/Library/OcMachoLib/HeaderX.h#L1248-L1250

Context->LinkEditSegment->Segment64.VirtualAddress,
Context->LinkEditSegment->Segment64.Size,
&Context->PrelinkedLastLoadAddress
))
{
PrelinkedContextFree (Context);
return EFI_INVALID_PARAMETER;
}
}

//
Expand Down Expand Up @@ -538,8 +554,16 @@ PrelinkedInjectPrepare (
// For newer variant (KC mode) __LINKEDIT is last, and we need to expand it to enable
// dyld fixup generation.
//
if ( (Context->PrelinkedAllocSize < LinkedExpansion)
|| (Context->PrelinkedAllocSize - LinkedExpansion < Context->PrelinkedSize))
//
// Align first and validate the aligned size, as that is what actually gets
// zeroed and committed below -- checking the unaligned LinkedExpansion here
// would let AlignedExpansion (rounded up) write past the validated bound.
//
AlignedExpansion = MACHO_ALIGN (LinkedExpansion);

@vit9696 vit9696 Aug 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@albert-mueller albert-mueller Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@vit9696 vit9696 Aug 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, hmm.

  1. The first two changes do nothing, they are definitely checked prior to the code you changed.
  2. 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?

@albert-mueller albert-mueller Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@vit9696 vit9696 Aug 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, thank you. Since 3 might happen, though very very unlikely, I made the change in master to make things cleaner anyway.


if ( (AlignedExpansion < LinkedExpansion)
|| (Context->PrelinkedAllocSize < AlignedExpansion)
|| (Context->PrelinkedAllocSize - AlignedExpansion < Context->PrelinkedSize))
{
return EFI_OUT_OF_RESOURCES;
}
Expand All @@ -556,7 +580,6 @@ PrelinkedInjectPrepare (

Context->KextsFixupChains = (VOID *)(Context->Prelinked + Context->PrelinkedSize);

AlignedExpansion = MACHO_ALIGN (LinkedExpansion);
//
// Zero the expansion to account for padding.
//
Expand Down
Loading