On 20.09.2012 17:29, Richard W.M. Jones wrote:
> From: "Richard W.M. Jones" <rjones@xxxxxxxxxx>
>
> This is just code motion, allowing us to reuse the same function to
> parse the <seclabel> from character devices too.
> ---
> src/conf/domain_conf.c | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 35814fb..06b3209 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3253,8 +3253,10 @@ error:
> return -1;
> }
>
> +/* Parse the <seclabel> from a disk or serial device. */
> static int
> -virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
> +virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
> + size_t *nseclabels_rtn,
> virSecurityLabelDefPtr *vmSeclabels,
> int nvmSeclabels, xmlXPathContextPtr ctxt)
> {
> @@ -3263,19 +3265,16 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
> virSecurityLabelDefPtr vmDef = NULL;
> char *model, *relabel, *label;
>
> - if (def == NULL)
> - return 0;
> -
> if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0)
> return 0;
>
> - def->nseclabels = n;
> - if (VIR_ALLOC_N(def->seclabels, n) < 0) {
> + *nseclabels_rtn = n;
> + if (VIR_ALLOC_N(*seclabels_rtn, n) < 0) {
I know it is not your fault, but this may lead to SIGSEGV. if this fails
then we proceed to error label where we dereference *seclabels_rtn.
I think the correct solution is:
- if VIR_ALLOC_N() fails, virReportOOMError(); return -1;
- set nseclabels_rtn = n; after alloc succeeds.
or check for *seclabels_rtn != NULL within error label.
> virReportOOMError();
> goto error;
> }
> for (i = 0; i < n; i++) {
> - if (VIR_ALLOC(def->seclabels[i]) < 0) {
> + if (VIR_ALLOC((*seclabels_rtn)[i]) < 0) {
> virReportOOMError();
> goto error;
> }
This is okay, since previous VIR_ALLOC zeroed the allocated memory.
> @@ -3292,7 +3291,7 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
> break;
> }
> }
> - def->seclabels[i]->model = model;
> + (*seclabels_rtn)[i]->model = model;
> }
>
> /* Can't use overrides if top-level doesn't allow relabeling. */
> @@ -3306,9 +3305,9 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
> relabel = virXMLPropString(list[i], "relabel");
> if (relabel != NULL) {
> if (STREQ(relabel, "yes")) {
> - def->seclabels[i]->norelabel = false;
> + (*seclabels_rtn)[i]->norelabel = false;
> } else if (STREQ(relabel, "no")) {
> - def->seclabels[i]->norelabel = true;
> + (*seclabels_rtn)[i]->norelabel = true;
> } else {
> virReportError(VIR_ERR_XML_ERROR,
> _("invalid security relabel value %s"),
> @@ -3318,19 +3317,19 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
> }
> VIR_FREE(relabel);
> } else {
> - def->seclabels[i]->norelabel = false;
> + (*seclabels_rtn)[i]->norelabel = false;
> }
>
> ctxt->node = list[i];
> label = virXPathStringLimit("string(./label)",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> - def->seclabels[i]->label = label;
> + (*seclabels_rtn)[i]->label = label;
>
> - if (label && def->seclabels[i]->norelabel) {
> + if (label && (*seclabels_rtn)[i]->norelabel) {
> virReportError(VIR_ERR_XML_ERROR,
> _("Cannot specify a label if relabelling is "
> "turned off. model=%s"),
> - NULLSTR(def->seclabels[i]->model));
> + NULLSTR((*seclabels_rtn)[i]->model));
> goto error;
> }
> }
> @@ -3339,10 +3338,11 @@ virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
>
> error:
> for (i = 0; i < n; i++) {
> - virSecurityDeviceLabelDefFree(def->seclabels[i]);
> + virSecurityDeviceLabelDefFree((*seclabels_rtn)[i]);
> }
> - VIR_FREE(def->seclabels);
> + VIR_FREE(*seclabels_rtn);
> VIR_FREE(list);
> + *nseclabels_rtn = 0;
> return -1;
> }
>
> @@ -3834,7 +3834,9 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> if (sourceNode) {
> xmlNodePtr saved_node = ctxt->node;
> ctxt->node = sourceNode;
> - if (virSecurityDeviceLabelDefParseXML(def, vmSeclabels,
> + if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
> + &def->nseclabels,
> + vmSeclabels,
> nvmSeclabels,
> ctxt) < 0)
> goto error;
>
ACK with the issue fixed.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list