This has been present since the introduction of phypAttachDevice
in commit 444fd07a.
* src/phyp/phyp_driver.c (phypAttachDevice): Don't dereference
NULL.
---
Found by clang, but the NULL dereference is very blatant.
However, I'm worried that this patch, while solving the NULL deref, is
dead wrong. In researching this patch, I also found a memory leak:
phypDomainCreateAndStart mallocs a virDomainDefPtr and passes it
phypBuildLpar, but phypBuildLpar neither stashes that memory into a
hash table nor frees it. If it were to stash it into a table, then
subsequent operations on the same domain should reuse that existing
def, rather than malloc'ing one from scratch. Conversely, if the
driver can reconstruct a def in entirety by reading lpar state, then
def should be freed, and there should be a function to recreate a def
at will. Mallocing a temporary def in functions like phypAttachDevice
is probably the wrong thing to do, when really we want to get at the
domain definition corresponding to the domain we are modifying.
src/phyp/phyp_driver.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 71a3d29..3b4235c 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1716,14 +1716,19 @@ phypAttachDevice(virDomainPtr domain, const char *xml)
char *vios_name = NULL;
char *profile = NULL;
virDomainDeviceDefPtr dev = NULL;
virDomainDefPtr def = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *domain_name = NULL;
+ if (VIR_ALLOC(def) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
domain_name = escape_specialcharacters(domain->name);
if (domain_name == NULL) {
goto cleanup;
}
def->os.type = strdup("aix");
--
1.7.4.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list