linux-kernel - RE: [PATCH V2 1/2] x86/efi: Unmap EFI boot services code/data regions from efi

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, 29 Oct 2018 19:11:09 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Prakhya, Sai Praneeth" <sai.praneeth.prakhya@...el.com>
cc:     "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>, Borislav Petkov <bp@...en8.de>,
        Ingo Molnar <mingo@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        Bhupesh Sharma <bhsharma@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>
Subject: RE: [PATCH V2 1/2] x86/efi: Unmap EFI boot services code/data regions
 from efi_pgd

Sai,

On Mon, 29 Oct 2018, Prakhya, Sai Praneeth wrote:

> Hi Thomas and Peter,
> 
> [off the mailing list because I wasn't sure if it's a good idea to spam others with my questions]

In general it's good to do on list because others can learn from the
answers as well.

> > > > +	retval = __change_page_attr_set_clr(&cpa, 0);
> > > > +	__flush_tlb_all();
> > >
> > > So this looks like you copied it from kernel_map_pages_in_pgd() which
> > > has been discussed before to be not sufficient, but it can't be
> > > changed right now due to locking issues.
> >
> > Managed to confuse myself. The place which cannot be changed is a different
> > one, but still for your call site __flush_tlb_all() might not be sufficient.
> 
> Since the mappings are changed, I thought a flush_tlb() might be needed.
> But, (to be honest), I don't completely understand the x86/mm code. So, could you 
> please elaborate the issue more for my better understanding?
> 
> So some questions I have are,
> 1. What did Peter Z mean here? "How is that not a TLB invalidation bug ?"

__flush_tlb_all() flushes only the mapping on the current CPU. So in a SMP
environment it's not sufficient.

> 2. I assumed kernel_map_pages_in_pgd() to be a good code but you said that 
> it has some locking issues. So, could you please elaborate more on that.

I corrected myself. It's the other function which cannot use the proper
cpa_flush.*() functions.

> Or, could you please provide me some pointers in the source code that I
> can take a look at so that I could understand the issue much better.

I'll have a second look at the whole thing and reply on list.

Thanks,

	tglx