> On 4/18/25 06:24, Chen Yufeng wrote:
> > A patch similar to commit 5593473a1e6c ("KVM: avoid NULL pointer
> > dereference in kvm_dirty_ring_push").
> >
> > If kvm_get_vcpu_by_id() or xa_insert() failed, kvm_vm_ioctl_create_vcpu()
> > will call kvm_dirty_ring_free(), freeing ring->dirty_gfns and setting it
> > to NULL. Then, it calls kvm_arch_vcpu_destroy(), which may call
> > kvm_dirty_ring_push() in specific call stack under the same conditions as
> > previous commit said. Finally, kvm_dirty_ring_push() will use
> > ring->dirty_gfns, leading to a NULL pointer dereference.
> >
> > Signed-off-by: Chen Yufeng <chenyufeng@xxxxxxxxx>
> > ---
> > virt/kvm/kvm_main.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index e85b33a92624..3c97e598d866 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -4178,7 +4178,9 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
> > xa_erase(&kvm->vcpu_array, vcpu->vcpu_idx);
> > unlock_vcpu_destroy:
> > mutex_unlock(&kvm->lock);
> > + kvm_arch_vcpu_destroy(vcpu);
> > kvm_dirty_ring_free(&vcpu->dirty_ring);
> > + goto vcpu_free_run_page;
>
> Makes sense, but the goto is not needed. Just move
> kvm_dirty_ring_free() above "vcpu_free_run_page:", in the same style as
> kvm_vcpu_destroy().
>
> Paolo
You're right, and I'll take your advice and send v2 patch.
> > arch_vcpu_destroy:
> > kvm_arch_vcpu_destroy(vcpu);
> > vcpu_free_run_page:
--
Thanks,
Chen Yufeng
