On 13 July 2018 at 15:29, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>
> * Lukas Wunner <lukas@xxxxxxxxx> wrote:
>
>> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
>> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
>> @@ -413,6 +413,32 @@ static efi_status_t efi_file_close(void *handle)
>> return efi_call_proto(efi_file_handle, close, handle);
>> }
>>
>> +static efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,
>> + efi_loaded_image_t *image,
>> + efi_file_handle_t **__fh)
>> +{
>> + efi_file_io_interface_t *io;
>> + efi_file_handle_t *fh;
>> + efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
>> + efi_status_t status;
>> + void *handle =
>> + (void *)efi_table_attr(efi_loaded_image, device_handle, image);
>> +
>> + status = efi_call_early(handle_protocol, handle,
>> + &fs_proto, (void **)&io);
>> + if (status != EFI_SUCCESS) {
>> + efi_printk(sys_table_arg, "Failed to handle fs_proto\n");
>> + return status;
>> + }
>> +
>> + status = efi_call_proto(efi_file_io_interface, open_volume, io, &fh);
>> + if (status != EFI_SUCCESS)
>> + efi_printk(sys_table_arg, "Failed to open volume\n");
>> +
>> + *__fh = fh;
>> + return status;
>
> BTW., in the second failure branch is 'fh' guaranteed to be set by the EFI call?
> If not then we set *__fh to a potentially undefined value, from the kernels stack?
>
> (I realize that your refactoring just inherited this existing pattern, but it
> caught my attention.)
>
No, only EFI_SUCCESS guarantees that fh will have been assigned.
Since we propagate 'status' here, it does not seem to matter in
practice, but it would indeed be better not to clobber *__fh with
random junk when returning failure.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
