linux-kernel - Re: KVM: Warn if mark_page_dirty() is called without an active vCPU

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:   Thu, 13 Jan 2022 12:30:19 +0000
From:   David Woodhouse <dwmw2@...radead.org>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>
Cc:     butterflyhuangxx@...il.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, seanjc@...gle.com,
        Cornelia Huck <cohuck@...hat.com>,
        Janosch Frank <frankja@...ux.ibm.com>,
        David Hildenbrand <david@...hat.com>,
        linux-s390 <linux-s390@...r.kernel.org>,
        Thomas Huth <thuth@...hat.com>,
        Claudio Imbrenda <imbrenda@...ux.ibm.com>
Subject: Re: KVM: Warn if mark_page_dirty() is called without an active vCPU

On Thu, 2022-01-13 at 13:14 +0100, Paolo Bonzini wrote:
> On 1/13/22 13:06, Christian Borntraeger wrote:
> > From: Christian Borntraeger<
> > borntraeger@...ibm.com
> > >
> > 
> > Quick heads-up.
> > The new warnon triggers on s390. Here we write to the guest from an
> > irqfd worker. Since we do not use dirty_ring yet this might be an
> > over-indication.
> > Still have to look into that.
> 
> Yes, it's okay to add an #ifdef around the warning.

That would be #ifndef CONFIG_HAVE_KVM_DIRTY_RING, yes? 

I already found it hard to write down the rules around how
kvm_vcpu_write_guest() doesn't use the vCPU it's passed, and how both
it and kvm_write_guest() need to be invoked on a pCPU which currently
owns *a* vCPU belonging to the same KVM... if we add "unless you're on
an architecture that doesn't support dirty ring logging", you may have
to pass me a bucket.

Are you proposing that as an officially documented part of the already
horrid API, or a temporary measure :)

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.

Although I gave myself a free pass on the dirty marking in that case,
by declaring that the shinfo page doesn't get marked dirty; it should
be considered *always* dirty. You might have less fun declaring that
retrospectively in your case.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)