RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

From: "Prakhya, Sai Praneeth" <sai.praneeth.prakhya@intel.com>
To: Borislav Petkov <bp@alien8.de>
Cc: "linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Chun-Yi Lee <jlee@suse.com>, "Luck, Tony" <tony.luck@intel.com>,
	Will Deacon <will.deacon@arm.com>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Bhupesh Sharma <bhsharma@redhat.com>,
	"Neri, Ricardo" <ricardo.neri@intel.com>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	Matt Fleming <matt@codeblueprint.co.uk>,
	"Zijlstra, Peter" <peter.zijlstra@intel.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	"Williams, Dan J" <dan.j.williams@intel.com>
Subject: RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()
Date: Sat, 10 Mar 2018 00:33:35 +0000	[thread overview]
Message-ID: <FFF73D592F13FD46B8700F0A279B802F2E5821F9@ORSMSX114.amr.corp.intel.com> (raw)
In-Reply-To: <20180309111157.GC10753@pd.tnic>

> > That's true! AFAIK, we don't have any issues handling NMI while in efi_pgd.
> > We might have issues only when, we are already in efi_pgd, NMI comes
> > along
> 
> Can you trigger this? Or is it something hypothetical?
> 

AFAIK, it's hypothetical. I did try to trigger the issue, but failed [1].
Maybe, I need to have some more constraints [2].

[1] https://lkml.org/lkml/2017/8/23/715
[2] https://lkml.org/lkml/2017/8/25/469

> > and NMI handler tries to touch the regions that are not mapped in
> > efi_pgd
> 
> If it is not hypothetical, the NMI handler should learn to look at CR3 first and
> return if CR3 has the efi pgd.

This solution and it's variants were discussed here [1], [2] and for varied reasons 
the community had decided to go with "Everything EFI as kthread" approach [3] [4].

Although the discussions were off my understanding, the present issue I see is, 
(and also the motivation for me to do the patch is)
when a thread tries to execute any  efi_runtime_service() we switch to efi_pgd 
(which doesn't have user space mappings) and all other subsystems in kernel 
aren't aware of this switch. This looks like a perfect case for kthread.

Kthread by definition doesn’t have user space mappings and if we run efi_runtime_services()
in a kthread context and if any other subsystem tries to access user space mappings 
while in efi_kthread, it's terminally broken [5].

There were several issues Andy, Peter and Mark raised.
One such (hypothetical) case is accessing user space from the back of an interrupt (NMI).
Others include
1. Issue specific to ARM because it runs efi_runtime_services() with interrupts enabled [6]
2. Interrupt taken while mmap_sem() is held for write that tries to access user memory [7]
3. If EFI were to have IO memory mapped at a "user" address, perf could end up reading it [8]

[1] https://lkml.org/lkml/2017/8/15/757
[2] https://lkml.org/lkml/2017/8/16/487
[3] https://lkml.org/lkml/2017/8/21/573
[4] https://lkml.org/lkml/2017/8/16/540

[5] https://lkml.org/lkml/2017/8/17/667
[6] https://lkml.org/lkml/2017/8/16/176
[7] https://lkml.org/lkml/2017/8/17/667
[8] https://lkml.org/lkml/2017/8/21/427

Regards,
Sai