linux-kernel - Re: [PATCH v19 120/130] KVM: TDX: Add a method to ignore dirty logging

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]

Date: Mon, 18 Mar 2024 16:16:56 -0700
From: Isaku Yamahata <isaku.yamahata@...el.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
Cc: "Yamahata, Isaku" <isaku.yamahata@...el.com>,
	"Zhang, Tina" <tina.zhang@...el.com>,
	"Yuan, Hang" <hang.yuan@...el.com>,
	"Huang, Kai" <kai.huang@...el.com>, "Chen, Bo2" <chen.bo@...el.com>,
	"sagis@...gle.com" <sagis@...gle.com>,
	"isaku.yamahata@...ux.intel.com" <isaku.yamahata@...ux.intel.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Aktas, Erdem" <erdemaktas@...gle.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"pbonzini@...hat.com" <pbonzini@...hat.com>,
	"seanjc@...gle.com" <seanjc@...gle.com>,
	"isaku.yamahata@...il.com" <isaku.yamahata@...il.com>
Subject: Re: [PATCH v19 120/130] KVM: TDX: Add a method to ignore dirty
 logging

On Mon, Mar 18, 2024 at 05:43:33PM +0000,
"Edgecombe, Rick P" <rick.p.edgecombe@...el.com> wrote:

> On Mon, 2024-03-18 at 10:12 -0700, Isaku Yamahata wrote:
> > I categorize as follows. Unless otherwise, I'll update this series.
> > 
> > - dirty log check
> >   As we will drop this ptach, we'll have no call site.
> > 
> > - KVM_BUG_ON() in main.c
> >   We should drop them because their logic isn't complex.
> What about "KVM: TDX: Add methods to ignore guest instruction
> emulation"? Is it cleanly blocked somehow?

KVM fault handler, kvm_mmu_page_fault(), is the caller into the emulation,
It should skip the emulation.

As the second guard, x86_emulate_instruction(), calls
check_emulate_instruction() callback to check if the emulation can/should be
done.  TDX callback can return it as X86EMUL_UNHANDLEABLE.  Then, the flow goes
to user space as error.  I'll update the vt_check_emulate_instruction().



> > - KVM_BUG_ON() in tdx.c
> >   - The error check of the return value from SEAMCALL
> >     We should keep it as it's unexpected error from TDX module. When
> > we hit
> >     this, we should mark the guest bugged and prevent further
> > operation.  It's
> >     hard to deduce the reason.  TDX mdoule might be broken.
> Yes. Makes sense.
> 
> > 
> >   - Other check
> >     We should drop them.
> 
> Offhand, I'm not sure what is in this category.

- Checking error code on TD enter/exit
  I'll revise how to check error from TD enter/exit.  We'll have new code.  I
  will update wrapper function to take struct kvm_tdx or struct tdx_vcpu, and
  revise to remove random check.  Cleanups related to kvm_rebooting,
  TDX_SW_ERROR, kvm_spurious_fault()

- Remaining random check for debug.
  The examples are as follows.  They were added for debug.


@@ -797,18 +788,14 @@ void tdx_vcpu_free(struct kvm_vcpu *vcpu)
         * list_{del,add}() on associated_tdvcpus list later.
         */
        tdx_disassociate_vp_on_cpu(vcpu);
-       WARN_ON_ONCE(vcpu->cpu != -1);
 
        /*
         * This methods can be called when vcpu allocation/initialization
         * failed. So it's possible that hkid, tdvpx and tdvpr are not assigned
         * yet.
         */
-       if (is_hkid_assigned(to_kvm_tdx(vcpu->kvm))) {
-               WARN_ON_ONCE(tdx->tdvpx_pa);
-               WARN_ON_ONCE(tdx->tdvpr_pa);
+       if (is_hkid_assigned(to_kvm_tdx(vcpu->kvm)))
                return;
-       }
 
        if (tdx->tdvpx_pa) {
                for (i = 0; i < tdx_info->nr_tdvpx_pages; i++) {
@@ -831,9 +818,9 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 {
 
        /* vcpu_deliver_init method silently discards INIT event. */
-       if (KVM_BUG_ON(init_event, vcpu->kvm))
+       if (init_event)
                return;
-       if (KVM_BUG_ON(is_td_vcpu_created(to_tdx(vcpu)), vcpu->kvm))
+       if (is_td_vcpu_created(to_tdx(vcpu)))
                return;
 
        /*
@@ -831,9 +818,9 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 {
 
        /* vcpu_deliver_init method silently discards INIT event. */
-       if (KVM_BUG_ON(init_event, vcpu->kvm))
+       if (init_event)
                return;
-       if (KVM_BUG_ON(is_td_vcpu_created(to_tdx(vcpu)), vcpu->kvm))
+       if (is_td_vcpu_created(to_tdx(vcpu)))
                return;
 
        /*
@@ -831,9 +818,9 @@ void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 {
 
        /* vcpu_deliver_init method silently discards INIT event. */
-       if (KVM_BUG_ON(init_event, vcpu->kvm))
+       if (init_event)
                return;
-       if (KVM_BUG_ON(is_td_vcpu_created(to_tdx(vcpu)), vcpu->kvm))
+       if (is_td_vcpu_created(to_tdx(vcpu)))
                return;
 
        /*

  
-- 
Isaku Yamahata <isaku.yamahata@...el.com>