| Message ID | 20220927154653.77296-1-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | kvm: dirty-ring: Fix race with vcpu creation | expand |
Ping?
Ping On Tue, Sep 27, 2022 at 11:46:53AM -0400, Peter Xu wrote: > It's possible that we want to reap a dirty ring on a vcpu that is during > creation, because the vcpu is put onto list (CPU_FOREACH visible) before > initialization of the structures. In this case: > > qemu_init_vcpu > x86_cpu_realizefn > cpu_exec_realizefn > cpu_list_add <---- can be probed by CPU_FOREACH > qemu_init_vcpu > cpus_accel->create_vcpu_thread(cpu); > kvm_init_vcpu > map kvm_dirty_gfns <--- kvm_dirty_gfns valid > > Don't try to reap dirty ring on vcpus during creation or it'll crash. > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2124756 > Reported-by: Xiaohui Li <xiaohli@redhat.com> > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > accel/kvm/kvm-all.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index 5acab1767f..df5fabd3a8 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -757,6 +757,15 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, CPUState *cpu) > uint32_t ring_size = s->kvm_dirty_ring_size; > uint32_t count = 0, fetch = cpu->kvm_fetch_index; > > + /* > + * It's possible that we race with vcpu creation code where the vcpu is > + * put onto the vcpus list but not yet initialized the dirty ring > + * structures. If so, skip it. > + */ > + if (!cpu->created) { > + return 0; > + } > + > assert(dirty_gfns && ring_size); > trace_kvm_dirty_ring_reap_vcpu(cpu->cpu_index); > > -- > 2.37.3 >
On 2/6/23 10:00 AM, Peter Xu wrote: > On Tue, Sep 27, 2022 at 11:46:53AM -0400, Peter Xu wrote: >> It's possible that we want to reap a dirty ring on a vcpu that is during >> creation, because the vcpu is put onto list (CPU_FOREACH visible) before >> initialization of the structures. In this case: >> >> qemu_init_vcpu >> x86_cpu_realizefn >> cpu_exec_realizefn >> cpu_list_add <---- can be probed by CPU_FOREACH >> qemu_init_vcpu >> cpus_accel->create_vcpu_thread(cpu); >> kvm_init_vcpu >> map kvm_dirty_gfns <--- kvm_dirty_gfns valid >> >> Don't try to reap dirty ring on vcpus during creation or it'll crash. >> >> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2124756 >> Reported-by: Xiaohui Li <xiaohli@redhat.com> >> Signed-off-by: Peter Xu <peterx@redhat.com> >> --- >> accel/kvm/kvm-all.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >> index 5acab1767f..df5fabd3a8 100644 >> --- a/accel/kvm/kvm-all.c >> +++ b/accel/kvm/kvm-all.c >> @@ -757,6 +757,15 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, CPUState *cpu) >> uint32_t ring_size = s->kvm_dirty_ring_size; >> uint32_t count = 0, fetch = cpu->kvm_fetch_index; >> >> + /* >> + * It's possible that we race with vcpu creation code where the vcpu is >> + * put onto the vcpus list but not yet initialized the dirty ring >> + * structures. If so, skip it. >> + */ >> + if (!cpu->created) { >> + return 0; >> + } >> + >> assert(dirty_gfns && ring_size); >> trace_kvm_dirty_ring_reap_vcpu(cpu->cpu_index); >> Reviewed-by: Gavin Shan <gshan@redhat.com>
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 5acab1767f..df5fabd3a8 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -757,6 +757,15 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, CPUState *cpu) uint32_t ring_size = s->kvm_dirty_ring_size; uint32_t count = 0, fetch = cpu->kvm_fetch_index; + /* + * It's possible that we race with vcpu creation code where the vcpu is + * put onto the vcpus list but not yet initialized the dirty ring + * structures. If so, skip it. + */ + if (!cpu->created) { + return 0; + } + assert(dirty_gfns && ring_size); trace_kvm_dirty_ring_reap_vcpu(cpu->cpu_index);
It's possible that we want to reap a dirty ring on a vcpu that is during creation, because the vcpu is put onto list (CPU_FOREACH visible) before initialization of the structures. In this case: qemu_init_vcpu x86_cpu_realizefn cpu_exec_realizefn cpu_list_add <---- can be probed by CPU_FOREACH qemu_init_vcpu cpus_accel->create_vcpu_thread(cpu); kvm_init_vcpu map kvm_dirty_gfns <--- kvm_dirty_gfns valid Don't try to reap dirty ring on vcpus during creation or it'll crash. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2124756 Reported-by: Xiaohui Li <xiaohli@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> --- accel/kvm/kvm-all.c | 9 +++++++++ 1 file changed, 9 insertions(+)