All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: "Woodhouse, David" <dwmw@amazon.co.uk>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	Janosch Frank <frankja@linux.ibm.com>,
	David Hildenbrand <david@redhat.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] KVM: Don't actually set a request when evicting vCPUs for GFN cache invd
Date: Mon, 14 Feb 2022 17:14:27 +0000	[thread overview]
Message-ID: <YgqN87rqc/vogbFE@google.com> (raw)
In-Reply-To: <d79aacb5-9069-4647-9332-86f7d74b747a@email.android.com>

On Sat, Feb 12, 2022, Woodhouse, David wrote:
> 
> (Apologies if this is HTML but I'm half-way to Austria and the laptop is
> buried somewhere in the car, and access to work email with sane email apps is
> difficult.)
> 
> On 12 Feb 2022 03:05, Sean Christopherson <seanjc@google.com> wrote:
> 
> Don't actually set a request bit in vcpu->requests when making a request
> purely to force a vCPU to exit the guest.  Logging the request but not
> actually consuming it causes the vCPU to get stuck in an infinite loop
> during KVM_RUN because KVM sees a pending request and bails from VM-Enter
> to service the request.
> 
> 
> Right, but there is no extant code which does this. The guest_uses_pa flag is
> unused.

Grr.  A WARN or something would have been nice to have.  Oh well.

> The series came with a proof-of-concept that attempted using it for
> fixing nesting UAFs but it was just that — a proof of concept to demonstrate
> that the new design of GPC was sufficient to address that problem.
> 
> IIRC, said proof of concept did also actually consume the req in question,

It did.  I saw that, but obviously didn't connect the dots to guest_uses_pa.

--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9826,6 +9826,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)

                if (kvm_check_request(KVM_REQ_UPDATE_CPU_DIRTY_LOGGING, vcpu))
                        static_call(kvm_x86_update_cpu_dirty_logging)(vcpu);
+               if (kvm_check_request(KVM_REQ_GPC_INVALIDATE, vcpu))
+                       ; /* Nothing to do. It just wanted to wake us */

> and one of the existing test cases did exercise it with an additional mmap
> torture added? Of course until we have kernel code that *does* this, it's
> hard to exercise it from userspace :)

Indeed.  I'll send a new version with a different changelog, that way we're not
leaving a trap for developers and each architecture doesn't need to manually handle
the request.

Thanks!

       reply	other threads:[~2022-02-14 17:14 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <d79aacb5-9069-4647-9332-86f7d74b747a@email.android.com>
2022-02-14 17:14 ` Sean Christopherson [this message]
2022-02-14 18:50   ` [PATCH] KVM: Don't actually set a request when evicting vCPUs for GFN cache invd David Woodhouse
2022-02-12  2:04 Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YgqN87rqc/vogbFE@google.com \
    --to=seanjc@google.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=david@redhat.com \
    --cc=dwmw@amazon.co.uk \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.