[libvirt] [PATCH] security: fix deadlock with prefork

Attempts to start a domain with both SELinux and DAC security modules loaded will deadlock; latent problem introduced in commit fdb3bde and exposed in commit 29fe5d7. Basically, when recursing into the security manager for other driver's prefork, we have to undo the asymmetric lock taken at the manager level. Reported by Jiri Denemark, with diagnosis help from Dan Berrange. * src/security/security_stack.c (virSecurityStackPreFork): Undo extra lock grabbed during recursion. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/security/security_stack.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/security/security_stack.c b/src/security/security_stack.c index ed69b9c..d7e690a 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -127,6 +127,11 @@ virSecurityStackPreFork(virSecurityManagerPtr mgr) rc = -1; break; } + /* Undo the unbalanced locking left behind after recursion; if + * PostFork ever delegates to driver callbacks, we'd instead + * of to recurse to an internal method that does not regrab a + * lock. */ + virSecurityManagerPostFork(item->securityManager); } return rc; -- 1.8.3.1

On Fri, Jul 19, 2013 at 09:09:40AM -0600, Eric Blake wrote:
Attempts to start a domain with both SELinux and DAC security modules loaded will deadlock; latent problem introduced in commit fdb3bde and exposed in commit 29fe5d7. Basically, when recursing into the security manager for other driver's prefork, we have to undo the asymmetric lock taken at the manager level.
Reported by Jiri Denemark, with diagnosis help from Dan Berrange.
* src/security/security_stack.c (virSecurityStackPreFork): Undo extra lock grabbed during recursion.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
src/security/security_stack.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/security/security_stack.c b/src/security/security_stack.c index ed69b9c..d7e690a 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -127,6 +127,11 @@ virSecurityStackPreFork(virSecurityManagerPtr mgr) rc = -1; break; } + /* Undo the unbalanced locking left behind after recursion; if + * PostFork ever delegates to driver callbacks, we'd instead + * of to recurse to an internal method that does not regrab a + * lock. */ + virSecurityManagerPostFork(item->securityManager); }
return rc;
ACK 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 :|

On 07/19/2013 09:11 AM, Daniel P. Berrange wrote:
On Fri, Jul 19, 2013 at 09:09:40AM -0600, Eric Blake wrote:
Attempts to start a domain with both SELinux and DAC security modules loaded will deadlock; latent problem introduced in commit fdb3bde and exposed in commit 29fe5d7. Basically, when recursing into the security manager for other driver's prefork, we have to undo the asymmetric lock taken at the manager level.
Reported by Jiri Denemark, with diagnosis help from Dan Berrange.
* src/security/security_stack.c (virSecurityStackPreFork): Undo extra lock grabbed during recursion.
ACK
Thanks, and sorry for the breakage (serves me right for not testing the stack driver closely enough). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange
-
Eric Blake