On Sun, Apr 29, 2018 at 09:43:18AM +0200, Ingo Molnar wrote: > * Hans de Goede <[email protected]> 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, which would unify the logic and would also > avoid the duplicate comment blocks?
The real fix would be to deduplicate __setup_efi_pci32 and __setup_efi_pci64 à la commits 2bd79f30eea1 and db4545d9a788. (That said the comment seems overly wordy. A short pointer to the UEFI spec with chapter number should be sufficient, anything else should be in the changelog. Just my 2 cents anyway.) Thanks, Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
