Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [Critical] The __kvm_adjust_pc hypercall translates and uses a host-provided vcpu pointer without validation, allowing a malicious host kernel to corrupt protected guest state. - [High] __pkvm_host_donate_hyp wraps fallible allocator primitives in WARN_ON(), allowing unprivileged host userspace to trigger a hypervisor panic. -- commit f41e99c0d7eef41b448f7016f3b2a6ae6358d4c0 Author: Fuad Tabba <[email protected]> Subject: KVM: arm64: Tag host-VA hypercall parameters __kern This commit introduces the __kern sparse address space to tag host virtual addresses passed to EL2 hypercalls. It updates the hypercall declarations and unwrapping functions to ensure host VAs are explicitly translated before use, preventing unchecked dereferences at EL2. > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c > b/arch/arm64/kvm/hyp/nvhe/hyp-main.c > index 9085f46c2880f..91e74a1a334f0 100644 > --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c [ ... ] > @@ -548,9 +548,9 @@ DEFINE_KVM_HOST_HCALL(int, __pkvm_host_mkyoung_guest, > } > > DEFINE_KVM_HOST_HCALL(void, __kvm_adjust_pc, > - struct kvm_vcpu *, vcpu) > + struct kvm_vcpu __kern *, vcpu) > { > - __kvm_adjust_pc(kern_hyp_va(vcpu)); > + __kvm_adjust_pc(kern_hyp_va_host(vcpu)); > } [Severity: Critical] This is a pre-existing issue, but does this hypercall fail to validate the translated vcpu pointer? If a compromised host kernel issues the __kvm_adjust_pc hypercall and passes a forged virtual address, kern_hyp_va_host() could resolve it to the physical memory of a protected VM's pkvm_hyp_vcpu structure residing in the EL2 linear map. Since this pointer is then used directly in __kvm_adjust_pc() to modify flags like PENDING_EXCEPTION or INCREMENT_PC, could this allow a malicious host to corrupt protected guest state and bypass pKVM isolation boundaries? [ ... ] > diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c > index 6c8e33ebf8bb6..5cdb0318fa7ba 100644 > --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c > +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c [ ... ] > @@ -657,9 +657,9 @@ static size_t pkvm_get_hyp_vm_size(unsigned int nr_vcpus) > size_mul(sizeof(struct pkvm_hyp_vcpu *), nr_vcpus)); > } > > -static void *map_donated_memory_noclear(void *host_va, size_t size) > +static void *map_donated_memory_noclear(void __kern *host_va, size_t size) > { > - void *va = kern_hyp_va(host_va); > + void *va = kern_hyp_va_host(host_va); > > if (!PAGE_ALIGNED(va)) > return NULL; > @@ -671,7 +671,7 @@ static void *map_donated_memory_noclear(void *host_va, > size_t size) > return va; > } [Severity: High] This isn't a bug introduced by this patch, but can this donation path lead to a hypervisor panic triggered by unprivileged host userspace? Host userspace can issue KVM_CREATE_VM or KVM_CREATE_VCPU ioctls, which trigger hypercalls like __pkvm_init_vm and __pkvm_init_vcpu. These eventually call map_donated_memory_noclear() and __pkvm_host_donate_hyp(). Looking at arch/arm64/kvm/hyp/nvhe/mem_protect.c:__pkvm_host_donate_hyp(): __hyp_set_page_state_range(phys, size, PKVM_PAGE_OWNED); WARN_ON(pkvm_create_mappings_locked(virt, virt + size, PAGE_HYP)); WARN_ON(host_stage2_set_owner_locked(phys, size, PKVM_ID_HYP)); Both pkvm_create_mappings_locked() and host_stage2_set_owner_locked() allocate page tables from the hypervisor's memory pool and can fail with -ENOMEM if the pool is exhausted. Since WARN_ON() at EL2 expands to a fatal BRK instruction, could an unprivileged user exhaust EL2 memory and trigger these paths to crash the entire system, rather than safely returning an error and rolling back? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
