[libvirt] [PATCH] Remove re-entrant API call in SELinux/AppArmor security managers
From: "Daniel P. Berrange" <berrange@redhat.com> The security manager drivers are not allowed to call back out to top level security manager APIs, since that results in recursive mutex acquisition and thus deadlock. Remove calls to virSecurityManagerGetModel from SELinux / AppArmor drivers Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/security/security_apparmor.c | 4 ++-- src/security/security_selinux.c | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index bf795b0..f281555 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -603,12 +603,12 @@ AppArmorSetSecurityProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) if ((profile_name = get_profile_name(def)) == NULL) return rc; - if (STRNEQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (STRNEQ(SECURITY_APPARMOR_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "\'%s\' model configured for domain, but " "hypervisor driver is \'%s\'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_APPARMOR_NAME); if (use_apparmor() > 0) goto clean; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 2f5012d..cfb99a3 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1803,12 +1803,12 @@ virSecuritySELinuxSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, if (secdef == NULL) return -1; - if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (!STREQ(SECURITY_SELINUX_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_SELINUX_NAME); return -1; } @@ -1837,12 +1837,12 @@ virSecuritySELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr, return 0; VIR_DEBUG("label=%s", secdef->label); - if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (!STREQ(SECURITY_SELINUX_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_SELINUX_NAME); if (security_getenforce() == 1) return -1; } @@ -1875,12 +1875,12 @@ virSecuritySELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr, if (secdef->label == NULL) return 0; - if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (!STREQ(SECURITY_SELINUX_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_SELINUX_NAME); goto done; } @@ -1925,12 +1925,12 @@ virSecuritySELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr, if (secdef->label == NULL) return 0; - if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (!STREQ(SECURITY_SELINUX_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_SELINUX_NAME); goto done; } @@ -1966,12 +1966,12 @@ virSecuritySELinuxClearSecuritySocketLabel(virSecurityManagerPtr mgr, if (secdef->label == NULL) return 0; - if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (!STREQ(SECURITY_SELINUX_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_SELINUX_NAME); if (security_getenforce() == 1) return -1; } -- 1.7.11.7
On Mon, Feb 11, 2013 at 02:26:15PM +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The security manager drivers are not allowed to call back out to top level security manager APIs, since that results in recursive mutex acquisition and thus deadlock. Remove calls to virSecurityManagerGetModel from SELinux / AppArmor drivers
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/security/security_apparmor.c | 4 ++-- src/security/security_selinux.c | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index bf795b0..f281555 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -603,12 +603,12 @@ AppArmorSetSecurityProcessLabel(virSecurityManagerPtr mgr, virDomainDefPtr def) if ((profile_name = get_profile_name(def)) == NULL) return rc;
- if (STRNEQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (STRNEQ(SECURITY_APPARMOR_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "\'%s\' model configured for domain, but " "hypervisor driver is \'%s\'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_APPARMOR_NAME); if (use_apparmor() > 0) goto clean; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 2f5012d..cfb99a3 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1803,12 +1803,12 @@ virSecuritySELinuxSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, if (secdef == NULL) return -1;
- if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (!STREQ(SECURITY_SELINUX_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_SELINUX_NAME); return -1; }
@@ -1837,12 +1837,12 @@ virSecuritySELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr, return 0;
VIR_DEBUG("label=%s", secdef->label); - if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (!STREQ(SECURITY_SELINUX_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_SELINUX_NAME); if (security_getenforce() == 1) return -1; } @@ -1875,12 +1875,12 @@ virSecuritySELinuxSetSecurityDaemonSocketLabel(virSecurityManagerPtr mgr, if (secdef->label == NULL) return 0;
- if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (!STREQ(SECURITY_SELINUX_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_SELINUX_NAME); goto done; }
@@ -1925,12 +1925,12 @@ virSecuritySELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr, if (secdef->label == NULL) return 0;
- if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (!STREQ(SECURITY_SELINUX_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_SELINUX_NAME); goto done; }
@@ -1966,12 +1966,12 @@ virSecuritySELinuxClearSecuritySocketLabel(virSecurityManagerPtr mgr, if (secdef->label == NULL) return 0;
- if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + if (!STREQ(SECURITY_SELINUX_NAME, secdef->model)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, virSecurityManagerGetModel(mgr)); + secdef->model, SECURITY_SELINUX_NAME); if (security_getenforce() == 1) return -1; } -- 1.7.11.7
The patch causes the following warning: security/security_selinux.c: In function 'virSecuritySELinuxSetSecurityProcessLabel': security/security_selinux.c:1826:65: error: unused parameter 'mgr' [-Werror=unused-parameter] security/security_selinux.c: In function 'virSecuritySELinuxSetSecurityDaemonSocketLabel': security/security_selinux.c:1862:70: error: unused parameter 'mgr' [-Werror=unused-parameter] security/security_selinux.c: In function 'virSecuritySELinuxSetSecuritySocketLabel': security/security_selinux.c:1915:64: error: unused parameter 'mgr' [-Werror=unused-parameter] security/security_selinux.c: In function 'virSecuritySELinuxClearSecuritySocketLabel': security/security_selinux.c:1956:66: error: unused parameter 'mgr' [-Werror=unused-parameter] I switched off warnings and compiled libvirt with the patch anyway, and it fixed the problem for me (both libvirtd not being able to be killed, and libvirtd not starting up the libguestfs appliance). Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
On 02/11/2013 07:50 AM, Richard W.M. Jones wrote:
On Mon, Feb 11, 2013 at 02:26:15PM +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The security manager drivers are not allowed to call back out to top level security manager APIs, since that results in recursive mutex acquisition and thus deadlock. Remove calls to virSecurityManagerGetModel from SELinux / AppArmor drivers
The patch causes the following warning:
security/security_selinux.c: In function 'virSecuritySELinuxSetSecurityProcessLabel': security/security_selinux.c:1826:65: error: unused parameter 'mgr' [-Werror=unused-parameter]
These can be fixed by adding ATTRIBUTE_UNUSED in the function signature.
I switched off warnings and compiled libvirt with the patch anyway, and it fixed the problem for me (both libvirtd not being able to be killed, and libvirtd not starting up the libguestfs appliance).
I concur with the patch; ACK with the warnings addressed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Richard W.M. Jones