Re: [PATCH v2] KVM: arm64: Check range args for pKVM mem transitions

[Oliver Upton <oliver.upton@linux.dev>]

On Mon, Sep 22, 2025 at 10:00:07PM +0100, Vincent Donnefort wrote:
> On Sun, Sep 21, 2025 at 12:29:08PM +0100, Marc Zyngier wrote:
> > On Fri, 19 Sep 2025 16:50:56 +0100,
> > Vincent Donnefort <vdonnefort@google.com> wrote:
> > > 
> > > There's currently no verification for host issued ranges in most of the
> > > pKVM memory transitions. The subsequent end boundary might therefore be
> > > subject to overflow and could evade the later checks.
> > > 
> > > Close this loophole with an additional check_range_args() check on a per
> > > public function basis.
> > > 
> > > host_unshare_guest transition is already protected via
> > > __check_host_shared_guest(), while assert_host_shared_guest() callers
> > > are already ignoring host checks.
> > > 
> > > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> > > 
> > > ---
> > > 
> > >  v1 -> v2:
> > >    - Also check for (nr_pages * PAGE_SIZE) overflow. (Quentin)
> > >    - Rename to check_range_args().
> > > 
> > > diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> > > index 8957734d6183..65fcd2148f59 100644
> > > --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> > > +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> > > @@ -712,6 +712,14 @@ static int __guest_check_page_state_range(struct pkvm_hyp_vm *vm, u64 addr,
> > >  	return check_page_state_range(&vm->pgt, addr, size, &d);
> > >  }
> > >  
> > > +static bool check_range_args(u64 start, u64 nr_pages, u64 *size)
> > > +{
> > > +	if (check_mul_overflow(nr_pages, PAGE_SIZE, size))
> > > +		return false;
> > > +
> > > +	return start < (start + *size);
> > 
> > I will echo Oliver's concern on v1: you probably want to convert the
> > boundary check to be inclusive of the end of the range. Otherwise, a
> > range that ends at the top of the 64bit range will be represented as
> > 0, and fail the  check despite being perfectly valid.
> 
> Do you mean allowing something like start == 0xfffffffffffff000 and size ==
> 4096?

Yes, this is what I was alluding to on v1.

> But I guess that would still put all the following checks using "addr + size" at
> risk. Also, I believe even the code in pgtable.c wouldn't support a such range
> as it is also using a u64 end boundary.

I'm not sure I follow. Ranges are pretty commonly expressed as a range
terminated by an exclusive value. This just hasn't been an issue yet as
the page table code is only ever dealing with TTBR0 or VTTBR
translations.

Anyway, I'd rather these range checks have as few assumptions of the
applied address space as possible.

Thanks,
Oliver