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 :|