linux-kernel - Re: [PATCH v3] x86/efi: Correct ident mapping of efi old

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]
Message-ID: <20170517033529.GC29117@x1>
Date:   Wed, 17 May 2017 11:35:29 +0800
From:   Baoquan He <bhe@...hat.com>
To:     Dave Young <dyoung@...hat.com>
Cc:     linux-kernel@...r.kernel.org, bp@...en8.de, bhsharma@...hat.com,
        rja@....com, Matt Fleming <matt@...eblueprint.co.uk>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Garnier <thgarnie@...gle.com>,
        Kees Cook <keescook@...omium.org>, x86@...nel.org,
        linux-efi@...r.kernel.org
Subject: Re: [PATCH v3] x86/efi: Correct ident mapping of efi old_map when
 kalsr enabled

On 05/17/17 at 10:55am, Dave Young wrote:
> Hi, Baoquan
> >  arch/x86/platform/efi/efi_64.c | 69 +++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 61 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> > index c488625..c9dffec 100644
> > --- a/arch/x86/platform/efi/efi_64.c
> > +++ b/arch/x86/platform/efi/efi_64.c
> > @@ -71,11 +71,13 @@ static void __init early_code_mapping_set_exec(int executable)
> >  
> >  pgd_t * __init efi_call_phys_prolog(void)
> >  {
> > -	unsigned long vaddress;
> > -	pgd_t *save_pgd;
> > +	unsigned long vaddr, addr_pgd, addr_p4d, addr_pud;
> > +	pgd_t *save_pgd, *pgd_k, *pgd_efi;
> > +	p4d_t *p4d, *p4d_k, *p4d_efi;
> > +	pud_t *pud;
> >  
> >  	int pgd;
> > -	int n_pgds;
> > +	int n_pgds, i, j;
> >  
> >  	if (!efi_enabled(EFI_OLD_MEMMAP)) {
> >  		save_pgd = (pgd_t *)read_cr3();
> > @@ -88,10 +90,44 @@ pgd_t * __init efi_call_phys_prolog(void)
> >  	n_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT), PGDIR_SIZE);
> >  	save_pgd = kmalloc_array(n_pgds, sizeof(*save_pgd), GFP_KERNEL);
> >  
> > +	/*
> > +	 * Build 1:1 ident mapping for old_map usage. It needs to be noticed
> > +	 * that PAGE_OFFSET is PGDIR_SIZE aligned with KASLR disabled, while
> > +	 * PUD_SIZE ALIGNED with KASLR enabled. So for a given physical
> > +	 * address X, the pud_index(X) != pud_index(__va(X)), we can only copy
> > +	 * pud entry of __va(X) to fill in pud entry of X to build 1:1 mapping
> > +	 * . Means here we can only reuse pmd table of direct mapping.
> > +	 */
> >  	for (pgd = 0; pgd < n_pgds; pgd++) {
> > -		save_pgd[pgd] = *pgd_offset_k(pgd * PGDIR_SIZE);
> > -		vaddress = (unsigned long)__va(pgd * PGDIR_SIZE);
> > -		set_pgd(pgd_offset_k(pgd * PGDIR_SIZE), *pgd_offset_k(vaddress));
> > +		addr_pgd = (unsigned long)(pgd * PGDIR_SIZE);
> > +		vaddr = (unsigned long)__va(pgd * PGDIR_SIZE);
> > +		pgd_efi = pgd_offset_k(addr_pgd);
> > +		save_pgd[pgd] = *pgd_efi;
> > +		p4d =  p4d_alloc(&init_mm, pgd_efi, addr_pgd);
> > +
> > +		if (!p4d) {
> > +			pr_err("Failed to allocate p4d table \n");
> > +			goto out;
> > +		}
> > +		for(i=0; i<PTRS_PER_P4D; i++) {
> 
> There is code style issue here and other for loop later..

Oops, I forget running scripts/checkpatch.pl to check patch. Will change
and post v4. May post with sgi v4 fix about the sgi uv mmioh region
issue, have discussed with HPE SGI developer about that.

Thanks for pointing it out!

Thanks
Baoquan

> 
> > +			addr_p4d = addr_pgd + i * P4D_SIZE;
> > +			p4d_efi = p4d + p4d_index(addr_p4d);
> > +			pud = pud_alloc(&init_mm, p4d_efi, addr_p4d);
> > +			if (!pud) {
> > +				pr_err("Failed to allocate pud table \n");
> > +				goto out;
> > +			}
> > +			for(j=0; j<PTRS_PER_PUD; j++) {
> > +				addr_pud = addr_p4d + j * PUD_SIZE;
> > +				if (addr_pud > (max_pfn << PAGE_SHIFT))
> > +					break;
> > +				vaddr = (unsigned long)__va(addr_pud);
> > +
> > +				pgd_k = pgd_offset_k(vaddr);
> > +				p4d_k = p4d_offset(pgd_k, vaddr);
> > +				pud[j] = *pud_offset(p4d_k, vaddr);
> > +			}
> > +		}
> >  	}
> >  out:
> >  	__flush_tlb_all();
> > @@ -104,8 +140,11 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
> >  	/*
> >  	 * After the lock is released, the original page table is restored.
> >  	 */
> > -	int pgd_idx;
> > +	int pgd_idx, i;
> >  	int nr_pgds;
> > +	pgd_t *pgd;
> > +        p4d_t *p4d;
> > +        pud_t *pud;
> >  
> >  	if (!efi_enabled(EFI_OLD_MEMMAP)) {
> >  		write_cr3((unsigned long)save_pgd);
> > @@ -115,9 +154,23 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
> >  
> >  	nr_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT) , PGDIR_SIZE);
> >  
> > -	for (pgd_idx = 0; pgd_idx < nr_pgds; pgd_idx++)
> > +	for (pgd_idx = 0; pgd_idx < nr_pgds; pgd_idx++) {
> > +		pgd = pgd_offset_k(pgd_idx * PGDIR_SIZE);
> >  		set_pgd(pgd_offset_k(pgd_idx * PGDIR_SIZE), save_pgd[pgd_idx]);
> >  
> > +		if (!(pgd_val(*pgd) & _PAGE_PRESENT))
> > +			continue;
> > +		for(i=0; i<PTRS_PER_P4D; i++) {
> > +			p4d = p4d_offset(pgd, pgd_idx * PGDIR_SIZE + i * P4D_SIZE);
> > +			if (!(p4d_val(*p4d) & _PAGE_PRESENT))
> > +				continue;
> > +			pud = (pud_t*)p4d_page_vaddr(*p4d);
> > +			pud_free(&init_mm, pud);
> > +                }
> > +		p4d = (p4d_t*)pgd_page_vaddr(*pgd);
> > +		p4d_free(&init_mm, p4d);
> > +	}
> > +
> >  	kfree(save_pgd);
> >  
> >  	__flush_tlb_all();
> > -- 
> > 2.5.5
> > 
> 
> Thanks
> Dave