linux-kernel - Re: Cannot load linux after recent efi-related changes

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 19 Sep 2016 14:36:36 +0300
From:   Mike Krinkin <krinkin.m.u@...il.com>
To:     Matt Fleming <matt@...eblueprint.co.uk>
Cc:     mingo@...nel.org, tglx@...utronix.de, hpa@...or.com,
        ricardo.neri-calderon@...ux.intel.com, ard.biesheuvel@...aro.org,
        pjones@...hat.com, scott.lawson@...el.com,
        linux-kernel@...r.kernel.org, linux-efi@...r.kernel.org,
        Mike Galbraith <umgwanakikbuti@...il.com>
Subject: Re: Cannot load linux after recent efi-related changes

On Mon, Sep 19, 2016 at 12:14:24PM +0100, Matt Fleming wrote:
> On Sun, 18 Sep, at 04:14:45AM, Mike Krinkin wrote:
> > 
> > diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c
> > index cd96086..34322d1 100644
> > --- a/drivers/firmware/efi/memmap.c
> > +++ b/drivers/firmware/efi/memmap.c
> > @@ -221,8 +221,8 @@ void __init efi_memmap_insert(struct efi_memory_map *old_memmap, void *buf,
> >  	void *old, *new;
> >  
> >  	/* modifying range */
> > -	m_start = mem->range.start;
> > -	m_end = mem->range.end;
> > +	m_start = mem->range.start & ~(u64)EFI_PAGE_SIZE;
> > +	m_end = ALIGN(mem->range.end, EFI_PAGE_SIZE) - 1;
> >  	m_attr = mem->attribute;
> >  
> >  	for (old = old_memmap->map, new = buf;
> 
> Thanks for the analysis and patch Mike, but this needs fixing further
> up the call stack so that we don't map things the caller didn't
> expect.
> 
> This bug was also reported in this thread,
> 
>   https://lkml.kernel.org/r/1474005912.3930.10.camel@gmail.com

Sorry, i haven't seen that.

> 
> Could you try this patch?

Works fine for me.

> 
> ---->8----
> 
> From 7e750e3289a44fe3ad693bde45aea1ad8577dd2a Mon Sep 17 00:00:00 2001
> From: Matt Fleming <matt@...eblueprint.co.uk>
> Date: Fri, 16 Sep 2016 15:12:47 +0100
> Subject: [PATCH] x86/efi: Round EFI memmap reservations to EFI_PAGE_SIZE
> 
> Mike Galbraith reported that his machine started rebooting during boot
> after,
> 
>   commit 8e80632fb23f ("efi/esrt: Use efi_mem_reserve() and avoid a kmalloc()")
> 
> The ESRT table on his machine is 56 bytes and at no point in the
> efi_arch_mem_reserve() call path is that size rounded up to
> EFI_PAGE_SIZE, nor is the start address on an EFI_PAGE_SIZE boundary.
> 
> Since the EFI memory map only deals with whole pages, inserting an EFI
> memory region with 56 bytes results in a new entry covering zero
> pages, and completely screws up the calculations for the old regions
> that were trimmed.
> 
> Round all sizes upwards, and start addresses downwards, to the nearest
> EFI_PAGE_SIZE boundary.
> 
> Additionally, efi_memmap_insert() expects the mem::range::end value to
> be one less than the end address for the region.
> 
> Reported-by: Mike Galbraith <umgwanakikbuti@...il.com>
> Reported-by: Mike Krinkin <krinkin.m.u@...il.com>
> Cc: Peter Jones <pjones@...hat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@...aro.org>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Taku Izumi <izumi.taku@...fujitsu.com>
> Signed-off-by: Matt Fleming <matt@...eblueprint.co.uk>
> ---
>  arch/x86/platform/efi/quirks.c |  6 +++++-
>  drivers/firmware/efi/memmap.c  | 11 +++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index f14b7a9da24b..10aca63a50d7 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -201,8 +201,12 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
>  		return;
>  	}
>  
> +	size += addr % EFI_PAGE_SIZE;
> +	size = round_up(size, EFI_PAGE_SIZE);
> +	addr = round_down(addr, EFI_PAGE_SIZE);
> +
>  	mr.range.start = addr;
> -	mr.range.end = addr + size;
> +	mr.range.end = addr + size - 1;
>  	mr.attribute = md.attribute | EFI_MEMORY_RUNTIME;
>  
>  	num_entries = efi_memmap_split_count(&md, &mr.range);
> diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c
> index cd96086fd851..f03ddecd232b 100644
> --- a/drivers/firmware/efi/memmap.c
> +++ b/drivers/firmware/efi/memmap.c
> @@ -225,6 +225,17 @@ void __init efi_memmap_insert(struct efi_memory_map *old_memmap, void *buf,
>  	m_end = mem->range.end;
>  	m_attr = mem->attribute;
>  
> +	/*
> +	 * The EFI memory map deals with regions in EFI_PAGE_SIZE
> +	 * units. Ensure that the region described by 'mem' is aligned
> +	 * correctly.
> +	 */
> +	if (!IS_ALIGNED(m_start, EFI_PAGE_SIZE) ||
> +	    !IS_ALIGNED(m_end + 1, EFI_PAGE_SIZE)) {
> +		WARN_ON(1);
> +		return;
> +	}
> +
>  	for (old = old_memmap->map, new = buf;
>  	     old < old_memmap->map_end;
>  	     old += old_memmap->desc_size, new += old_memmap->desc_size) {
> -- 
> 2.9.3
>