kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Woodhouse <dwmw2@infradead.org>
To: Christian Borntraeger <borntraeger@linux.ibm.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Cc: butterflyhuangxx@gmail.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, seanjc@google.com,
	Cornelia Huck <cohuck@redhat.com>,
	Janosch Frank <frankja@linux.ibm.com>,
	David Hildenbrand <david@redhat.com>,
	linux-s390 <linux-s390@vger.kernel.org>,
	Thomas Huth <thuth@redhat.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>
Subject: Re: KVM: Warn if mark_page_dirty() is called without an active vCPU
Date: Thu, 13 Jan 2022 13:22:20 +0000	[thread overview]
Message-ID: <bfc53985a178f43a3a21796f33449570cf9e4649.camel@infradead.org> (raw)
In-Reply-To: <c685a543-a524-9c95-4b85-f53a0ff744a9@linux.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]

On Thu, 2022-01-13 at 13:51 +0100, Christian Borntraeger wrote:
> > Btw, that get_map_page() in arch/s390/kvm/interrupt.c looks like it has
> > the same use-after-free problem that kvm_map_gfn() used to have. It
> > probably wants converting to the new gfn_to_pfn_cache.
> > 
> > Take a look at how I resolve the same issue for delivering Xen event
> > channel interrupts.
> 
> Do you have a commit ID for your Xen event channel fix?

14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event
channel delivery") and the commits reworking the gfn_to_pfn_cache which
lead up to it.

Questions: In your kvm_set_routing_entry() where it calls
gmap_translate() to turn the summary_addr and ind_addr from guest
addresses to userspace virtual addresses, what protects those
translations? If the gmap changes before kvm_set_routing_entry() even
returns, what ensures that the IRQ gets retranslated?


And later in adapter_indicators_set() where you take that userspace
virtual address and pass it to your get_map_page() function, the same
question: what if userspace does a concurrent mmap() and changes the
physical page that the userspace address points to? 

In the latter case, at least it does look like you don't support
external memory accessed only by a PFN without having a corresponding
struct page. So at least you don't end up accessing a page that can now
belong to *another* guest, because the original underlying page is
locked. You're probably OK in that case, so it's just the gmap changing
that we need to focus on?

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

  reply	other threads:[~2022-01-13 13:22 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-18 17:14 There is a null-ptr-deref bug in kvm_dirty_ring_get in virt/kvm/dirty_ring.c butt3rflyh4ck
2021-10-21 20:08 ` Paolo Bonzini
2021-10-28  7:42   ` butt3rflyh4ck
2021-11-08  5:11   ` butt3rflyh4ck
2021-11-16 15:41   ` butt3rflyh4ck
2021-11-16 16:22   ` [EXTERNAL] " David Woodhouse
2021-11-16 17:07     ` David Woodhouse
2021-11-17  9:46   ` Woodhouse, David
2021-11-17 16:49     ` Paolo Bonzini
2021-11-17 18:10       ` Woodhouse, David
2021-11-20 10:16   ` KVM: Warn if mark_page_dirty() is called without an active vCPU David Woodhouse
2021-11-22 17:01     ` Sean Christopherson
2021-11-22 17:52       ` David Woodhouse
2021-11-22 18:49         ` Sean Christopherson
2022-01-13 12:06     ` Christian Borntraeger
2022-01-13 12:14       ` Paolo Bonzini
2022-01-13 12:29         ` [PATCH] KVM: avoid warning on s390 in mark_page_dirty Christian Borntraeger
2022-01-13 12:31           ` David Woodhouse
2022-01-18  8:37           ` Christian Borntraeger
2022-01-18  8:44             ` Paolo Bonzini
2022-01-18  8:53               ` Christian Borntraeger
2022-01-18 11:44                 ` Paolo Bonzini
2022-01-13 12:30         ` KVM: Warn if mark_page_dirty() is called without an active vCPU David Woodhouse
2022-01-13 12:51           ` Christian Borntraeger
2022-01-13 13:22             ` David Woodhouse [this message]
2022-01-13 15:09               ` Christian Borntraeger
2022-01-13 14:36           ` Paolo Bonzini

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=bfc53985a178f43a3a21796f33449570cf9e4649.camel@infradead.org \
    --to=dwmw2@infradead.org \
    --cc=borntraeger@linux.ibm.com \
    --cc=butterflyhuangxx@gmail.com \
    --cc=cohuck@redhat.com \
    --cc=david@redhat.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=thuth@redhat.com \
    --subject='Re: KVM: Warn if mark_page_dirty() is called without an active vCPU' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).