From: Chuanxiao Dong <chuanxiao.dong@xxxxxxxxx> The current pkvm_pgtable_unmap API takes a phys parameter which will check if the unmap entry really maps this phys memory. This is not necessary for all the unmap cases, and actually phys is unknown to some cases. However, for some cases, still prefer to perform such check to avoid unmapping an incorrect entry because such failure is not easy to be detected but cause a serial security issue. The unmap from page state API and from the host EPT are the cases better to have such check. So introduce a safe version unmap API. The safe version unmap API will be used by page state machine and host EPT to do unmap. For the MMU page table, it can still use the normal version of the unmap. Signed-off-by: Chuanxiao Dong <chuanxiao.dong@xxxxxxxxx> --- arch/x86/kvm/vmx/pkvm/hyp/ept.c | 4 +- arch/x86/kvm/vmx/pkvm/hyp/mmu.c | 4 +- arch/x86/kvm/vmx/pkvm/hyp/mmu.h | 3 +- arch/x86/kvm/vmx/pkvm/hyp/pgtable.c | 67 ++++++++++++++++++++--------- arch/x86/kvm/vmx/pkvm/hyp/pgtable.h | 4 +- 5 files changed, 55 insertions(+), 27 deletions(-) diff --git a/arch/x86/kvm/vmx/pkvm/hyp/ept.c b/arch/x86/kvm/vmx/pkvm/hyp/ept.c index 65f3a39210db..a0793e4d02ef 100644 --- a/arch/x86/kvm/vmx/pkvm/hyp/ept.c +++ b/arch/x86/kvm/vmx/pkvm/hyp/ept.c @@ -147,9 +147,9 @@ int pkvm_host_ept_map(unsigned long vaddr_start, unsigned long phys_start, } int pkvm_host_ept_unmap(unsigned long vaddr_start, unsigned long phys_start, - unsigned long size) + unsigned long size) { - return pkvm_pgtable_unmap(&host_ept, vaddr_start, phys_start, size); + return pkvm_pgtable_unmap_safe(&host_ept, vaddr_start, phys_start, size); } static void reset_rsvds_bits_mask_ept(struct rsvd_bits_validate *rsvd_check, diff --git a/arch/x86/kvm/vmx/pkvm/hyp/mmu.c b/arch/x86/kvm/vmx/pkvm/hyp/mmu.c index b32ca706fa4b..f2fe8aa45b46 100644 --- a/arch/x86/kvm/vmx/pkvm/hyp/mmu.c +++ b/arch/x86/kvm/vmx/pkvm/hyp/mmu.c @@ -192,11 +192,11 @@ int pkvm_mmu_map(unsigned long vaddr_start, unsigned long phys_start, return ret; } -int pkvm_mmu_unmap(unsigned long vaddr_start, unsigned long phys_start, unsigned long size) +int pkvm_mmu_unmap(unsigned long vaddr_start, unsigned long size) { int ret; - ret = pkvm_pgtable_unmap(&hyp_mmu, vaddr_start, phys_start, size); + ret = pkvm_pgtable_unmap(&hyp_mmu, vaddr_start, size); return ret; } diff --git a/arch/x86/kvm/vmx/pkvm/hyp/mmu.h b/arch/x86/kvm/vmx/pkvm/hyp/mmu.h index 218e3d4ef92c..776e6ceebd4c 100644 --- a/arch/x86/kvm/vmx/pkvm/hyp/mmu.h +++ b/arch/x86/kvm/vmx/pkvm/hyp/mmu.h @@ -8,8 +8,7 @@ int pkvm_mmu_map(unsigned long vaddr_start, unsigned long phys_start, unsigned long size, int pgsz_mask, u64 prot); -int pkvm_mmu_unmap(unsigned long vaddr_start, unsigned long phys_start, - unsigned long size); +int pkvm_mmu_unmap(unsigned long vaddr_start, unsigned long size); int pkvm_early_mmu_init(struct pkvm_pgtable_cap *cap, void *mmu_pool_base, unsigned long mmu_pool_pages); diff --git a/arch/x86/kvm/vmx/pkvm/hyp/pgtable.c b/arch/x86/kvm/vmx/pkvm/hyp/pgtable.c index d55acc84f4e1..54107d4685ed 100644 --- a/arch/x86/kvm/vmx/pkvm/hyp/pgtable.c +++ b/arch/x86/kvm/vmx/pkvm/hyp/pgtable.c @@ -8,6 +8,7 @@ #include "pgtable.h" #include "memory.h" #include "debug.h" +#include "bug.h" struct pgt_walk_data { struct pkvm_pgtable *pgt; @@ -34,12 +35,11 @@ struct pkvm_pgtable_lookup_data { int level; }; -static bool leaf_mapping_allowed(struct pkvm_pgtable_ops *pgt_ops, - unsigned long vaddr, - unsigned long vaddr_end, - unsigned long phys, - int pgsz_mask, - int level) +static bool leaf_mapping_valid(struct pkvm_pgtable_ops *pgt_ops, + unsigned long vaddr, + unsigned long vaddr_end, + int pgsz_mask, + int level) { unsigned long page_size = pgt_ops->pgt_level_to_size(level); @@ -49,15 +49,27 @@ static bool leaf_mapping_allowed(struct pkvm_pgtable_ops *pgt_ops, if (!IS_ALIGNED(vaddr, page_size)) return false; - if (!IS_ALIGNED(phys, page_size)) - return false; - if (page_size > (vaddr_end - vaddr)) return false; return true; } +static bool leaf_mapping_allowed(struct pkvm_pgtable_ops *pgt_ops, + unsigned long vaddr, + unsigned long vaddr_end, + unsigned long phys, + int pgsz_mask, + int level) +{ + unsigned long page_size = pgt_ops->pgt_level_to_size(level); + + if (!IS_ALIGNED(phys, page_size)) + return false; + + return leaf_mapping_valid(pgt_ops, vaddr, vaddr_end, pgsz_mask, level); +} + static void pgtable_split(struct pkvm_pgtable_ops *pgt_ops, struct pkvm_mm_ops *mm_ops, unsigned long vaddr, unsigned long phys, @@ -224,22 +236,22 @@ static int pgtable_unmap_cb(struct pkvm_pgtable *pgt, unsigned long vaddr, * Can direct unmap if matches with a large entry or a 4K entry */ if (level == PG_LEVEL_4K || (pgt_ops->pgt_entry_huge(ptep) && - leaf_mapping_allowed(pgt_ops, vaddr, vaddr_end, - data->phys, 1 << level, level))) { - unsigned long phys = pgt_ops->pgt_entry_to_phys(ptep); - - if (phys != data->phys) { - pkvm_err("%s: unmap incorrect phys (0x%lx vs 0x%lx) at vaddr 0x%lx level %d\n", - __func__, phys, data->phys, vaddr, level); - return 0; + leaf_mapping_valid(pgt_ops, vaddr, vaddr_end, + 1 << level, level))) { + if (data->phys != INVALID_ADDR) { + unsigned long phys = pgt_ops->pgt_entry_to_phys(ptep); + + PKVM_ASSERT(phys == data->phys); } pgt_ops->pgt_set_entry(ptep, 0); flush_data->flushtlb |= true; mm_ops->put_page(ptep); - data->phys = ALIGN_DOWN(data->phys, size); - data->phys += size; + if (data->phys != INVALID_ADDR) { + data->phys = ALIGN_DOWN(data->phys, size); + data->phys += size; + } return 0; } @@ -497,7 +509,22 @@ int pkvm_pgtable_map(struct pkvm_pgtable *pgt, unsigned long vaddr_start, } int pkvm_pgtable_unmap(struct pkvm_pgtable *pgt, unsigned long vaddr_start, - unsigned long phys_start, unsigned long size) + unsigned long size) +{ + struct pkvm_pgtable_unmap_data data = { + .phys = INVALID_ADDR, + }; + struct pkvm_pgtable_walker walker = { + .cb = pgtable_unmap_cb, + .arg = &data, + .flags = PKVM_PGTABLE_WALK_LEAF | PKVM_PGTABLE_WALK_TABLE_POST, + }; + + return pgtable_walk(pgt, vaddr_start, size, &walker); +} + +int pkvm_pgtable_unmap_safe(struct pkvm_pgtable *pgt, unsigned long vaddr_start, + unsigned long phys_start, unsigned long size) { struct pkvm_pgtable_unmap_data data = { .phys = ALIGN_DOWN(phys_start, PAGE_SIZE), diff --git a/arch/x86/kvm/vmx/pkvm/hyp/pgtable.h b/arch/x86/kvm/vmx/pkvm/hyp/pgtable.h index 00d3742b7f48..61ee00ee07af 100644 --- a/arch/x86/kvm/vmx/pkvm/hyp/pgtable.h +++ b/arch/x86/kvm/vmx/pkvm/hyp/pgtable.h @@ -74,7 +74,9 @@ int pkvm_pgtable_map(struct pkvm_pgtable *pgt, unsigned long vaddr_start, unsigned long phys_start, unsigned long size, int pgsz_mask, u64 entry_prot); int pkvm_pgtable_unmap(struct pkvm_pgtable *pgt, unsigned long vaddr_start, - unsigned long phys_start, unsigned long size); + unsigned long size); +int pkvm_pgtable_unmap_safe(struct pkvm_pgtable *pgt, unsigned long vaddr_start, + unsigned long phys_start, unsigned long size); void pkvm_pgtable_lookup(struct pkvm_pgtable *pgt, unsigned long vaddr, unsigned long *pphys, u64 *pprot, int *plevel); void pkvm_pgtable_destroy(struct pkvm_pgtable *pgt); -- 2.25.1