From 49eccab7c304e975d08dca657337e42ef8332814 Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Mon, 22 Nov 2021 15:26:00 +0000 Subject: [PATCH] ANDROID: BACKPORT: KVM: arm64: Zero protected guest pages on teardown When a protected guest is torn down, the hypervisor currently transitions the ownership of all the guest pages back to the host without further intervention, which might leak guest secrets. To prevent this, let's have the hypervisor zero the page before they can be reclaimed by the host. Signed-off-by: Quentin Perret [willdeacon@: Move pkvm_host_poison to mem_protect.h] Bug: 209580772 Change-Id: Ib39de531284fef02c1cb84b83f0819f9e6f36f9b Signed-off-by: Will Deacon --- arch/arm64/include/asm/kvm_asm.h | 1 + arch/arm64/kvm/arm.c | 4 ++ arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 2 + arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 +++ arch/arm64/kvm/hyp/nvhe/mem_protect.c | 64 +++++++++++++++++-- 5 files changed, 73 insertions(+), 6 deletions(-) diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index efbad6f97f89..0e4a0b179a85 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -63,6 +63,7 @@ enum __kvm_host_smccc_func { /* Hypercalls available after pKVM finalisation */ __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp, __KVM_HOST_SMCCC_FUNC___pkvm_host_unshare_hyp, + __KVM_HOST_SMCCC_FUNC___pkvm_host_reclaim_page, __KVM_HOST_SMCCC_FUNC___pkvm_host_donate_guest, __KVM_HOST_SMCCC_FUNC___kvm_adjust_pc, __KVM_HOST_SMCCC_FUNC___kvm_vcpu_run, diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 542f05684f4a..97b40bd1d429 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -195,6 +195,10 @@ static void kvm_shadow_destroy(struct kvm *kvm) ppages = &kvm->arch.pkvm.pinned_pages; list_for_each_entry_safe(ppage, tmp, ppages, link) { + WARN_ON(kvm_call_hyp_nvhe(__pkvm_host_reclaim_page, + page_to_pfn(ppage->page))); + cond_resched(); + account_locked_vm(mm, 1, false); unpin_user_pages_dirty_lock(&ppage->page, 1, true); list_del(&ppage->link); diff --git a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h index 998a54a52093..72a7c4a58fa6 100644 --- a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h +++ b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h @@ -54,12 +54,14 @@ extern struct host_kvm host_kvm; typedef u32 pkvm_id; static const pkvm_id pkvm_host_id = 0; static const pkvm_id pkvm_hyp_id = (1 << 16); +static const pkvm_id pkvm_host_poison = pkvm_hyp_id + 1; extern unsigned long hyp_nr_cpus; int __pkvm_prot_finalize(void); int __pkvm_host_share_hyp(u64 pfn); int __pkvm_host_unshare_hyp(u64 pfn); +int __pkvm_host_reclaim_page(u64 pfn); int __pkvm_host_donate_hyp(u64 pfn, u64 nr_pages); int __pkvm_hyp_donate_host(u64 pfn, u64 nr_pages); int __pkvm_host_share_guest(u64 pfn, u64 gfn, struct kvm_vcpu *vcpu); diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c index 32997ba4abf0..7d61b5fad4a4 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c @@ -811,6 +811,13 @@ static void handle___pkvm_host_unshare_hyp(struct kvm_cpu_context *host_ctxt) cpu_reg(host_ctxt, 1) = __pkvm_host_unshare_hyp(pfn); } +static void handle___pkvm_host_reclaim_page(struct kvm_cpu_context *host_ctxt) +{ + DECLARE_REG(u64, pfn, host_ctxt, 1); + + cpu_reg(host_ctxt, 1) = __pkvm_host_reclaim_page(pfn); +} + static void handle___pkvm_create_private_mapping(struct kvm_cpu_context *host_ctxt) { DECLARE_REG(phys_addr_t, phys, host_ctxt, 1); @@ -860,6 +867,7 @@ static const hcall_t host_hcall[] = { HANDLE_FUNC(__pkvm_host_share_hyp), HANDLE_FUNC(__pkvm_host_unshare_hyp), + HANDLE_FUNC(__pkvm_host_reclaim_page), HANDLE_FUNC(__pkvm_host_donate_guest), HANDLE_FUNC(__kvm_adjust_pc), HANDLE_FUNC(__kvm_vcpu_run), diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c index 4a6b4d1a0795..4baf271b7c51 100644 --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c @@ -304,12 +304,7 @@ static int reclaim_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep, * stage-2 so no need to waste effort trying to keep it in sync. */ phys = kvm_pte_to_phys(pte); - BUG_ON(host_stage2_set_owner_locked(phys, PAGE_SIZE, pkvm_host_id)); - - /* - * XXX: if protected guest mark the page 'dirty' instead, and zero it - * lazily on host s2 aborts. - */ + BUG_ON(host_stage2_set_owner_locked(phys, PAGE_SIZE, pkvm_host_poison)); return 0; } @@ -523,6 +518,11 @@ static kvm_pte_t kvm_init_invalid_leaf_owner(pkvm_id owner_id) return FIELD_PREP(KVM_INVALID_PTE_OWNER_MASK, owner_id); } +static pkvm_id kvm_get_owner_id(kvm_pte_t pte) +{ + return FIELD_GET(KVM_INVALID_PTE_OWNER_MASK, pte); +} + int host_stage2_set_owner_locked(phys_addr_t addr, u64 size, pkvm_id owner_id) { @@ -1767,3 +1767,55 @@ int __pkvm_host_donate_guest(u64 pfn, u64 gfn, struct kvm_vcpu *vcpu) return ret; } + +static int hyp_zero_page(phys_addr_t phys) +{ + void *addr; + + addr = hyp_fixmap_map(phys); + if (!addr) + return -EINVAL; + memset(addr, 0, PAGE_SIZE); + __clean_dcache_guest_page(addr, PAGE_SIZE); + + return hyp_fixmap_unmap(); +} + +int __pkvm_host_reclaim_page(u64 pfn) +{ + u64 addr = hyp_pfn_to_phys(pfn); + enum pkvm_page_state state; + kvm_pte_t pte; + int ret; + + host_lock_component(); + + ret = kvm_pgtable_get_leaf(&host_kvm.pgt, addr, &pte, NULL); + if (ret) + goto unlock; + + if (kvm_pte_valid(pte)) { + state = host_get_page_state(pte); + ret = (state == PKVM_PAGE_OWNED) ? 0 : -EPERM; + goto unlock; + } + + switch (kvm_get_owner_id(pte)) { + case pkvm_host_id: + ret = 0; + break; + case pkvm_host_poison: + ret = hyp_zero_page(addr); + if (ret) + goto unlock; + ret = host_stage2_set_owner_locked(addr, PAGE_SIZE, pkvm_host_id); + break; + default: + ret = -EPERM; + } + +unlock: + host_unlock_component(); + + return ret; +}