On Thursday 20 Oct 2022 at 14:38:22 (+0100), Will Deacon wrote:
> +static void
> +teardown_donated_memory(struct kvm_hyp_memcache *mc, void *addr, size_t size)
> +{
> + size = PAGE_ALIGN(size);
> + memset(addr, 0, size);
> +
> + for (void *start = addr; start < addr + size; start += PAGE_SIZE)
> + push_hyp_memcache(mc, start, hyp_virt_to_phys);
> +
> + unmap_donated_memory_noclear(addr, size);
> +}
> +
> int __pkvm_teardown_vm(pkvm_handle_t handle)
> {
> + struct kvm_hyp_memcache *mc;
> struct pkvm_hyp_vm *hyp_vm;
> unsigned int idx;
> size_t vm_size;
> @@ -552,7 +565,8 @@ int __pkvm_teardown_vm(pkvm_handle_t handle)
> hyp_spin_unlock(&vm_table_lock);
>
> /* Reclaim guest pages (including page-table pages) */
> - reclaim_guest_pages(hyp_vm);
> + mc = &hyp_vm->host_kvm->arch.pkvm.teardown_mc;
> + reclaim_guest_pages(hyp_vm, mc);
> unpin_host_vcpus(hyp_vm->vcpus, hyp_vm->nr_vcpus);
>
> /* Push the metadata pages to the teardown memcache */
> @@ -561,11 +575,11 @@ int __pkvm_teardown_vm(pkvm_handle_t handle)
> for (idx = 0; idx < hyp_vm->nr_vcpus; ++idx) {
> struct pkvm_hyp_vcpu *hyp_vcpu = hyp_vm->vcpus[idx];
>
> - unmap_donated_memory(hyp_vcpu, sizeof(*hyp_vcpu));
> + teardown_donated_memory(mc, hyp_vcpu, sizeof(*hyp_vcpu));
> }
>
> vm_size = pkvm_get_hyp_vm_size(hyp_vm->kvm.created_vcpus);
> - unmap_donated_memory(hyp_vm, vm_size);
> + teardown_donated_memory(mc, hyp_vm, vm_size);
We should move the unpinning of the host's kvm struct down here as 'mc'
here is part of it. Otherwise nothing prevents the host from unsharing
the pages and donating them, etc. Probably hard to exploit but still
worth fixing IMO.
Thanks,
Quentin
> return 0;
