linux-kernel - Re: [RFC PATCH v2 17/26] KVM: arm64: Elevate Hyp mappings creation at EL2

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:   Fri, 5 Feb 2021 18:01:13 +0000
From:   Will Deacon <will@...nel.org>
To:     Quentin Perret <qperret@...gle.com>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Marc Zyngier <maz@...nel.org>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        devicetree@...r.kernel.org, android-kvm@...gle.com,
        linux-kernel@...r.kernel.org, kernel-team@...roid.com,
        kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
        Fuad Tabba <tabba@...gle.com>,
        Mark Rutland <mark.rutland@....com>,
        David Brazdil <dbrazdil@...gle.com>
Subject: Re: [RFC PATCH v2 17/26] KVM: arm64: Elevate Hyp mappings creation
 at EL2

On Thu, Feb 04, 2021 at 11:08:33AM +0000, Quentin Perret wrote:
> On Wednesday 03 Feb 2021 at 15:31:39 (+0000), Will Deacon wrote:
> > On Fri, Jan 08, 2021 at 12:15:15PM +0000, Quentin Perret wrote:
> > > @@ -1481,7 +1486,10 @@ static void cpu_set_hyp_vector(void)
> > >  	struct bp_hardening_data *data = this_cpu_ptr(&bp_hardening_data);
> > >  	void *vector = hyp_spectre_vector_selector[data->slot];
> > >  
> > > -	*this_cpu_ptr_hyp_sym(kvm_hyp_vector) = (unsigned long)vector;
> > > +	if (!is_protected_kvm_enabled())
> > > +		*this_cpu_ptr_hyp_sym(kvm_hyp_vector) = (unsigned long)vector;
> > > +	else
> > > +		kvm_call_hyp_nvhe(__pkvm_cpu_set_vector, data->slot);
> > 
> > *Very* minor nit, but it might be cleaner to have static inline functions
> > with the same prototypes as the hypercalls, just to make the code even
> > easier to read. e.g
> > 
> > 	if (!is_protected_kvm_enabled())
> > 		_cpu_set_vector(data->slot);
> > 	else
> > 		kvm_call_hyp_nvhe(__pkvm_cpu_set_vector, data->slot);
> > 
> > you could then conceivably wrap that in a macro and avoid having the
> > "is_protected_kvm_enabled()" checks explicit every time.
> 
> Happy to do this here, but are you suggesting to generalize this pattern
> to other places as well?

I think it's probably a good pattern to follow, but no need to generalise it
prematurely.

> > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > > index 3cf9397dabdb..9d4c9251208e 100644
> > > --- a/arch/arm64/kvm/mmu.c
> > > +++ b/arch/arm64/kvm/mmu.c
> > > @@ -225,15 +225,39 @@ void free_hyp_pgds(void)
> > >  	if (hyp_pgtable) {
> > >  		kvm_pgtable_hyp_destroy(hyp_pgtable);
> > >  		kfree(hyp_pgtable);
> > > +		hyp_pgtable = NULL;
> > >  	}
> > >  	mutex_unlock(&kvm_hyp_pgd_mutex);
> > >  }
> > >  
> > > +static bool kvm_host_owns_hyp_mappings(void)
> > > +{
> > > +	if (static_branch_likely(&kvm_protected_mode_initialized))
> > > +		return false;
> > > +
> > > +	/*
> > > +	 * This can happen at boot time when __create_hyp_mappings() is called
> > > +	 * after the hyp protection has been enabled, but the static key has
> > > +	 * not been flipped yet.
> > > +	 */
> > > +	if (!hyp_pgtable && is_protected_kvm_enabled())
> > > +		return false;
> > > +
> > > +	BUG_ON(!hyp_pgtable);
> > 
> > Can we fail more gracefully, e.g. by continuing without KVM?
> 
> Got any suggestion as to how that can be done? We could also just remove
> that line -- that really should not happen.

Or downgrade to WARN_ON.

Will