[PATCH v2] virhostcpu: Fix potential use of unallocated memory
In case of a host that has a large number of cpus offline the count of
host cpus and the last bit set in the virHostCPUGetOnlineBitmap might
diverge significantly. This can e.g. be the case when disabling smt via
/sys/devices/system/cpu/smt/control.
On the host this looks like:
```
$ cat /sys/devices/system/cpu/present
0-255
$ cat /sys/devices/system/cpu/online
0-127
```
However in this case virBitmapToData previously only allocated 16 bytes
for the output bitmap. This is becase the last set bit is on the 15th
byte.
Users of virHostCPUGetMap however rely on the "cpumap" containing enough
space for all existing cpus (so they would expect 32 bytes in this case).
E.g. cmdNodeCpuMap relies on this for its output. It will then actually
read 32 bytes from the start of the "cpumap" address where in this case
the last 16 of these bytes are uninitialized.
This manifests itself in flapping outputs of "virsh nodecpumap --pretty" like:
```
$ virsh nodecpumap --pretty
CPUs present: 256
CPUs online: 128
CPU map: 0-127,192,194,202
$ virsh nodecpumap --pretty
CPUs present: 256
CPUs online: 128
CPU map: 0-127,192,194,197
$ virsh nodecpumap --pretty
CPUs present: 256
CPUs online: 128
CPU map: 0-127,192,194,196-197
```
This in turn potentially causes users of this data to report wrong cpu
counts.
Note that this only seems to happen with at least 256 physical cpus
where at least 128 are offline.
We fix this by preallocating the expected bitmap size.
Signed-off-by: Felix Huettner <felix.huettner(a)stackit.cloud>
---
src/util/virhostcpu.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 5dbcc8987c..4805484b1d 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1090,28 +1090,26 @@ virHostCPUGetMap(unsigned char **cpumap,
unsigned int flags)
{
g_autoptr(virBitmap) cpus = NULL;
- int ret = -1;
- int dummy;
+ int ncpus = virHostCPUGetCount();
virCheckFlags(0, -1);
if (!cpumap && !online)
- return virHostCPUGetCount();
+ return ncpus;
if (!(cpus = virHostCPUGetOnlineBitmap()))
- goto cleanup;
+ return -1;
+
+ if (cpumap) {
+ int len = (ncpus + CHAR_BIT) / CHAR_BIT;
+ *cpumap = g_new0(unsigned char, len);
+ virBitmapToDataBuf(cpus, *cpumap, len);
+ }
- if (cpumap)
- virBitmapToData(cpus, cpumap, &dummy);
if (online)
*online = virBitmapCountBits(cpus);
- ret = virHostCPUGetCount();
-
- cleanup:
- if (ret < 0 && cpumap)
- VIR_FREE(*cpumap);
- return ret;
+ return ncpus;
}
base-commit: c5a73f75bc6cae4f466d0a6344d5b3277ac9c2f4
--
2.43.0