[libvirt] [PATCH] Avoid async signal safety problem in glibc's setxid

The glibc setxid is supposed to be async signal safe, but libc developers confirm that it is not. This causes a problem when libvirt_lxc starts the FUSE thread and then runs clone() to start the container. If the clone() was done before the FUSE thread has completely started up, then the container will hang in setxid after clone(). The fix is to avoid creating any threads until after the container has been clone()'d. By avoiding any threads in the parent, the child is no longer required to run in an async signal safe context, and we thus avoid the glibc bug. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_controller.c | 11 +++++++++-- src/lxc/lxc_fuse.c | 21 +++++++++++++++------ src/lxc/lxc_fuse.h | 1 + 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 232af54..c013147 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1983,6 +1983,12 @@ virLXCControllerSetupFuse(virLXCControllerPtr ctrl) } static int +virLXCControllerStartFuse(virLXCControllerPtr ctrl) +{ + return lxcStartFuse(ctrl->fuse); +} + +static int virLXCControllerSetupConsoles(virLXCControllerPtr ctrl, char **containerTTYPaths) { @@ -2187,6 +2193,9 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerMoveInterfaces(ctrl) < 0) goto cleanup; + if (virLXCControllerStartFuse(ctrl) < 0) + goto cleanup; + if (lxcContainerSendContinue(control[0]) < 0) { virReportSystemError(errno, "%s", _("Unable to send container continue message")); @@ -2199,8 +2208,6 @@ virLXCControllerRun(virLXCControllerPtr ctrl) goto cleanup; } - /* Now the container is fully setup... */ - /* ...and reduce our privileges */ if (lxcControllerClearCapabilities() < 0) goto cleanup; diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 9d12832..88e122e 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -322,12 +322,6 @@ int lxcSetupFuse(virLXCFusePtr *f, virDomainDefPtr def) goto cleanup1; } - if (virThreadCreate(&fuse->thread, false, lxcFuseRun, - (void *)fuse) < 0) { - lxcFuseDestroy(fuse); - goto cleanup1; - } - ret = 0; cleanup: fuse_opt_free_args(&args); @@ -341,6 +335,17 @@ cleanup2: goto cleanup; } +int lxcStartFuse(virLXCFusePtr fuse) +{ + if (virThreadCreate(&fuse->thread, false, lxcFuseRun, + (void *)fuse) < 0) { + lxcFuseDestroy(fuse); + return -1; + } + + return 0; +} + void lxcFreeFuse(virLXCFusePtr *f) { virLXCFusePtr fuse = *f; @@ -364,6 +369,10 @@ int lxcSetupFuse(virLXCFusePtr *f ATTRIBUTE_UNUSED, return 0; } +int lxcStartFuse(virLXCFusePtr f ATTRIBUTE_UNUSED) +{ +} + void lxcFreeFuse(virLXCFusePtr *f ATTRIBUTE_UNUSED) { } diff --git a/src/lxc/lxc_fuse.h b/src/lxc/lxc_fuse.h index b3713af..d60492b 100644 --- a/src/lxc/lxc_fuse.h +++ b/src/lxc/lxc_fuse.h @@ -58,6 +58,7 @@ struct virLXCFuse { typedef struct virLXCFuse *virLXCFusePtr; extern int lxcSetupFuse(virLXCFusePtr *f, virDomainDefPtr def); +extern int lxcStartFuse(virLXCFusePtr f); extern void lxcFreeFuse(virLXCFusePtr *f); #endif /* LXC_FUSE_H */ -- 1.8.4.2

