Re: [tip:efi/core] efi/x86: Ignore unrealistically large option ROMs

From: Ingo Molnar
Date: Thu Jun 21 2018 - 11:14:08 EST



* Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:

> On 14 May 2018 at 09:50, tip-bot for Hans de Goede <tipbot@xxxxxxxxx> wrote:
> > Commit-ID: 1de3a1be8a9345fd0c7d9bb1009b21afe6b6b10f
> > Gitweb: https://git.kernel.org/tip/1de3a1be8a9345fd0c7d9bb1009b21afe6b6b10f
> > Author: Hans de Goede <hdegoede@xxxxxxxxxx>
> > AuthorDate: Fri, 4 May 2018 08:00:01 +0200
> > Committer: Ingo Molnar <mingo@xxxxxxxxxx>
> > CommitDate: Mon, 14 May 2018 08:57:49 +0200
> >
> > efi/x86: Ignore unrealistically large option ROMs
> >
> > setup_efi_pci() tries to save a copy of each PCI option ROM as this may
> > be necessary for the device driver for the PCI device to have access too.
> >
> > On some systems the efi_pci_io_protocol's romimage and romsize fields
> > contain invalid data, which looks a bit like pointers pointing back into
> > other EFI code or data. Interpreting these pointers as romsize leads to
> > a very large value and if we then try to alloc this amount of memory to
> > save a copy the alloc call fails.
> >
> > This leads to a "Failed to alloc mem for rom" error being printed on the
> > EFI console for each PCI device.
> >
> > This commit avoids the printing of these errors, by checking romsize before
> > doing the alloc and if it is larger then the EFI spec limit of 16 MiB
> > silently ignore the ROM fields instead of trying to alloc mem and fail.
> >
> > Tested-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > [ardb: deduplicate 32/64 bit changes, use SZ_16M symbolic constant]
> > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>
> This looks odd now: I sent this out as
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> [ardb: deduplicate 32/64 bit changes, use SZ_16M symbolic constant]
> Tested-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>
> which clearly conveys that Hans tested the updated version of the patch.
>
> In general, I don't think there is a need to reorder signoffs unless
> there is anything wrong with them, no?

Indeed - this is a script bug that I failed to notice.

Thanks,

Ingo