Skip to content

Commit

Permalink
OcBootManagementLib: Add guard for static mImageLoaderCaps
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikebeaton
mikebeaton committed Oct 29, 2023
1 parent 909008c commit c6060b2
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions Library/OcBootManagementLib/ImageLoader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ STATIC EFI_HANDLE mCurrentImageHandle;
STATIC OC_IMAGE_LOADER_PATCH mImageLoaderPatch;
STATIC OC_IMAGE_LOADER_CONFIGURE mImageLoaderConfigure;
STATIC UINT32 mImageLoaderCaps;
STATIC EFI_HANDLE mImageLoaderCapsHandle;
STATIC BOOLEAN mImageLoaderEnabled;

STATIC BOOLEAN mProtectUefiServices;
Expand Down Expand Up @@ -773,6 +774,8 @@ InternalEfiLoadImage (
VOID *AllocatedBuffer;
UINT32 RealSize;

mImageLoaderCapsHandle = NULL;

if ((ParentImageHandle == NULL) || (ImageHandle == NULL)) {
return EFI_INVALID_PARAMETER;
}
Expand Down Expand Up @@ -924,6 +927,8 @@ InternalEfiLoadImage (
InternalUpdateLoadedImage (*ImageHandle, DevicePath);
}

mImageLoaderCapsHandle = *ImageHandle;

return Status;
}

Expand All @@ -940,6 +945,13 @@ InternalEfiStartImage (
OC_LOADED_IMAGE_PROTOCOL *OcLoadedImage;
EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;

if ( (mImageLoaderConfigure != NULL)
&& (ImageHandle != mImageLoaderCapsHandle))
{
DEBUG ((DEBUG_ERROR, "OCB: load/start unsupported ordering, %p != %p\n", ImageHandle, mImageLoaderCapsHandle));
return EFI_INVALID_PARAMETER;
}

//
// If we loaded the image, invoke the entry point manually.
//
Expand Down

10 comments on commit c6060b2

@CobanRamo
Copy link

Choose a reason for hiding this comment

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

With this commit, the recovery from Ventura and Sonoma no longer starts.
Catalina Recovery, on the other hand, starts with an extra long delay.
This wasn't a problem with previous builds.

Greetings Coban

@mikebeaton
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you are sure this is replicable and tied to this commit, please can you provide a debug error log of a failing boot.

@CobanRamo
Copy link

@CobanRamo CobanRamo commented on c6060b2 Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, should have thought of that before :)

I repeated the whole thing with the debug version of the build.
Recovery Catalina starts delayed.
Recovery Ventura does not start
Recovery Sonoma does not start
The last two end with the error message only in the Debug build;
"OCB: load/start unsupported ordering, 6DEC2298 != 709F3318"
With the release version there is no message, just a black image.

MacOS itself, all 3 versions, can start normally.

Here are the logs.

Greetings Coban
Archiv.zip

PS: I have to go to work and can't be able to reply until late tonight.

@mikebeaton
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can replicate the issue. Thank you for the report. Actually this does not mean that the change here was incorrect, but means that a more comprehensive fix for the issue identified is needed. To follow.

@mikebeaton
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@CobanRamo - The subsequent commit should resolve the issue.

@CobanRamo
Copy link

Choose a reason for hiding this comment

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

Thanks guys and girls...
Great support...

@mikebeaton
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you confirm that all the recoveries where you saw problems are back to normal, when you have a chance?

@CobanRamo
Copy link

Choose a reason for hiding this comment

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

Of course I can, but only again tonight.

@CobanRamo
Copy link

Choose a reason for hiding this comment

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

I was able to have it tested, and apparently it's working again.
All recoveries start again normally.

Thanks Mike.

@mikebeaton
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for quick report and quick tests - appreciated.

Please sign in to comment.