On 11/15/2013 09:20 AM, Daniel P. Berrange wrote:
The glibc setxid is supposed to be async signal safe, but libc developers confirm that it is not. This causes a problem when libvirt_lxc starts the FUSE thread and then runs clone() to start the container. If the clone() was done before the FUSE thread has completely started up, then the container will hang in setxid after clone().
The fix is to avoid creating any threads until after the container has been clone()'d. By avoiding any threads in the parent, the child is no longer required to run in an async signal safe context, and we thus avoid the glibc bug.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_controller.c | 11 +++++++++-- src/lxc/lxc_fuse.c | 21 +++++++++++++++------ src/lxc/lxc_fuse.h | 1 + 3 files changed, 25 insertions(+), 8 deletions(-)
I can review the code, but I'd feel better if this also got field testing as resolving the problem before you push it. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Fri, Nov 15, 2013 at 10:37:53AM -0700, Eric Blake wrote:
On 11/15/2013 09:20 AM, Daniel P. Berrange wrote:
The glibc setxid is supposed to be async signal safe, but libc developers confirm that it is not. This causes a problem when libvirt_lxc starts the FUSE thread and then runs clone() to start the container. If the clone() was done before the FUSE thread has completely started up, then the container will hang in setxid after clone().
The fix is to avoid creating any threads until after the container has been clone()'d. By avoiding any threads in the parent, the child is no longer required to run in an async signal safe context, and we thus avoid the glibc bug.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_controller.c | 11 +++++++++-- src/lxc/lxc_fuse.c | 21 +++++++++++++++------ src/lxc/lxc_fuse.h | 1 + 3 files changed, 25 insertions(+), 8 deletions(-)
I can review the code, but I'd feel better if this also got field testing as resolving the problem before you push it.
ACK.
I was able to reproduce the problem one time in 3 without the patch and it appears gone after applying it. 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 11/16/2013 12:20 AM, Daniel P. Berrange wrote:
The glibc setxid is supposed to be async signal safe, but libc developers confirm that it is not. This causes a problem when libvirt_lxc starts the FUSE thread and then runs clone() to start the container. If the clone() was done before the FUSE thread has completely started up, then the container will hang in setxid after clone().
The fix is to avoid creating any threads until after the container has been clone()'d. By avoiding any threads in the parent, the child is no longer required to run in an async signal safe context, and we thus avoid the glibc bug.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
AC thanks
--- src/lxc/lxc_controller.c | 11 +++++++++-- src/lxc/lxc_fuse.c | 21 +++++++++++++++------ src/lxc/lxc_fuse.h | 1 + 3 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 232af54..c013147 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1983,6 +1983,12 @@ virLXCControllerSetupFuse(virLXCControllerPtr ctrl) }
static int +virLXCControllerStartFuse(virLXCControllerPtr ctrl) +{ + return lxcStartFuse(ctrl->fuse); +} + +static int virLXCControllerSetupConsoles(virLXCControllerPtr ctrl, char **containerTTYPaths) { @@ -2187,6 +2193,9 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerMoveInterfaces(ctrl) < 0) goto cleanup;
+ if (virLXCControllerStartFuse(ctrl) < 0) + goto cleanup; + if (lxcContainerSendContinue(control[0]) < 0) { virReportSystemError(errno, "%s", _("Unable to send container continue message")); @@ -2199,8 +2208,6 @@ virLXCControllerRun(virLXCControllerPtr ctrl) goto cleanup; }
- /* Now the container is fully setup... */ - /* ...and reduce our privileges */ if (lxcControllerClearCapabilities() < 0) goto cleanup; diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 9d12832..88e122e 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -322,12 +322,6 @@ int lxcSetupFuse(virLXCFusePtr *f, virDomainDefPtr def) goto cleanup1; }
- if (virThreadCreate(&fuse->thread, false, lxcFuseRun, - (void *)fuse) < 0) { - lxcFuseDestroy(fuse); - goto cleanup1; - } - ret = 0; cleanup: fuse_opt_free_args(&args); @@ -341,6 +335,17 @@ cleanup2: goto cleanup; }
+int lxcStartFuse(virLXCFusePtr fuse) +{ + if (virThreadCreate(&fuse->thread, false, lxcFuseRun, + (void *)fuse) < 0) { + lxcFuseDestroy(fuse); + return -1; + } + + return 0; +} + void lxcFreeFuse(virLXCFusePtr *f) { virLXCFusePtr fuse = *f; @@ -364,6 +369,10 @@ int lxcSetupFuse(virLXCFusePtr *f ATTRIBUTE_UNUSED, return 0; }
+int lxcStartFuse(virLXCFusePtr f ATTRIBUTE_UNUSED) +{ +} + void lxcFreeFuse(virLXCFusePtr *f ATTRIBUTE_UNUSED) { } diff --git a/src/lxc/lxc_fuse.h b/src/lxc/lxc_fuse.h index b3713af..d60492b 100644 --- a/src/lxc/lxc_fuse.h +++ b/src/lxc/lxc_fuse.h @@ -58,6 +58,7 @@ struct virLXCFuse { typedef struct virLXCFuse *virLXCFusePtr;
extern int lxcSetupFuse(virLXCFusePtr *f, virDomainDefPtr def); +extern int lxcStartFuse(virLXCFusePtr f); extern void lxcFreeFuse(virLXCFusePtr *f);
#endif /* LXC_FUSE_H */

On 15.11.2013 17:20, Daniel P. Berrange wrote:
The glibc setxid is supposed to be async signal safe, but libc developers confirm that it is not. This causes a problem when libvirt_lxc starts the FUSE thread and then runs clone() to start the container. If the clone() was done before the FUSE thread has completely started up, then the container will hang in setxid after clone().
The fix is to avoid creating any threads until after the container has been clone()'d. By avoiding any threads in the parent, the child is no longer required to run in an async signal safe context, and we thus avoid the glibc bug.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
[...]
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 9d12832..88e122e 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c
[...]
@@ -364,6 +369,10 @@ int lxcSetupFuse(virLXCFusePtr *f ATTRIBUTE_UNUSED, return 0; }
+int lxcStartFuse(virLXCFusePtr f ATTRIBUTE_UNUSED) +{ +} + void lxcFreeFuse(virLXCFusePtr *f ATTRIBUTE_UNUSED) { }
Hi Daniel, This hunk breaks the build on RHEL-6.4. lxc/lxc_fuse.c:374: error: control reaches end of non-void function [-Wreturn-type] Pavel

On Mon, Nov 18, 2013 at 05:09:31PM +0100, Pavel Hrdina wrote:
On 15.11.2013 17:20, Daniel P. Berrange wrote:
The glibc setxid is supposed to be async signal safe, but libc developers confirm that it is not. This causes a problem when libvirt_lxc starts the FUSE thread and then runs clone() to start the container. If the clone() was done before the FUSE thread has completely started up, then the container will hang in setxid after clone().
The fix is to avoid creating any threads until after the container has been clone()'d. By avoiding any threads in the parent, the child is no longer required to run in an async signal safe context, and we thus avoid the glibc bug.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
[...]
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 9d12832..88e122e 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c
[...]
@@ -364,6 +369,10 @@ int lxcSetupFuse(virLXCFusePtr *f ATTRIBUTE_UNUSED, return 0; }
+int lxcStartFuse(virLXCFusePtr f ATTRIBUTE_UNUSED) +{ +} + void lxcFreeFuse(virLXCFusePtr *f ATTRIBUTE_UNUSED) { }
Hi Daniel,
This hunk breaks the build on RHEL-6.4. lxc/lxc_fuse.c:374: error: control reaches end of non-void function [-Wreturn-type]
Fixed as commit 784bb73eaa5507375c21f801376a512dc6ae658d Author: Daniel P. Berrange <berrange@redhat.com> Date: Mon Nov 18 16:12:39 2013 +0000 Add missing 'return 0;' in stub lxcStartFuse() method impl. Without a 'return 0' in the stub lxcStartFuse() method, the compiler warns: lxc/lxc_fuse.c:374: error: control reaches end of non-void function [-Wreturn-type] Signed-off-by: Daniel P. Berrange <berrange@redhat.com> diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 88e122e..d3d8f85 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -371,6 +371,7 @@ int lxcSetupFuse(virLXCFusePtr *f ATTRIBUTE_UNUSED, int lxcStartFuse(virLXCFusePtr f ATTRIBUTE_UNUSED) { + return 0; } void lxcFreeFuse(virLXCFusePtr *f ATTRIBUTE_UNUSED) 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 :|
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
Gao feng
-
Pavel Hrdina