[libvirt] [PATCH 0/2] host-validate: Be more careful when checking for cgroups
Basically, the existing code was built around the assumption that no cgroup name could appear as part of the name of another cgroup... Unfortunately, valid cgroup names include 'cpu', 'cpuacct' and 'cpuset'. This series removes the assumption. Cheers. Andrea Bolognani (2): host-validate: Be more careful when checking for cgroup support host-validate: Be more careful when checking for cgroup mounts tools/virt-host-validate-common.c | 67 +++++++++++++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 14 deletions(-) -- 2.5.5
Simply checking whether the cgroup name appears somewhere inside /proc/self/cgroup is enough most of the time, but there are some corner cases that require a more mindful parsing. As a bonus, after the rewrite 'line' is no longer leaked. --- tools/virt-host-validate-common.c | 49 +++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 8ebf73e..18b9f20 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -265,18 +265,53 @@ static int virHostValidateCGroupSupport(const char *hvname, goto error; while ((ret = getline(&line, &len, fp)) >= 0 && !matched) { - char *tmp = strstr(line, cg_name); - if (!tmp) + char **cgroups; + char *start; + char *end; + size_t ncgroups; + size_t i; + + /* Each line in this file looks like + * + * 4:cpu,cpuacct:/machine.slice/machine-qemu\x2dtest.scope/emulator + * + * Since multiple cgroups can be part of the same line and some cgroup + * names can appear as part of other cgroup names (eg. 'cpu' is a + * prefix for both 'cpuacct' and 'cpuset'), it's not enough to simply + * check whether the cgroup name is present somewhere inside the file + */ + + /* Look for the first colon. + * The part we're interested in starts right after it */ + if (!(start = strstr(line, ":"))) { + VIR_FREE(line); continue; + } + start++; - tmp += strlen(cg_name); - if (*tmp != ',' && - *tmp != ':') + /* Look for the second colon. + * The part we're interested in ends exactly there */ + if (!(end = strstr(start, ":"))) { + VIR_FREE(line); continue; + } + *end = '\0'; - matched = true; + if (!(cgroups = virStringSplitCount(start, ",", 0, &ncgroups))) { + VIR_FREE(line); + continue; + } + + /* Look for the matching cgroup */ + for (i = 0; i < ncgroups; i++) { + if (STREQ(cgroups[i], cg_name)) + matched = true; + } + + virStringFreeListCount(cgroups, ncgroups); + VIR_FREE(line); } - VIR_FREE(line); + VIR_FORCE_FCLOSE(fp); if (!matched) goto error; -- 2.5.5
The existing code was built with the assumption that no cgroup name could appear as part of another cgroup name. Rewrite it to handle such cases correctly. --- tools/virt-host-validate-common.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index 18b9f20..ced29e3 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -340,16 +340,20 @@ static int virHostValidateCGroupMount(const char *hvname, goto error; while (getmntent_r(fp, &ent, mntbuf, sizeof(mntbuf)) && !matched) { - char *tmp = strstr(ent.mnt_opts, cg_name); - if (!tmp) - continue; + char **opts; + size_t nopts; + size_t i; - tmp += strlen(cg_name); - if (*tmp != ',' && - *tmp != '\0') + if (!(opts = virStringSplitCount(ent.mnt_opts, ",", 0, &nopts))) continue; - matched = true; + /* Look for a mount option matching the cgroup name */ + for (i = 0; i < nopts; i++) { + if (STREQ(opts[i], cg_name)) + matched = true; + } + + virStringFreeListCount(opts, nopts); } endmntent(fp); if (!matched) -- 2.5.5
On Mon, 2016-04-04 at 16:46 +0200, Andrea Bolognani wrote:
Basically, the existing code was built around the assumption that no cgroup name could appear as part of the name of another cgroup...
Unfortunately, valid cgroup names include 'cpu', 'cpuacct' and 'cpuset'.
This series removes the assumption.
v2's out: https://www.redhat.com/archives/libvir-list/2016-April/msg00182.html Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
participants (1)
-
Andrea Bolognani