$subj: util: Fix cgroup processing NULL pointer dereferencing On 9/26/18 11:53 AM, Marc Hartmayer wrote:
When virCgroupEnableMissingControllers fails it's possible that *group is still set to NULL. Therefore let's add a guard and an attribute for this.
Prefix paragraph with rather than at the bottom "Fixes:". Introduced by commit 1602aa28f,
[#0] virCgroupRemove(group=0x0) [#1] virCgroupNewMachineSystemd [#2] virCgroupNewMachine [#3] qemuInitCgroup [#4] qemuSetupCgroup [#5] qemuProcessLaunch [#6] qemuProcessStart [#7] qemuDomainObjStart [#8] qemuDomainCreateWithFlags [#9] qemuDomainCreate ...
Fixes: 1602aa28f820ada66f707cef3e536e8572fbda1e
I think it's safe to remove the stack trace...
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> --- src/util/vircgroup.c | 3 ++- src/util/vircgroup.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)
While this patch is correct to remove the NULL deref, there "may" be a problem with the patch that introduced this. Rather than usurp this thread, I'll respond to the other one and see where it takes us. John
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 23957c82c7fa..06e1d158febb 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1104,7 +1104,8 @@ virCgroupNewMachineSystemd(const char *name,
error: saved = virSaveLastError(); - virCgroupRemove(*group); + if (*group) + virCgroupRemove(*group); virCgroupFree(group); if (saved) { virSetError(saved); diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 1f676f21c380..9e1ae3706b1e 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -268,7 +268,8 @@ int virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate); int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus); int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus);
-int virCgroupRemove(virCgroupPtr group); +int virCgroupRemove(virCgroupPtr group) + ATTRIBUTE_NONNULL(1);
FWIW: This only helps if someone tried to call virCgroupRemove(NULL); it doesn't help if the parameter itself is NULL. One could add a "if (!group) return 0" to virCgroupRemove to avoid.
int virCgroupKillRecursive(virCgroupPtr group, int signum); int virCgroupKillPainfully(virCgroupPtr group);