[XEN PATCH] tools/xl: Fix when config "cpus" is set, but "vcpus" is missing

Anthony PERARD posted 1 patch 6 hours ago
tools/xl/xl_parse.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
[XEN PATCH] tools/xl: Fix when config "cpus" is set, but "vcpus" is missing
Posted by Anthony PERARD 6 hours ago
From: Anthony PERARD <anthony.perard@vates.tech>

If we start a guest with 'cpus="all"' and without 'vcpus' or
'maxvcpus' (or sets them to 0), we execute parse_vcpu_affinity() with
`num_cpus=0`. This malloc "b_info->vcpu_hard_affinity" with a buffer
of size 0, which is implementation defined, and we still initialise
the "first" bitmap of this allocation, which mean we have a buffer
overflow.

On Alpine Linux, this result in a segv when the buffer is being
disposed of.

Since libxl will assume there's at least 1 vcpu, we default to 1 in
xl as well. (libxl sets max_vcpus to 1 if unset, and allocate
avail_vcpus if its size is 0.)

Link: https://gitlab.alpinelinux.org/alpine/aports/-/issues/17809
Fixes: a5dbdcf6743a ("libxl/xl: push VCPU affinity pinning down to libxl")
Signed-off-by: Anthony PERARD <anthony.perard@vates.tech>
---

Notes:
    The fixes tag is approximate, it looks like before that commit, having
    max_cpus=0 was ok, I mean no buffer overflow, but still malloc(0).

 tools/xl/xl_parse.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index af86d3186d..1a2ea8b5d5 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1518,14 +1518,22 @@ void parse_config_data(const char *config_source,
 
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         vcpus = l;
-        if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, l)) {
-            fprintf(stderr, "Unable to allocate cpumap\n");
-            exit(1);
-        }
-        libxl_bitmap_set_none(&b_info->avail_vcpus);
-        while (l-- > 0)
-            libxl_bitmap_set((&b_info->avail_vcpus), l);
     }
+    if (vcpus < 1) {
+        /*
+         * Default to 1 vCPU, libxl is already assuming this
+         * when vcpus == 0 and parse_vcpu_affinity() also assume there's at
+         * least one vcpu.
+         */
+        vcpus = 1;
+    }
+    if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, vcpus)) {
+        fprintf(stderr, "Unable to allocate cpumap\n");
+        exit(1);
+    }
+    libxl_bitmap_set_none(&b_info->avail_vcpus);
+    for (long vcpu = vcpus; vcpu-- > 0;)
+        libxl_bitmap_set((&b_info->avail_vcpus), vcpu);
 
     if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
         b_info->max_vcpus = l;
-- 
Anthony PERARD
Re: [XEN PATCH] tools/xl: Fix when config "cpus" is set, but "vcpus" is missing
Posted by Jason Andryuk 5 hours ago
On 2025-12-17 12:26, Anthony PERARD wrote:
> From: Anthony PERARD <anthony.perard@vates.tech>
> 
> If we start a guest with 'cpus="all"' and without 'vcpus' or
> 'maxvcpus' (or sets them to 0), we execute parse_vcpu_affinity() with
> `num_cpus=0`. This malloc "b_info->vcpu_hard_affinity" with a buffer
> of size 0, which is implementation defined, and we still initialise
> the "first" bitmap of this allocation, which mean we have a buffer
> overflow.
> 
> On Alpine Linux, this result in a segv when the buffer is being
> disposed of.
> 
> Since libxl will assume there's at least 1 vcpu, we default to 1 in
> xl as well. (libxl sets max_vcpus to 1 if unset, and allocate
> avail_vcpus if its size is 0.)
> 
> Link: https://gitlab.alpinelinux.org/alpine/aports/-/issues/17809
> Fixes: a5dbdcf6743a ("libxl/xl: push VCPU affinity pinning down to libxl")
> Signed-off-by: Anthony PERARD <anthony.perard@vates.tech>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Thanks,
Jason