Re: [PATCH v3 5/6] MdeModulePkg/DxeCore: defer PE/COFF emulator registration to StartImage

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Wed, 26 Feb 2020 at 20:43, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> EDK2's implementation of the LoadImage() boot service permits non-native
> binaries to be loaded (i.e., X64 images on IA32 firmware), but any
> attempts to start such an image using StartImage() will return
> EFI_UNSUPPORTED.
>
> The integration of the PE/COFF emulator protocol into the DXE core
> deviates slightly from this paradigm, given that its IsImageSupported
> hook as well as its RegisterImage hook are invoked from LoadImage,
> and by the time StartImage is called, no opportunity is given to the
> provider of the PE/COFF emulator protocol to prevent an image from
> being started if it only supports loading it.
>
> To address this disparity, let's move the invocation of RegisterImage()
> to the implementation of the StartImage() boot service, allowing the
> emulator to permit LoadImage() but reject StartImage() on images that
> turn out not to meet the requirements of the emulator as it is being
> started.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

@MdeModulePkg maintainers: do you have any comments on this patch?


> ---
>  MdeModulePkg/Core/Dxe/Image/Image.c | 24 +++++++++++---------
>  1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
> index 22a87ecf6d7c..d86da89ee704 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -756,17 +756,6 @@ CoreLoadPeImage (
>    // Get the image entry point.
>    //
>    Image->EntryPoint   = (EFI_IMAGE_ENTRY_POINT)(UINTN)Image->ImageContext.EntryPoint;
> -  if (Image->PeCoffEmu != NULL) {
> -    Status = Image->PeCoffEmu->RegisterImage (Image->PeCoffEmu,
> -                                 Image->ImageBasePage,
> -                                 EFI_PAGES_TO_SIZE (Image->NumberOfPages),
> -                                 &Image->EntryPoint);
> -    if (EFI_ERROR (Status)) {
> -      DEBUG ((DEBUG_LOAD | DEBUG_ERROR,
> -        "CoreLoadPeImage: Failed to register foreign image with emulator.\n"));
> -      goto Done;
> -    }
> -  }
>
>    //
>    // Fill in the image information for the Loaded Image Protocol
> @@ -1603,6 +1592,19 @@ CoreStartImage (
>      return EFI_UNSUPPORTED;
>    }
>
> +  if (Image->PeCoffEmu != NULL) {
> +    Status = Image->PeCoffEmu->RegisterImage (Image->PeCoffEmu,
> +                                 Image->ImageBasePage,
> +                                 EFI_PAGES_TO_SIZE (Image->NumberOfPages),
> +                                 &Image->EntryPoint);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_LOAD | DEBUG_ERROR,
> +        "CoreLoadPeImage: Failed to register foreign image with emulator - %r\n",
> +          Status));
> +      return Status;
> +    }
> +  }
> +
>    PERF_START_IMAGE_BEGIN (Handle);
>
>
> --
> 2.17.1
>