[U-Boot] [PATCH 1/1] efi_loader: always call Exit after an image returns.

[Alexander Graf]

> Am 24.01.2018 um 01:05 schrieb Heinrich Schuchardt <xypron.glpk at gmx.de>:
> 
>> On 01/24/2018 12:31 AM, Alexander Graf wrote:
>> 
>> 
>>> On 23.01.18 23:46, Heinrich Schuchardt wrote:
>>> If an application or driver started via StartImage returns without
>>> calling Exit, StartImage has to call Exit. This is mandated by the
>>> UEFI spec and we do the same in efi_do_enter().
>>> 
>>> The patch looks bigger than it is. To avoid a forward declaration function
>>> efi_exit() was moved up. Only one (void*) was replaced by (void *).
>>> The only real change is in efi_start_image().
>>> 
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>> Without this patch on Ubuntu 16.04 a crash could be observed in
>>> efi_selftest_startimage_return.c. With the patch it is gone.
>>> But this could be coincidence.
>>> ---
>>> lib/efi_loader/efi_boottime.c | 99 ++++++++++++++++++++++---------------------
>>> 1 file changed, 51 insertions(+), 48 deletions(-)
>>> 
>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>>> index 0ac5b44..62a24bf 100644
>>> --- a/lib/efi_loader/efi_boottime.c
>>> +++ b/lib/efi_loader/efi_boottime.c
>>> @@ -1519,6 +1519,54 @@ failure:
>>> }
>>> 
>>> /*
>>> + * Leave an EFI application or driver.
>>> + *
>>> + * This function implements the Exit service.
>>> + * See the Unified Extensible Firmware Interface (UEFI) specification
>>> + * for details.
>>> + *
>>> + * @image_handle    handle of the application or driver that is exiting
>>> + * @exit_status        status code
>>> + * @exit_data_size    size of the buffer in bytes
>>> + * @exit_data        buffer with data describing an error
>>> + * @return        status code
>>> + */
>>> +static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle,
>>> +            efi_status_t exit_status, unsigned long exit_data_size,
>>> +            int16_t *exit_data)
>>> +{
>>> +    /*
>>> +     * We require that the handle points to the original loaded
>>> +     * image protocol interface.
>>> +     *
>>> +     * For getting the longjmp address this is safer than locating
>>> +     * the protocol because the protocol may have been reinstalled
>>> +     * pointing to another memory location.
>>> +     *
>>> +     * TODO: We should call the unload procedure of the loaded
>>> +     *     image protocol.
>>> +     */
>>> +    struct efi_loaded_image *loaded_image_info = (void *)image_handle;
>>> +
>>> +    EFI_ENTRY("%p, %ld, %ld, %p", image_handle, exit_status,
>>> +          exit_data_size, exit_data);
>>> +
>>> +    /* Make sure entry/exit counts for EFI world cross-overs match */
>>> +    EFI_EXIT(exit_status);
>>> +
>>> +    /*
>>> +     * But longjmp out with the U-Boot gd, not the application's, as
>>> +     * the other end is a setjmp call inside EFI context.
>>> +     */
>>> +    efi_restore_gd();
>>> +
>>> +    loaded_image_info->exit_status = exit_status;
>>> +    longjmp(&loaded_image_info->exit_jmp, 1);
>>> +
>>> +    panic("EFI application exited");
>>> +}
>>> +
>>> +/*
>>>  * Call the entry point of an image.
>>>  *
>>>  * This function implements the StartImage service.
>>> @@ -1575,59 +1623,14 @@ static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
>>> 
>>>    ret = EFI_CALL(entry(image_handle, &systab));
>>> 
>>> +    /* Clean up the image */
>>> +    ret = EFI_CALL(efi_exit(image_handle, ret, 0, NULL));
>> 
>> Just use the systab here, like we do in efi_do_enter().
> 
> No, efi_do_enter passes image_handle to efi_exit().
> 
> The UEFI API requires the image handle. It is the same image handle that
> the application would pass when calling Exit.
> 
> efi_exit() uses the handle as pointer to the loaded image information.

I mean use the systab to get to efi_exit instead of shuffling code around :). It‘s what we do in bootefi as well.


Alex