Skip to content

Commit ef47070

Browse files
dfaggiolijbeulich
authored andcommitted
tools/libs/light: don't touch nr_vcpus_out if listing vcpus and returning NULL
If we are in libxl_list_vcpu() and we are returning NULL, let's avoid touching the output parameter *nr_vcpus_out, which the caller should have initialized to 0. The current behavior could be problematic if are creating a domain and, in the meantime, an existing one is destroyed when we have already done some steps of the loop. At which point, we'd return a NULL list of vcpus but with something different than 0 as the number of vcpus in that list. And this can cause troubles in the callers (e.g., nr_vcpus_on_nodes()), when they do a libxl_vcpuinfo_list_free(). Crashes due to this are rare and difficult to reproduce, but have been observed, with stack traces looking like this one: #0 libxl_bitmap_dispose (map=map@entry=0x50) at libxl_utils.c:626 #1 0x00007fe72c993a32 in libxl_vcpuinfo_dispose (p=p@entry=0x38) at _libxl_types.c:692 #2 0x00007fe72c94e3c4 in libxl_vcpuinfo_list_free (list=0x0, nr=<optimized out>) at libxl_utils.c:1059 xen-project#3 0x00007fe72c9528bf in nr_vcpus_on_nodes (vcpus_on_node=0x7fe71000eb60, suitable_cpumap=0x7fe721df0d38, tinfo_elements=48, tinfo=0x7fe7101b3900, gc=0x7fe7101bbfa0) at libxl_numa.c:258 xen-project#4 libxl__get_numa_candidate (gc=gc@entry=0x7fe7100033a0, min_free_memkb=4233216, min_cpus=4, min_nodes=min_nodes@entry=0, max_nodes=max_nodes@entry=0, suitable_cpumap=suitable_cpumap@entry=0x7fe721df0d38, numa_cmpf=0x7fe72c940110 <numa_cmpf>, cndt_out=0x7fe721df0cf0, cndt_found=0x7fe721df0cb4) at libxl_numa.c:394 xen-project#5 0x00007fe72c94152b in numa_place_domain (d_config=0x7fe721df11b0, domid=975, gc=0x7fe7100033a0) at libxl_dom.c:209 xen-project#6 libxl__build_pre (gc=gc@entry=0x7fe7100033a0, domid=domid@entry=975, d_config=d_config@entry=0x7fe721df11b0, state=state@entry=0x7fe710077700) at libxl_dom.c:436 xen-project#7 0x00007fe72c92c4a5 in libxl__domain_build (gc=0x7fe7100033a0, d_config=d_config@entry=0x7fe721df11b0, domid=975, state=0x7fe710077700) at libxl_create.c:444 xen-project#8 0x00007fe72c92de8b in domcreate_bootloader_done (egc=0x7fe721df0f60, bl=0x7fe7100778c0, rc=<optimized out>) at libxl_create.c:1222 #9 0x00007fe72c980425 in libxl__bootloader_run (egc=egc@entry=0x7fe721df0f60, bl=bl@entry=0x7fe7100778c0) at libxl_bootloader.c:403 #10 0x00007fe72c92f281 in initiate_domain_create (egc=egc@entry=0x7fe721df0f60, dcs=dcs@entry=0x7fe7100771b0) at libxl_create.c:1159 #11 0x00007fe72c92f456 in do_domain_create (ctx=ctx@entry=0x7fe71001c840, d_config=d_config@entry=0x7fe721df11b0, domid=domid@entry=0x7fe721df10a8, restore_fd=restore_fd@entry=-1, send_back_fd=send_back_fd@entry=-1, params=params@entry=0x0, ao_how=0x0, aop_console_how=0x7fe721df10f0) at libxl_create.c:1856 #12 0x00007fe72c92f776 in libxl_domain_create_new (ctx=0x7fe71001c840, d_config=d_config@entry=0x7fe721df11b0, domid=domid@entry=0x7fe721df10a8, ao_how=ao_how@entry=0x0, aop_console_how=aop_console_how@entry=0x7fe721df10f0) at libxl_create.c:2075 Signed-off-by: Dario Faggioli <[email protected]> Tested-by: James Fehlig <[email protected]> Reviewed-by: Anthony PERARD <[email protected]> master commit: d9d3496 master date: 2022-01-31 10:58:07 +0100
1 parent 59a5fbd commit ef47070

File tree

1 file changed

+8
-6
lines changed

1 file changed

+8
-6
lines changed

tools/libs/light/libxl_domain.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1660,6 +1660,7 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
16601660
libxl_vcpuinfo *ptr, *ret;
16611661
xc_domaininfo_t domaininfo;
16621662
xc_vcpuinfo_t vcpuinfo;
1663+
unsigned int nr_vcpus;
16631664

16641665
if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
16651666
LOGED(ERROR, domid, "Getting infolist");
@@ -1676,33 +1677,34 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
16761677
ret = ptr = libxl__calloc(NOGC, domaininfo.max_vcpu_id + 1,
16771678
sizeof(libxl_vcpuinfo));
16781679

1679-
for (*nr_vcpus_out = 0;
1680-
*nr_vcpus_out <= domaininfo.max_vcpu_id;
1681-
++*nr_vcpus_out, ++ptr) {
1680+
for (nr_vcpus = 0;
1681+
nr_vcpus <= domaininfo.max_vcpu_id;
1682+
++nr_vcpus, ++ptr) {
16821683
libxl_bitmap_init(&ptr->cpumap);
16831684
if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap, 0))
16841685
goto err;
16851686
libxl_bitmap_init(&ptr->cpumap_soft);
16861687
if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap_soft, 0))
16871688
goto err;
1688-
if (xc_vcpu_getinfo(ctx->xch, domid, *nr_vcpus_out, &vcpuinfo) == -1) {
1689+
if (xc_vcpu_getinfo(ctx->xch, domid, nr_vcpus, &vcpuinfo) == -1) {
16891690
LOGED(ERROR, domid, "Getting vcpu info");
16901691
goto err;
16911692
}
16921693

1693-
if (xc_vcpu_getaffinity(ctx->xch, domid, *nr_vcpus_out,
1694+
if (xc_vcpu_getaffinity(ctx->xch, domid, nr_vcpus,
16941695
ptr->cpumap.map, ptr->cpumap_soft.map,
16951696
XEN_VCPUAFFINITY_SOFT|XEN_VCPUAFFINITY_HARD) == -1) {
16961697
LOGED(ERROR, domid, "Getting vcpu affinity");
16971698
goto err;
16981699
}
1699-
ptr->vcpuid = *nr_vcpus_out;
1700+
ptr->vcpuid = nr_vcpus;
17001701
ptr->cpu = vcpuinfo.cpu;
17011702
ptr->online = !!vcpuinfo.online;
17021703
ptr->blocked = !!vcpuinfo.blocked;
17031704
ptr->running = !!vcpuinfo.running;
17041705
ptr->vcpu_time = vcpuinfo.cpu_time;
17051706
}
1707+
*nr_vcpus_out = nr_vcpus;
17061708
GC_FREE;
17071709
return ret;
17081710

0 commit comments

Comments
 (0)