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

[Christian Borntraeger]

Am 13.01.22 um 13:30 schrieb David Woodhouse:
> 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@xxxxxxxxxx
> > > >
> > >
> > > 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.

Do you have a commit ID for your Xen event channel fix?

> 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.