[libvirt] [PATCH] Fix crash in DAC driver with no seclabels
With dynamic_ownership = 1 but no seclabels, RestoreChardevLabel dereferences the NULL seclabel when checking if norelabel is set. Remove this check, since it is already done in RestoreSecurityAllLabel and if norelabel is set, RestoreChardevLabel is never called. --- src/security/security_dac.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 05303e7..00f47cb 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -768,22 +768,19 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, static int virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr def, + virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainChrDefPtr dev, virDomainChrSourceDefPtr dev_source) { - virSecurityLabelDefPtr seclabel; virSecurityDeviceLabelDefPtr chr_seclabel = NULL; char *in = NULL, *out = NULL; int ret = -1; - seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - if (dev) chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev, SECURITY_DAC_NAME); - if (seclabel->norelabel || (chr_seclabel && chr_seclabel->norelabel)) + if (chr_seclabel && chr_seclabel->norelabel) return 0; switch ((enum virDomainChrType) dev_source->type) { -- 1.8.3.2
On 05/19/2014 02:59 PM, Ján Tomko wrote:
With dynamic_ownership = 1 but no seclabels, RestoreChardevLabel dereferences the NULL seclabel when checking if norelabel is set.
Uh, ACK :-) (Since this patch allows a newly rebuilt libvirtd to once again startup without an immediate crash)
Remove this check, since it is already done in RestoreSecurityAllLabel and if norelabel is set, RestoreChardevLabel is never called. --- src/security/security_dac.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 05303e7..00f47cb 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -768,22 +768,19 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
static int virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr def, + virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainChrDefPtr dev, virDomainChrSourceDefPtr dev_source) { - virSecurityLabelDefPtr seclabel; virSecurityDeviceLabelDefPtr chr_seclabel = NULL; char *in = NULL, *out = NULL; int ret = -1;
- seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - if (dev) chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev, SECURITY_DAC_NAME);
- if (seclabel->norelabel || (chr_seclabel && chr_seclabel->norelabel)) + if (chr_seclabel && chr_seclabel->norelabel) return 0;
switch ((enum virDomainChrType) dev_source->type) {
On 05/19/2014 03:20 PM, Laine Stump wrote:
On 05/19/2014 02:59 PM, Ján Tomko wrote:
With dynamic_ownership = 1 but no seclabels, RestoreChardevLabel dereferences the NULL seclabel when checking if norelabel is set.
Uh, ACK :-) (Since this patch allows a newly rebuilt libvirtd to once again startup without an immediate crash)
Thanks, pushed now. Jan
Ján Tomko wrote:
With dynamic_ownership = 1 but no seclabels, RestoreChardevLabel dereferences the NULL seclabel when checking if norelabel is set.
Yikes!
Remove this check, since it is already done in RestoreSecurityAllLabel and if norelabel is set, RestoreChardevLabel is never called.
I missed removing this after doing the same analysis for virSecurityDACSetChardevLabel. Sorry for crashing everyone's libvirtds :-(. Regards, Jim
--- src/security/security_dac.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 05303e7..00f47cb 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -768,22 +768,19 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
static int virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, - virDomainDefPtr def, + virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainChrDefPtr dev, virDomainChrSourceDefPtr dev_source) { - virSecurityLabelDefPtr seclabel; virSecurityDeviceLabelDefPtr chr_seclabel = NULL; char *in = NULL, *out = NULL; int ret = -1;
- seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); - if (dev) chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev, SECURITY_DAC_NAME);
- if (seclabel->norelabel || (chr_seclabel && chr_seclabel->norelabel)) + if (chr_seclabel && chr_seclabel->norelabel) return 0;
switch ((enum virDomainChrType) dev_source->type) {
participants (3)
-
Jim Fehlig -
Ján Tomko -
Laine Stump