Re: [libvirt] [PATCH] PHYP: Checking for NULL values when building new guest
2010/8/20 Eduardo Otubo <otubo(a)linux.vnet.ibm.com>:
When creating a new gust, the function phypBuildLpar() was not
checking for NULL values, making the driver to have a segmentation
fault.
---
src/phyp/phyp_driver.c | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 251111d..999870e 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -3701,6 +3701,25 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def)
int exit_status = 0;
virBuffer buf = VIR_BUFFER_INITIALIZER;
+ if (!def->name) {
+ VIR_ERROR0(_("Field \"<name>\" on the domain XML file
missing."));
+ goto err;
def->name cannot be NULL, virDomainDefParseXML fails when there is no
name given in the domain XML
+ } else if (!def->memory) {
+ VIR_ERROR0(_
+ ("Field \"<memory>\" on the domain XML file
missing."));
+ goto err;
+ } else if (!def->maxmem) {
+ VIR_ERROR0(_
+ ("Field \"<currentMemory>\" on the domain XML
file missing."));
+ goto err;
memory and maxmem might be 0 when the user sets the to 0 in the domain
XML. IMHO comparing > 0 is cleaner here.
Also the error message is misleading here. The elements aren't
missing, but the user might have specified a value that is invalid for
this driver.
+ } else if (!def->vcpus) {
+ VIR_ERROR0(_("Field \"<vcpu>\" on the domain XML file
missing."));
+ goto err;
vcpus can only be 0 when the user explicitly specified it that way,
the vcpu element isn't missing in that case. If the element is really
missing then vcpus defaults to 1.
+ } else if (!def->disks[0]->src) {
+ VIR_ERROR0(_("Field \"<disk>\" on the domain XML file
missing."));
+ goto err;
+ }
This can segfault because you dereference the first disk element
without even checking if there are any disks.
Once again the error message is misleading.
Also you should use PHYP_ERROR instead of VIR_ERROR here, because
VIR_ERROR will just output the error to the log and doesn't raise an
error at the libvirt API level, but this is required when a driver
functions fails.
In general all other occurrences of VIR_ERROR in this driver should
be replaced by PHYP_ERROR for the same reason.
There are also several instances of VIR_WARN followed by goto err that
should be PHYP_ERRORs too.
Matthias