2010/8/20 Eduardo Otubo <otubo@xxxxxxxxxxxxxxxxxx>:
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list