On Thu, Mar 21, 2013 at 11:46:18AM +0100, Michal Privoznik wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=923946

The <seclabel type='none'/> should be added iff there is no other seclabel defined within a domain. This bug can be easily reproduced: 1) configure selinux seclabel for a domain 2) disable system's selinux and restart libvirtd 3) observe <seclabel type='none'/> being appended to a domain on its startup --- src/security/security_manager.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/security/security_manager.c b/src/security/security_manager.c index c621366..26262ed 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -425,7 +425,7 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, virDomainDefPtr vm) { int rc = 0; - size_t i; + size_t i, j, nsec_managers; virSecurityManagerPtr* sec_managers = NULL; virSecurityLabelDefPtr seclabel;

@@ -435,6 +435,26 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, if ((sec_managers = virSecurityManagerGetNested(mgr)) == NULL) return -1;

+ for (nsec_managers = 0; sec_managers[nsec_managers]; nsec_managers++) + ; + + for (i = 0; sec_managers[i]; i++) { + if (STRNEQ(sec_managers[i]->drv->name, "none")) + continue; + + /* If there's a seclabel defined for a @vm other than NOP, + * we don't want to define seclabel of type 'none' */ + for (j = 0; i < vm->nseclabels; j++) { + if (vm->seclabels[j]->type == VIR_DOMAIN_SECLABEL_NONE) + continue; + + VIR_DEBUG("Skipping NOP security manager"); + memmove(sec_managers + i, sec_managers + i + 1, + (nsec_managers - i + 1) * sizeof(sec_managers)); + break; + } + }

I don't really like this code at all.

+ virObjectLock(mgr); for (i = 0; sec_managers[i]; i++) { seclabel = virDomainDefGetSecurityLabelDef(vm,

IMHO the flaw is in this method - despite being a 'getter' it is actually modifying the the domain def to add <seclabel> elements when called. IMHO this is totally bogus behaviour that should be removed. The only code which should be adding <seclabel> is this security manager / driver code, not XML handling APIs. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|