Re: [libvirt] [libvirt-sandbox PATCH v2] Only set SELinux seclabel if supported by the host.

On Thu, Jun 12, 2014 at 10:52:21AM +0200, Cédric Bosdonnat wrote: > This code depends on new API in libvirt-gconfig to extract the > secmodels handled by the host. > --- > > Diff to v1: > * Adapted the naming change from libvirt-gconfig > > libvirt-sandbox/libvirt-sandbox-builder.c | 68 ++++++++++++++++++++----------- > 1 file changed, 45 insertions(+), 23 deletions(-) > > diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c b/libvirt-sandbox/libvirt-sandbox-builder.c > index 48b3acc..327f144 100644 > --- a/libvirt-sandbox/libvirt-sandbox-builder.c > +++ b/libvirt-sandbox/libvirt-sandbox-builder.c > @@ -323,38 +323,60 @@ static gboolean gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *build > } > > > -static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *builder G_GNUC_UNUSED, > +static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *builder, > GVirSandboxConfig *config G_GNUC_UNUSED, > const gchar *statedir G_GNUC_UNUSED, > GVirConfigDomain *domain, > - GError **error G_GNUC_UNUSED) > + GError **error) > { > GVirConfigDomainSeclabel *sec = gvir_config_domain_seclabel_new(); > const char *label = gvir_sandbox_config_get_security_label(config); > + GVirConnection *connection = gvir_sandbox_builder_get_connection(builder); > + GVirConfigCapabilities *configCapabilities; > + GVirConfigCapabilitiesHost *hostCapabilities; > + GList *secmodels, *iter; > + gboolean supportsSelinux = FALSE; > + > + /* What security models are available on the host? */ > + if (!(configCapabilities = gvir_connection_get_capabilities(connection, error))) { configCapabilities needs to be unref'ed after use > + g_object_unref(sec); > + return FALSE; > + } > + > + hostCapabilities = gvir_config_capabilities_get_host(configCapabilities); Same for hostCapabilities > > - gvir_config_domain_seclabel_set_model(sec, "selinux"); > - if (gvir_sandbox_config_get_security_dynamic(config)) { > - gvir_config_domain_seclabel_set_type(sec, > - GVIR_CONFIG_DOMAIN_SECLABEL_DYNAMIC); > - if (label) > - gvir_config_domain_seclabel_set_baselabel(sec, label); > - else if (gvir_config_domain_get_virt_type(domain) == > - GVIR_CONFIG_DOMAIN_VIRT_LXC) > - gvir_config_domain_seclabel_set_baselabel(sec, "system_u:system_r:svirt_lxc_net_t:s0"); > - else if (gvir_config_domain_get_virt_type(domain) == > - GVIR_CONFIG_DOMAIN_VIRT_QEMU) > - gvir_config_domain_seclabel_set_baselabel(sec, "system_u:system_r:svirt_tcg_t:s0"); > - else if (gvir_config_domain_get_virt_type(domain) == > - GVIR_CONFIG_DOMAIN_VIRT_KVM) > - gvir_config_domain_seclabel_set_baselabel(sec, "system_u:system_r:svirt_t:s0"); > - } else { > - gvir_config_domain_seclabel_set_type(sec, > - GVIR_CONFIG_DOMAIN_SECLABEL_STATIC); > - if (label) > - gvir_config_domain_seclabel_set_label(sec, label); > + secmodels = gvir_config_capabilities_host_get_secmodels(hostCapabilities); > + for (iter = secmodels; iter != NULL; iter = iter->next) { > + supportsSelinux = g_str_equal(gvir_config_capabilities_host_secmodel_get_model( > + GVIR_CONFIG_CAPABILITIES_HOST_SECMODEL(iter->data)), "selinux"); > + g_object_unref(iter->data); You unref all the elements of 'secmodels', but you also need to g_list_free(secmodels) when you are done with it. I think I'd move the code setting supportsSelinux to a separate helper, this would keep gvir_sandbox_builder_construct_security() more readable.