Messages in this thread Patch in this message | | | Date | Tue, 17 Mar 2020 15:18:14 +0000 | From | Will Deacon <> | Subject | Re: [RFC PATCH] arm64: fix the missing ktpi= cmdline check in arm64_kernel_unmapped_at_el0() |
| |
On Tue, Mar 17, 2020 at 01:57:19PM +0000, Mark Brown wrote: > On Tue, Mar 17, 2020 at 12:43:24PM +0000, Will Deacon wrote: > > On Tue, Mar 17, 2020 at 12:10:51PM +0000, Mark Rutland wrote: > > > On Tue, Mar 17, 2020 at 07:47:08PM +0800, Hongbo Yao wrote: > > > > > Kpti cannot be disabled from the kernel cmdline after the commit > > > > 09e3c22a("arm64: Use a variable to store non-global mappings decision"). > > > > > Bring back the missing check of kpti= command-line option to fix the > > > > case where the SPE driver complains the missing "kpti-off" even it has > > > > already been set. > > > > > - return arm64_use_ng_mappings; > > > > + return arm64_use_ng_mappings && > > > > + cpus_have_const_cap(ARM64_UNMAP_KERNEL_AT_EL0); > > > > } > > > This probably isn't the right fix, since this will mean that early mappings > > will be global and we'll have to go through the painful page-table rewrite > > logic when the cap gets enabled for KASLR-enabled kernels. > > Aren't we looking for a rewrite from non-global to global here (disable > KPTI where we would otherwise have it), which we don't currently have > code for?
What I mean is that cpus_have_const_cap() will be false initially, so we'll put down global mappings early on because PTE_MAYBE_NG will be 0, which means that we'll have to invoke the rewriting code if we then realise we want non-global mappings after the caps are finalised.
> > Maybe a better bodge is something like: > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > > index 0b6715625cf6..ad10f55b7bb9 100644 > > --- a/arch/arm64/kernel/cpufeature.c > > +++ b/arch/arm64/kernel/cpufeature.c > > @@ -1085,6 +1085,8 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry, > > if (!__kpti_forced) { > > str = "KASLR"; > > __kpti_forced = 1; > > + } else if (__kpti_forced < 0) { > > + arm64_use_ng_mappings = false; > > } > > } > > That is probably a good idea but I think that runs too late to affect > the early mappings, they're done based on kaslr_requires_kpti() well > before we start secondaries. My first pass not having paged everything > back in yet is that there needs to be command line parsing in > kaslr_requires_kpti() but as things stand the command line isn't > actually ready then...
Yeah, and I think you probably run into chicken and egg problems mapping the thing. With the change above, it's true that /some/ mappings will still be nG if you pass kpti=off, but I was hoping that didn't really matter :)
What was the behaviour prior to your patch? If it used to work without any nG mappings, then I suppose we should try to restore that behaviour.
Will
| |