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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list