linux-kernel - Re: [PATCH 14/25] KVM: TDX: initialize VM with TDX specific parameters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZtWUATuc5wim02rN@tlindgre-MOBL1>
Date: Mon, 2 Sep 2024 13:31:29 +0300
From: Tony Lindgren <tony.lindgren@...ux.intel.com>
To: Yan Zhao <yan.y.zhao@...el.com>
Cc: Rick Edgecombe <rick.p.edgecombe@...el.com>, seanjc@...gle.com,
	pbonzini@...hat.com, kvm@...r.kernel.org, kai.huang@...el.com,
	isaku.yamahata@...il.com, xiaoyao.li@...el.com,
	linux-kernel@...r.kernel.org,
	Isaku Yamahata <isaku.yamahata@...el.com>
Subject: Re: [PATCH 14/25] KVM: TDX: initialize VM with TDX specific
 parameters

On Thu, Aug 29, 2024 at 02:27:56PM +0800, Yan Zhao wrote:
> On Mon, Aug 12, 2024 at 03:48:09PM -0700, Rick Edgecombe wrote:
> > From: Isaku Yamahata <isaku.yamahata@...el.com>
> > 
> ...
> > +static int tdx_td_init(struct kvm *kvm, struct kvm_tdx_cmd *cmd)
> > +{
...

> > +	kvm_tdx->tsc_offset = td_tdcs_exec_read64(kvm_tdx, TD_TDCS_EXEC_TSC_OFFSET);
> > +	kvm_tdx->attributes = td_params->attributes;
> > +	kvm_tdx->xfam = td_params->xfam;
> > +
> > +	if (td_params->exec_controls & TDX_EXEC_CONTROL_MAX_GPAW)
> > +		kvm->arch.gfn_direct_bits = gpa_to_gfn(BIT_ULL(51));
> > +	else
> > +		kvm->arch.gfn_direct_bits = gpa_to_gfn(BIT_ULL(47));
> > +
> Could we introduce a initialized field in struct kvm_tdx and set it true
> here? e.g
> +       kvm_tdx->initialized = true;
> 
> Then reject vCPU creation in tdx_vcpu_create() before KVM_TDX_INIT_VM is
> executed successfully? e.g.
> 
> @@ -584,6 +589,9 @@ int tdx_vcpu_create(struct kvm_vcpu *vcpu)
>         struct kvm_tdx *kvm_tdx = to_kvm_tdx(vcpu->kvm);
>         struct vcpu_tdx *tdx = to_tdx(vcpu);
> 
> +       if (!kvm_tdx->initialized)
> +               return -EIO;
> +
>         /* TDX only supports x2APIC, which requires an in-kernel local APIC. */
>         if (!vcpu->arch.apic)
>                 return -EINVAL;
> 
> Allowing vCPU creation only after TD is initialized can prevent unexpected
> userspace access to uninitialized TD primitives.

Makes sense to check for initialized TD before allowing other calls. Maybe
the check is needed in other places too in additoin to the tdx_vcpu_create().

How about just a function to check for one or more of the already existing
initialized struct kvm_tdx values?

Regards,

Tony