Re: [libvirt] [PATCH v2 7/10] selinux: Resolve resource leak using the default disk label

Commit id a994ef2d1 changed the mechanism to store/update the default security label from using disk->seclabels[0] to allocating one on the fly. That change allocated the label, but never saved it. This patch will save the label. The new virDomainDiskDefAddSecurityLabelDef() is a copy of the virDomainDefAddSecurityLabelDef(). --- src/conf/domain_conf.c | 51 ++++++++++++++++++++++++++++++----------- src/conf/domain_conf.h | 3 +++ src/security/security_selinux.c | 6 ++--- 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0b9ba13..7640af7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16041,26 +16041,51 @@ virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model) { virSecurityLabelDefPtr seclabel = NULL; - if (VIR_ALLOC(seclabel) < 0) { - virReportOOMError(); - return NULL; - } + if (VIR_ALLOC(seclabel) < 0) + goto no_memory; if (model) { seclabel->model = strdup(model); - if (seclabel->model == NULL) { - virReportOOMError(); - virSecurityLabelDefFree(seclabel); - return NULL; - } + if (seclabel->model == NULL) + goto no_memory; } - if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) { - virReportOOMError(); - virSecurityLabelDefFree(seclabel); - return NULL; + if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) + goto no_memory; + + def->seclabels[def->nseclabels - 1] = seclabel; + + return seclabel; + +no_memory: + virReportOOMError(); + virSecurityLabelDefFree(seclabel); + return NULL; +}

+ +virSecurityDeviceLabelDefPtr +virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model) +{ + virSecurityDeviceLabelDefPtr seclabel = NULL; + + if (VIR_ALLOC(seclabel) < 0) + goto no_memory; + + if (model) { + seclabel->model = strdup(model); + if (seclabel->model == NULL) + goto no_memory; } + + if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) + goto no_memory; + def->seclabels[def->nseclabels - 1] = seclabel; return seclabel; + +no_memory: + virReportOOMError(); + virSecurityDeviceLabelDefFree(seclabel); + return NULL; }

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ce36003..9770ffb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2222,6 +2222,9 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model); virSecurityLabelDefPtr virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model); +virSecurityDeviceLabelDefPtr +virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model); + typedef const char* (*virEventActionToStringFunc)(int type); typedef int (*virEventActionFromStringFunc)(const char *type); diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b5e1a9a..511e923 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1095,10 +1095,10 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, if (ret == 1 && !disk_seclabel) { /* If we failed to set a label, but virt_use_nfs let us * proceed anyway, then we don't need to relabel later. */ - if (VIR_ALLOC(disk_seclabel) < 0) { - virReportOOMError(); + disk_seclabel = + virDomainDiskDefAddSecurityLabelDef(disk, SECURITY_SELINUX_NAME); + if (!disk_seclabel) return -1; - } disk_seclabel->norelabel = true; ret = 0; }