[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFcO6XMyBDU8hYNAa7s9sGHEntTz5iJmLF2QbRN-S8PPpYd_ZQ@mail.gmail.com>
Date: Tue, 16 Nov 2021 23:41:48 +0800
From: butt3rflyh4ck <butterflyhuangxx@...il.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: "Woodhouse, David" <dwmw@...zon.co.uk>, kvm@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: There is a null-ptr-deref bug in kvm_dirty_ring_get in virt/kvm/dirty_ring.c
For this issue, I have reviewed the implementation code of vm
creating,vCPU creating and dirty_ring creating,
I have some ideas.If only judge kvm->dirty_ring_size, determine
whether to call kvm_dirty_ring_push(), this condition is not
sufficient.
can we add a judgement on kvm->created_vcpus. kvm->created_vcpus is not NULL.
After all, there is a situation, no vCPU was created, but
kvm->dirty_ring_size has a value.
Regards,
butt3rflyh4ck.
On Fri, Oct 22, 2021 at 4:08 AM Paolo Bonzini <pbonzini@...hat.com> 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...
>
> Paolo
>
--
Active Defense Lab of Venustech
Powered by blists - more mailing lists