On 29 April 2018 at 11:08, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > Hi, > > > On 29-04-18 11:07, Ard Biesheuvel wrote: >> >> On 29 April 2018 at 10:40, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >>> >>> Hi, >>> >>> >>> On 29-04-18 09:43, Ingo Molnar wrote: >>>> >>>> >>>> >>>> * Hans de Goede <hdegoede@xxxxxxxxxx> wrote: >>>> >>>>> diff --git a/arch/x86/boot/compressed/eboot.c >>>>> b/arch/x86/boot/compressed/eboot.c >>>>> index 47d3efff6805..8650ab268ee7 100644 >>>>> --- a/arch/x86/boot/compressed/eboot.c >>>>> +++ b/arch/x86/boot/compressed/eboot.c >>>>> @@ -122,7 +122,14 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, >>>>> struct pci_setup_rom **__rom) >>>>> if (status != EFI_SUCCESS) >>>>> return status; >>>>> - if (!pci->romimage || !pci->romsize) >>>>> + /* >>>>> + * Some firmwares contain EFI function pointers at the place >>>>> where the >>>>> + * romimage and romsize fields are supposed to be. Typically >>>>> the >>>>> EFI >>>>> + * code is mapped at high addresses, translating to an >>>>> unrealistically >>>>> + * large romsize. The UEFI spec limits the size of option ROMs >>>>> to >>>>> 16 >>>>> + * MiB so we reject any roms over 16 MiB in size to catch this. >>>>> + */ >>>>> + if (!pci->romimage || !pci->romsize || pci->romsize > >>>>> 0x1000000) >>>>> return EFI_INVALID_PARAMETER; >>>>> size = pci->romsize + sizeof(*rom); >>>>> @@ -230,7 +237,14 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, >>>>> struct pci_setup_rom **__rom) >>>>> if (status != EFI_SUCCESS) >>>>> return status; >>>>> - if (!pci->romimage || !pci->romsize) >>>>> + /* >>>>> + * Some firmwares contain EFI function pointers at the place >>>>> where the >>>>> + * romimage and romsize fields are supposed to be. Typically >>>>> the >>>>> EFI >>>>> + * code is mapped at high addresses, translating to an >>>>> unrealistically >>>>> + * large romsize. The UEFI spec limits the size of option ROMs >>>>> to >>>>> 16 >>>>> + * MiB so we reject any roms over 16 MiB in size to catch this. >>>>> + */ >>>>> + if (!pci->romimage || !pci->romsize || pci->romsize > >>>>> 0x1000000) >>>>> return EFI_INVALID_PARAMETER; >>>> >>>> >>>> >>>> Any reason why this couldn't be factored out into a efi_check_rom(pci) >>>> kind of >>>> helper function >>> >>> >>> >>> The pci pointer is of 2 different types: >>> >>> __setup_efi_pci32(efi_pci_io_protocol_32 *pci, ... >>> __setup_efi_pci64(efi_pci_io_protocol_64 *pci, ... >>> >>> I guess I could give the helper a romimage and romsize argument to get >>> around that. >>> Ard, do you want me to do a v4 with such a helper ? >>> >> >> I think Lukas has a point, although I shouldn't expect you to implement >> that. >> >> I will look into this myself, and rebase your patch on top of that. > > > Ok, thanks. > Actually, looking into this, I think I may have spotted a bug in this code. Does the issue you are solving here occur when running a 64-bit kernel on 32-bit EFI? -- 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
