kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Woodhouse, David" <dwmw@amazon.co.uk>
To: "pbonzini@redhat.com" <pbonzini@redhat.com>,
	"butterflyhuangxx@gmail.com" <butterflyhuangxx@gmail.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: There is a null-ptr-deref bug in kvm_dirty_ring_get in virt/kvm/dirty_ring.c
Date: Wed, 17 Nov 2021 18:10:11 +0000	[thread overview]
Message-ID: <f83554ba7bfea1ab45d316db4b68569382727175.camel@amazon.co.uk> (raw)
In-Reply-To: <20eddd70-7abb-e1a8-a003-62ed08fc1cac@redhat.com>

On Wed, 2021-11-17 at 17:49 +0100, Paolo Bonzini wrote:
> On 11/17/21 10:46, Woodhouse, David wrote:
> > > The remaining
> > > option would be just "do not mark the page as dirty if the ring buffer
> > > is active".  This is feasible because userspace itself has passed the
> > > shared info gfn; but again, it's ugly...
> > I think I am coming to quite like that 'remaining option' as long as we
> > rephrase it as follows:
> > 
> >   KVM does not mark the shared_info page as dirty, and userspace is
> >   expected to*assume*  that it is dirty at all times. It's used for
> >   delivering event channel interrupts and the overhead of marking it
> >   dirty each time is just pointless.
> 
> For the case of dirty-bitmap, one solution could be to only set a bool
> and actually mark the page dirty lazily, at the time of
> KVM_GET_DIRTY_LOG.  For dirty-ring, I agree that it's easiest if
> userspace just "knows" the page is dirty.

TBH we get that former behaviour for free if we just do the access via
the shiny new gfn_to_pfn_cache. The page is marked dirty once, when the
cache is invalidated. I was actually tempted to avoid even setting the
dirty bit even when we write to the shinfo page.

None of which *immediately* solves it for the wall clock part because
we just call the same kvm_write_wall_clock() that the KVM MSR version
invokes, and that uses kvm_write_guest().

I think I'll just reimplement the interesting part of
kvm_write_wall_clock() directly in kvm_xen_shared_info_init(). I don't
much like the duplication but it isn't much and it's the simplest
option I see. And it actually simplifies kvm_write_wall_clock() which
no longer needs the 'sec_hi_ofs' argument. And the Xen version is also
simpler because it can just access the kernel mapping directly.



Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.



  reply	other threads:[~2021-11-17 18:10 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-18 17:14 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 [this message]
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
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=f83554ba7bfea1ab45d316db4b68569382727175.camel@amazon.co.uk \
    --to=dwmw@amazon.co.uk \
    --cc=butterflyhuangxx@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --subject='Re: There is a null-ptr-deref bug in kvm_dirty_ring_get in virt/kvm/dirty_ring.c' \
    /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).