From 5e5acbc8d67e1ac074320176bbc3682b9ba934c0 Mon Sep 17 00:00:00 2001 From: Jiri Denemark Date: Fri, 7 Jan 2011 12:34:12 +0100 Subject: [PATCH] daemon: Fix core dumps if unix_sock_group is set Setting unix_sock_group to something else than default "root" in /etc/libvirt/libvirtd.conf prevents system libvirtd from dumping core on crash. This is because we used setgid(unix_sock_group) before binding to /var/run/libvirt/libvirt-sock* and setgid() back to original group. However, if a process changes its effective or filesystem group ID, it will be forbidden from leaving core dumps unless fs.suid_dumpable sysctl is set to something else then 0 (and it is 0 by default). Changing socket's group ownership after bind works better. And we can do so without introducing a race condition since we loosen access rights by changing the group from root to something else. --- daemon/libvirtd.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 2df9337279..b1539b10d3 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -542,7 +542,6 @@ static int qemudListenUnix(struct qemud_server *server, char *path, int readonly, int auth) { struct qemud_socket *sock; mode_t oldmask; - gid_t oldgrp; char ebuf[1024]; if (VIR_ALLOC(sock) < 0) { @@ -579,21 +578,21 @@ static int qemudListenUnix(struct qemud_server *server, if (sock->addr.data.un.sun_path[0] == '@') sock->addr.data.un.sun_path[0] = '\0'; - oldgrp = getgid(); oldmask = umask(readonly ? ~unix_sock_ro_mask : ~unix_sock_rw_mask); - if (server->privileged && setgid(unix_sock_gid)) { - VIR_ERROR(_("Failed to set group ID to %d"), unix_sock_gid); - goto cleanup; - } - if (bind(sock->fd, &sock->addr.data.sa, sock->addr.len) < 0) { VIR_ERROR(_("Failed to bind socket to '%s': %s"), path, virStrerror(errno, ebuf, sizeof ebuf)); goto cleanup; } umask(oldmask); - if (server->privileged && setgid(oldgrp)) { - VIR_ERROR(_("Failed to restore group ID to %d"), oldgrp); + + /* chown() doesn't work for abstract sockets but we use them only + * if libvirtd runs unprivileged + */ + if (server->privileged && chown(path, -1, unix_sock_gid)) { + VIR_ERROR(_("Failed to change group ID of '%s' to %d: %s"), + path, unix_sock_gid, + virStrerror(errno, ebuf, sizeof ebuf)); goto cleanup; }