Re: [PATCH 12/17] efi: fix efi_pci_io_protocol32 prototype for mixed mode

[Ard Biesheuvel]

On 14 May 2018 at 08:57, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>
> * Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:
>
>> Mixed mode allows a kernel built for x86_64 to interact with 32-bit
>> EFI firmware, but requires us to define all struct definitions carefully
>> when it comes to pointer sizes. efi_pci_io_protocol32 currently uses a
>> void* for the 'romimage' field, which will be interpreted as a 64-bit
>> field on such kernels, potentially resulting in bogus memory references
>> and subsequent crashes.
>
> Yeah, so the first confusion I ran into is:
>
>  s/efi_pci_io_protocol32
>   /efi_pci_io_protocol_32
>
> Once I found it in the code I made this change:
>
>  s/efi: fix efi_pci_io_protocol32 prototype for mixed mode
>   /efi: Fix 'struct efi_pci_io_protocol32' definition for mixed mode
>
> Because we normally use the 'prototype' name for function declarations, not for
> data type definitions. Adding 'struct' and putting it between quotes makes it
> obvious at a glance that we are talking about a structure definition here.
>

OK.

> BTW., since it's marked -stable, due to:
>
>  > potentially resulting in bogus memory references
>  > and subsequent crashes.
>
> I'm moving it to efi/urgent: the principle here is that if a patch is urgent
> enough for -stable then it should generally not wait for the next merge window.
>

The issue was found by inspection rather than a crash report, so I was
in doubt about this.

> Also, because this actually fixes a crash, I extended the title to spell this out
> more clearly:
>
>   Subject: efi: Avoid potential crashes, fix the 'struct efi_pci_io_protocol_32' definition for mixed mode
>
> ... which also makes it easier for maintainers of older stable kernels to decide
> whether to backport the patch or not.
>

OK.

> Anyway, the patch is looking good otherwise, no need to resend.
>
> Thanks,
>
>         Ingo