lkml.org 
[lkml]   [2021]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [EXTERNAL] There is a null-ptr-deref bug in kvm_dirty_ring_get in virt/kvm/dirty_ring.c
    From
    Date
    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?
    [unhandled content-type:application/pkcs7-signature]
    \
     
     \ /
      Last update: 2021-11-16 17:23    [W:3.478 / U:0.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site