> On 4/18/25 15:44, 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.
>
> Actually I'm not convinced that this can happen; at least not in the
> same way as commit 5593473a1e6c, because KVM_RUN can only be invoked
> after the "point of no return" of create_vcpu_fd().
>
> The patch is good anyway because it's cleaner to use the same order as
> in kvm_vcpu_destroy(), but perhaps you can also move
> kvm_dirty_ring_alloc() before kvm_arch_vcpu_create(), so that the order
> remains opposite for creation and destruction?
As you pointed out, I'm also unsure whether this can actually be
triggered.
I agree that the fix following the approach of commit 5593473a1e6c will
work. However, I wonder if swapping the order of kvm_dirty_ring_alloc()
and kvm_arch_vcpu_create() might introduce other potential issues?
> Thanks,
>
> Paolo
--
Thanks,
Chen Yufeng
