linux-kernel - Re: [PATCH v5 33/36] KVM: arm64: Wrap the host with a stage 2

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: <ec9df9a4-a419-089c-469f-1a1509597dd9@arm.com>
Date:   Tue, 16 Mar 2021 13:28:42 +0100
From:   Mate Toth-Pal <mate.toth-pal@....com>
To:     Quentin Perret <qperret@...gle.com>, catalin.marinas@....com,
        will@...nel.org, maz@...nel.org, james.morse@....com,
        julien.thierry.kdev@...il.com, suzuki.poulose@....com
Cc:     android-kvm@...gle.com, seanjc@...gle.com,
        linux-kernel@...r.kernel.org, robh+dt@...nel.org,
        linux-arm-kernel@...ts.infradead.org, kernel-team@...roid.com,
        kvmarm@...ts.cs.columbia.edu, tabba@...gle.com, ardb@...nel.org,
        mark.rutland@....com, dbrazdil@...gle.com
Subject: Re: [PATCH v5 33/36] KVM: arm64: Wrap the host with a stage 2

On 2021-03-15 15:35, Quentin Perret wrote:
> +static int host_stage2_idmap(u64 addr)
> +{
> +	enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W;
> +	struct kvm_mem_range range;
> +	bool is_memory = find_mem_range(addr, &range);
> +	struct hyp_pool *pool = is_memory ? &host_s2_mem : &host_s2_dev;
> +	int ret;
> +
> +	if (is_memory)
> +		prot |= KVM_PGTABLE_PROT_X;
> +
>   		/* Ensure write of the host VMID */

Hi Quentin,

Testing the latest version of the patchset, we seem to have found 
another thing related to FEAT_S2FWB.

This function always sets Normal memory in the stage 2 table, even if 
the address in stage 1 was mapped as a device memory. However with the 
current settings for normal memory (i.e. MT_S2_FWB_NORMAL being defined 
to 6) according to the architecture (See Arm ARM, 'D5.5.5 Stage 2 memory 
region type and cacheability attributes when FEAT_S2FWB is implemented') 
the resulting attributes will be 'Normal Write-Back' even if the stage 1 
mapping sets device memory. Accessing device memory mapped like this 
causes an SError on some platforms with FEAT_S2FWB being implemented.

Changing the value of MT_S2_FWB_NORMAL to 7 would change this behavior, 
and the resulting memory type would be device.

Another solution would be to add an else branch to the last 'if' above 
like this:

diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c 
b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index fffa432ce3eb..54e5d3b0b2e1 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -214,6 +214,8 @@ static int host_stage2_idmap(u64 addr)

         if (is_memory)
                 prot |= KVM_PGTABLE_PROT_X;
+       else
+               prot |= KVM_PGTABLE_PROT_DEVICE;

         hyp_spin_lock(&host_kvm.lock);
         ret = kvm_pgtable_stage2_find_range(&host_kvm.pgt, addr, prot, 
&range);

Best regards,
Mate Toth-Pal