linux-kernel - Re: [PATCH -v2] EFI: Runtime services virtual mapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130924124555.GA4690@dhcp-16-126.nay.redhat.com>
Date:	Tue, 24 Sep 2013 20:45:56 +0800
From:	Dave Young <dyoung@...hat.com>
To:	Borislav Petkov <bp@...en8.de>
Cc:	"H. Peter Anvin" <hpa@...or.com>, X86 ML <x86@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Borislav Petkov <bp@...e.de>,
	Matt Fleming <matt@...sole-pimps.org>,
	Matthew Garrett <mjg59@...f.ucam.org>,
	James Bottomley <james.bottomley@...senpartnership.com>,
	Vivek Goyal <vgoyal@...hat.com>, linux-efi@...r.kernel.org
Subject: Re: [PATCH -v2] EFI: Runtime services virtual mapping

On 09/24/13 at 11:43am, Borislav Petkov wrote:
> Crap,
> 
> I need to send from the web interface since the network here doesn't
> somehow let through port 587.
> 
> On Tue, September 24, 2013 6:57 am, Dave Young wrote:
> > On 09/23/13 at 08:06pm, H. Peter Anvin wrote:
> >> Okay... I see two problems.
> >>
> >> 1. It looks like we subtract the region size after, rather than before,
> >> assigning an address.
> >>
> >> 2. The second region is assigned the same address in the secondary
> >> kernel as in the first, implying the size of the first region was
> >> somehow set to zero.
> >
> > I find the reason, efi_reserve_boot_services will reserve the
> > BOOT_SERVICE_DATA region
> > thus the memmap size is changed to 0, so in 2nd kernel the virtual mapping
> > addr after
> > the md will be not same as 1st kernel, see below code:
> >
> > void __init efi_map_region(efi_memory_desc_t *md)
> > {
> >         unsigned long size = md->num_pages << PAGE_SHIFT;
> >
> >         efi_va -= size;
> >         ^^^^^^^^^^^^^^^
> 
> Anyway, yes, this is wrong. We probably want to something like the
> following, instead (patch might be whitespace-damaged):
> 
> --
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index a235dc95d629..ea0ea4fd3dab 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -85,8 +85,7 @@ void __init efi_map_region(efi_memory_desc_t *md)
>  {
>  	unsigned long size = md->num_pages << PAGE_SHIFT;
> 
> -	efi_va -= size;
> -	if (efi_va < EFI_VA_END) {
> +	if (efi_va - size < EFI_VA_END) {
>  		pr_warning(FW_WARN "VA address range overflow!\n");
>  		return;
>  	}
> @@ -101,6 +100,8 @@ void __init efi_map_region(efi_memory_desc_t *md)
>  	/* Do the VA map */
>  	__map_region(md, efi_va);
>  	md->virt_addr = efi_va;
> +
> +	efi_va -= size;
>  }
> 
>  void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long
> size,
> 

Think again about this, how about 1:1 map them from a base address like -64G
phy_addr  ->  (-64G + phy_addr), in this way we can avoid depending on the
previous region size.

For the zero region problem, we can resolve it as a standalone problem.

--
Thanks
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/