Re: [PATCH 2/9] KVM: arm64: Add a range to __pkvm_host_share_guest()

Vincent Donnefort
2025-03-03T09:03:15Z

On Fri, Feb 28, 2025 at 07:06:46PM +0000, Quentin Perret wrote:
> On Friday 28 Feb 2025 at 10:25:18 (+0000), Vincent Donnefort wrote:
> > +int __pkvm_host_share_guest(u64 pfn, u64 gfn, u64 nr_pages, struct pkvm_hyp_vcpu *vcpu,
> > enum kvm_pgtable_prot prot)
> > {
> > struct pkvm_hyp_vm *vm = pkvm_hyp_vcpu_to_hyp_vm(vcpu);
> > u64 phys = hyp_pfn_to_phys(pfn);
> > u64 ipa = hyp_pfn_to_phys(gfn);
> > struct hyp_page *page;
> > + u64 size;
> > int ret;
> >
> > if (prot & ~KVM_PGTABLE_PROT_RWX)
> > return -EINVAL;
> >
> > - ret = check_range_allowed_memory(phys, phys + PAGE_SIZE);
>
> I'm not sure it is safe to drop this check here, see below.
>
> > + ret = __guest_check_transition_size(phys, ipa, nr_pages, &size);
> > if (ret)
> > return ret;
> >
> > host_lock_component();
> > guest_lock_component(vm);
> >
> > - ret = __guest_check_page_state_range(vcpu, ipa, PAGE_SIZE, PKVM_NOPAGE);
> > + ret = __guest_check_page_state_range(vm, ipa, size, PKVM_NOPAGE);
> > if (ret)
> > goto unlock;
> >
> > page = hyp_phys_to_page(phys);
>
> Phys really needs to be a valid memory address here for 'page' to be
> present in the vmemma -- dereference right below. So we can't rely on
> the check in __host_check_page_state_range() sadly ...

Haaa you're right. Sad to have this double check. Perhaps I won't use
__host_check_page_state_range() then.

>
> > + ret = __host_check_page_state_range(phys, size, page->host_state);
> > + if (ret)
> > + goto unlock;
> > + …