Re: [libvirt] [PATCH] vircgroup: fix NULL pointer dereferencing
$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(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay(a)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);