Re: [PATCH v2 1/2] KVM: SEV: Explicitly disallow NULL user address for SNP_LAUNCH_UPDATE

[Sean Christopherson]

On Wed, Jul 01, 2026, Ackerley Tng wrote:
> Sean Christopherson <seanjc@xxxxxxxxxx> writes:
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 74fb15551e83..621a2eaa58f2 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -2330,9 +2330,6 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> >  	int level;
> >  	int ret;
> >
> > -	if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page))
> > -		return -EINVAL;
> > -
> >  	ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
> >  	if (ret || assigned) {
> >  		pr_debug("%s: Failed to ensure GFN 0x%llx RMP entry is initial shared state, ret: %d assigned: %d\n",
> > @@ -2421,10 +2418,12 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
> >  	     params.type != KVM_SEV_SNP_PAGE_TYPE_CPUID))
> >  		return -EINVAL;
> >
> > -	src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
> > -
> > -	if (!PAGE_ALIGNED(src))
> > +	if (params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO)
> > +		src = NULL;
> > +	else if (!params.uaddr || !PAGE_ALIGNED(params.uaddr))
> >  		return -EINVAL;
> > +	else
> > +		src = u64_to_user_ptr(params.uaddr);
> >
> 
> I think separating validation from src assignment logic is better, like
> 
>     if (!PAGE_ALIGNED(params.uaddr))
> 	return -EINVAL;
> 
>     if (!params.uaddr && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
>         return -EINVAL;
> 
>     src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL :
> u64_to_user_ptr(params.uaddr);
>
> This version is also slightly stricter, since it enforces that
> params.uaddr is aligned even if params.type is ZERO. 

That's technically an ABI change since KVM fully ignored params.uaddr before
this, i.e. is a potentially breaking change.  As noted in the changelog, KVM
should have required the address to be '0', but unfortunately we can't do that
without breaking userspace.

I highly doubt requiring the address to be page-aligned would break userspace,
but KVM also gains nothing from such a requirement, and from an ABI perspective,
placing restrictions on a value that is completely ignored is rather bizarre.

> (unless the intention is to really not care about uaddr and permit an
> unaligned uaddr?)

I don't think there was any intention (which is the whole problem).