linux-kernel - Re: [EXTERNAL] There is a null-ptr-deref bug in kvm_dirty_ring_get in virt/kvm/dirty

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:   Tue, 16 Nov 2021 16:22:07 +0000
From:   David Woodhouse <dwmw2@...radead.org>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        butt3rflyh4ck <butterflyhuangxx@...il.com>
Cc:     kvm@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [EXTERNAL] There is a null-ptr-deref bug in kvm_dirty_ring_get
 in virt/kvm/dirty_ring.c

On Thu, 2021-10-21 at 22:08 +0200, Paolo Bonzini wrote:
> On 18/10/21 19:14, butt3rflyh4ck wrote:
> > {
> > struct kvm_vcpu *vcpu = kvm_get_running_vcpu();  //-------> invoke
> > kvm_get_running_vcpu() to get a vcpu.
> > 
> > WARN_ON_ONCE(vcpu->kvm != kvm); [1]
> > 
> > return &vcpu->dirty_ring;
> > }
> > ```
> > but we had not called KVM_CREATE_VCPU ioctl to create a kvm_vcpu so
> > vcpu is NULL.
> 
> It's not just because there was no call to KVM_CREATE_VCPU; in general
> kvm->dirty_ring_size only works if all writes are associated to a
> specific vCPU, which is not the case for the one of
> kvm_xen_shared_info_init.
> 
> David, what do you think?  Making dirty-page ring buffer incompatible
> with Xen is ugly and I'd rather avoid it; taking the mutex for vcpu 0 is
> not an option because, as the reporter said, you might not have even
> created a vCPU yet when you call KVM_XEN_HVM_SET_ATTR.  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...

Sorry for delayed response.

So, from the point of view of Xen guests setting a shinfo page at
runtime, there *is* a vCPU running, and it's made the
XENMAPSPACE_shared_info hypercall to request that the shinfo page be
mapped.

So from KVM's point of view, that vCPU will have exited with
KVM_EXIT_XEN / KVM_EXIT_XEN_HCALL. The VMM is then invoking the
KVM_XEN_HVM_SET_ATTR ioctl from that vCPU's userspace thread, while the
KVM vCPU is idle.

I suppose we could make it a KVM_XEN_VCPU_SET_ATTR instead, and thus
associate it with a particular CPU at least for the initial wallclock
write?

Interrupts get delivered to the shinfo page too, but those will also
have a vCPU associated with them.

On live migration / live update the solution isn't quite so natural;
right now it *is* restored before any vCPUs are created. But if the
kernel made it a KVM_XEN_VCPU_SET_ATTR then the VMM will just have to
restore it as part of restoring the BSP or something. That can work?

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