In vcpu_create after scheduler data is allocated, if
vmtrace_alloc_buffer fails, it will jump to the wrong cleanup label
resulting in a memory leak.

Move sched_destroy_vcpu and destroy_waitqueue_vcpu to vcpu_teardown.
Make vcpu_teardown idempotent: deal with NULL unit.

Fix vcpu_runstate_get (called during XEN_SYSCTL_getdomaininfolist post
vcpu_teardown) when v->sched_unit is NULL.

Fixes: 217dd79ee292 ("xen/domain: Add vmtrace_size domain creation parameter")
Signed-off-by: Stewart Hildebrand <stewart.hildebr...@amd.com>
---
v1->v2:
* move cleanup functions to vcpu_teardown
* renamed, was ("xen: fix memory leak on error in vcpu_create")
---
 xen/common/domain.c     | 14 ++++++--------
 xen/common/sched/core.c |  5 ++++-
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 5241a1629eeb..9c65c2974ea3 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -388,6 +388,8 @@ static int vmtrace_alloc_buffer(struct vcpu *v)
 static int vcpu_teardown(struct vcpu *v)
 {
     vmtrace_free_buffer(v);
+    sched_destroy_vcpu(v);
+    destroy_waitqueue_vcpu(v);
 
     return 0;
 }
@@ -448,13 +450,13 @@ struct vcpu *vcpu_create(struct domain *d, unsigned int 
vcpu_id)
     }
 
     if ( sched_init_vcpu(v) != 0 )
-        goto fail_wq;
+        goto fail;
 
     if ( vmtrace_alloc_buffer(v) != 0 )
-        goto fail_wq;
+        goto fail;
 
     if ( arch_vcpu_create(v) != 0 )
-        goto fail_sched;
+        goto fail;
 
     d->vcpu[vcpu_id] = v;
     if ( vcpu_id != 0 )
@@ -472,11 +474,7 @@ struct vcpu *vcpu_create(struct domain *d, unsigned int 
vcpu_id)
 
     return v;
 
- fail_sched:
-    sched_destroy_vcpu(v);
- fail_wq:
-    destroy_waitqueue_vcpu(v);
-
+ fail:
     /* Must not hit a continuation in this context. */
     if ( vcpu_teardown(v) )
         ASSERT_UNREACHABLE();
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 2ab4313517c3..fb7c99b05360 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -321,7 +321,7 @@ void vcpu_runstate_get(const struct vcpu *v,
      */
     unit = is_idle_vcpu(v) ? get_sched_res(v->processor)->sched_unit_idle
                            : v->sched_unit;
-    lock = likely(v == current) ? NULL : unit_schedule_lock_irq(unit);
+    lock = likely(v == current || !unit) ? NULL : unit_schedule_lock_irq(unit);
     memcpy(runstate, &v->runstate, sizeof(*runstate));
     delta = NOW() - runstate->state_entry_time;
     if ( delta > 0 )
@@ -839,6 +839,9 @@ void sched_destroy_vcpu(struct vcpu *v)
 {
     struct sched_unit *unit = v->sched_unit;
 
+    if ( !unit )
+        return;
+
     kill_timer(&v->periodic_timer);
     kill_timer(&v->singleshot_timer);
     kill_timer(&v->poll_timer);

base-commit: f6c990ac3cddc2d1965a7ab09324d821b05e4b6c
-- 
2.50.1


Reply via email to