Re: BGRT warns again on my system

[Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>]

On Mon, Jun 20, 2016 at 01:38:19PM +0800, Dave Young wrote:
> On 06/02/16 at 10:24am, Matt Fleming wrote:
> > On Sun, 29 May, at 06:09:01AM, Môshe van der Sterre wrote:
> > > ---
> > >  arch/x86/platform/efi/efi-bgrt.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/arch/x86/platform/efi/efi-bgrt.c b/arch/x86/platform/efi/efi-bgrt.c
> > > index 6a2f569..a7fdb61 100644
> > > --- a/arch/x86/platform/efi/efi-bgrt.c
> > > +++ b/arch/x86/platform/efi/efi-bgrt.c
> > > @@ -19,6 +19,8 @@
> > >  #include <linux/efi.h>
> > >  #include <linux/efi-bgrt.h>
> > >  
> > > +#include "../../mm/physaddr.h"
> > > +
> > >  struct acpi_table_bgrt *bgrt_tab;
> > >  void *__initdata bgrt_image;
> > >  size_t __initdata bgrt_image_size;
> > > @@ -67,6 +69,11 @@ void __init efi_bgrt_init(void)
> > >  		return;
> > >  	}
> > >  
> > > +	if (!phys_addr_valid(bgrt_tab->image_address)) {
> > > +		pr_notice("Ignoring BGRT: image address is bogus\n");
> > > +		return;
> > > +	}
> > > +
> > >  	image = memremap(bgrt_tab->image_address, sizeof(bmp_header), MEMREMAP_WB);
> > >  	if (!image) {
> > >  		pr_notice("Ignoring BGRT: failed to map image header memory\n");
> > > -- 
> > > 2.4.3
> > > 
> > 
> > If this does indeed fix Andy's immediate issue (and it looks like
> > it would) I'm happy to apply this as an interim solution.
> > 
> > Once we have a persistent EFI memmap available at runtime on x86 we
> > should attempt to see whether the physical address in ->image_address
> > is covered by some region in that table. The persistent EFI memmap
> > patches should be posted in the coming weeks.
> 
> I may missed the background but I worry about who knows if it is meant
> to be used for BGRT image. Some vendors could respect the BGRT valid
> bit and just leave the address as a random address. And the random
> address could be a valid physical address though. It is possible to
> leak memory infomation. 

Not unless the random address they point to happens to work as a valid
BMP image.  And even then, we figure all this out and copy the BGRT out
of that memory before we actually create the non-early memory map.

> Could we reconsider about below commit? It may be a Windows 10 bug?
> 
> commit 66dbe99cfe30e113d2e571e68b9b6a1a8985a157
> Author: Môshe van der Sterre <me-A/3C56C7qwM@public.gmane.org>
> Date:   Mon Feb 1 22:07:03 2016 +0000
> 
>     x86/efi/bgrt: Don't ignore the BGRT if the 'valid' bit is 0

I'd certainly welcome further investigation there. But if it is a bug,
it's one that BIOSes take advantage of.