9 a.m.
There is a minor bug when running QEMU non-root, and having
capng enabled. libvirt is unable to write the PID file in
/var/run/libvirt/qemu, since its now owned by 'qemu', but
libvirtd has dropped all capabilties at this point. The fix
is to delay dropping capabilities until after the PID file
has been created. We should also be sure to kill the child
if writing the PID file fails
Daniel
commit ce246587178bc6539a3ea6181cdf06ea45878fd3
Author: Daniel P. Berrange <berrange(a)redhat.com>
Date: Thu Jul 30 14:58:16 2009 +0100
Fix problem writing QEMU pidfile
* src/util.c: Don't drop capabilities until after the PID file has
been written. Kill off child if writing the PID file fails
* src/qemu_driver.c: Remove bogus trailing '/' in state dir
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 9fb8506..26897d3 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -468,7 +468,7 @@ qemudStartup(int privileged) {
goto out_of_memory;
if (virAsprintf(&qemu_driver->stateDir,
- "%s/run/libvirt/qemu/", LOCAL_STATE_DIR) == -1)
+ "%s/run/libvirt/qemu", LOCAL_STATE_DIR) == -1)
goto out_of_memory;
} else {
uid_t uid = geteuid();
diff --git a/src/util.c b/src/util.c
index ee64b28..39aae24 100644
--- a/src/util.c
+++ b/src/util.c
@@ -513,12 +513,6 @@ __virExec(virConnectPtr conn,
if ((hook)(data) != 0)
_exit(1);
- /* The hook above may need todo something privileged, so
- * we delay clearing capabilities until now */
- if ((flags & VIR_EXEC_CLEAR_CAPS) &&
- virClearCapabilities() < 0)
- _exit(1);
-
/* Daemonize as late as possible, so the parent process can detect
* the above errors with wait* */
if (flags & VIR_EXEC_DAEMON) {
@@ -543,6 +537,9 @@ __virExec(virConnectPtr conn,
if (pid > 0) {
if (pidfile && virFileWritePidPath(pidfile,pid)) {
+ kill(pid, SIGTERM);
+ usleep(500*1000);
+ kill(pid, SIGTERM);
virReportSystemError(conn, errno,
"%s", _("could not write
pidfile"));
_exit(1);
@@ -551,6 +548,12 @@ __virExec(virConnectPtr conn,
}
}
+ /* The steps above may need todo something privileged, so
+ * we delay clearing capabilities until the last minute */
+ if ((flags & VIR_EXEC_CLEAR_CAPS) &&
+ virClearCapabilities() < 0)
+ _exit(1);
+
if (envp)
execve(argv[0], (char **) argv, (char**)envp);
else
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
9:12 a.m.
On Thu, Jul 30, 2009 at 03:00:53PM +0100, Daniel P. Berrange wrote:
There is a minor bug when running QEMU non-root, and having
capng enabled. libvirt is unable to write the PID file in
/var/run/libvirt/qemu, since its now owned by 'qemu', but
libvirtd has dropped all capabilties at this point. The fix
is to delay dropping capabilities until after the PID file
has been created. We should also be sure to kill the child
if writing the PID file fails
[...]
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 9fb8506..26897d3 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -468,7 +468,7 @@ qemudStartup(int privileged) {
goto out_of_memory;
if (virAsprintf(&qemu_driver->stateDir,
- "%s/run/libvirt/qemu/", LOCAL_STATE_DIR) == -1)
+ "%s/run/libvirt/qemu", LOCAL_STATE_DIR) == -1)
goto out_of_memory;
} else {
uid_t uid = geteuid();
unrelated but fine
diff --git a/src/util.c b/src/util.c
index ee64b28..39aae24 100644
--- a/src/util.c
+++ b/src/util.c
@@ -513,12 +513,6 @@ __virExec(virConnectPtr conn,
if ((hook)(data) != 0)
_exit(1);
- /* The hook above may need todo something privileged, so
- * we delay clearing capabilities until now */
- if ((flags & VIR_EXEC_CLEAR_CAPS) &&
- virClearCapabilities() < 0)
- _exit(1);
-
/* Daemonize as late as possible, so the parent process can detect
* the above errors with wait* */
if (flags & VIR_EXEC_DAEMON) {
@@ -543,6 +537,9 @@ __virExec(virConnectPtr conn,
if (pid > 0) {
if (pidfile && virFileWritePidPath(pidfile,pid)) {
+ kill(pid, SIGTERM);
+ usleep(500*1000);
+ kill(pid, SIGTERM);
virReportSystemError(conn, errno,
"%s", _("could not write
pidfile"));
_exit(1);
minor nitpick I would error first and then do the kill
@@ -551,6 +548,12 @@ __virExec(virConnectPtr conn,
}
}
+ /* The steps above may need todo something privileged, so
+ * we delay clearing capabilities until the last minute */
+ if ((flags & VIR_EXEC_CLEAR_CAPS) &&
+ virClearCapabilities() < 0)
+ _exit(1);
+
if (envp)
execve(argv[0], (char **) argv, (char**)envp);
else
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
3:28 a.m.
On Thu, 2009-07-30 at 15:00 +0100, Daniel P. Berrange wrote:
There is a minor bug when running QEMU non-root, and having
capng enabled. libvirt is unable to write the PID file in
/var/run/libvirt/qemu, since its now owned by 'qemu', but
libvirtd has dropped all capabilties at this point. The fix
is to delay dropping capabilities until after the PID file
has been created. We should also be sure to kill the child
if writing the PID file fails
I haven't looked into it much yet, but don't we need to open up the
permissions on /var/lib/libvirt/images now? At least from 700 to 711 so
qemu can open images?
Cheers,
Mark.
3:41 a.m.
On Fri, Jul 31, 2009 at 09:28:37AM +0100, Mark McLoughlin wrote:
On Thu, 2009-07-30 at 15:00 +0100, Daniel P. Berrange wrote:
> There is a minor bug when running QEMU non-root, and having
> capng enabled. libvirt is unable to write the PID file in
> /var/run/libvirt/qemu, since its now owned by 'qemu', but
> libvirtd has dropped all capabilties at this point. The fix
> is to delay dropping capabilities until after the PID file
> has been created. We should also be sure to kill the child
> if writing the PID file fails
I haven't looked into it much yet, but don't we need to open up the
permissions on /var/lib/libvirt/images now? At least from 700 to 711 so
qemu can open images?
Hmm, that's a good point, we definitely need to do that. 711 shoudl be
good because that lets us chmod the individual imagges to allow QEMU
user to open them, while not allowing people to list the contents of
the directory
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
3:51 a.m.
On Fri, 2009-07-31 at 09:41 +0100, Daniel P. Berrange wrote:
On Fri, Jul 31, 2009 at 09:28:37AM +0100, Mark McLoughlin wrote:
> On Thu, 2009-07-30 at 15:00 +0100, Daniel P. Berrange wrote:
> > There is a minor bug when running QEMU non-root, and having
> > capng enabled. libvirt is unable to write the PID file in
> > /var/run/libvirt/qemu, since its now owned by 'qemu', but
> > libvirtd has dropped all capabilties at this point. The fix
> > is to delay dropping capabilities until after the PID file
> > has been created. We should also be sure to kill the child
> > if writing the PID file fails
>
> I haven't looked into it much yet, but don't we need to open up the
> permissions on /var/lib/libvirt/images now? At least from 700 to 711 so
> qemu can open images?
Hmm, that's a good point, we definitely need to do that. 711 shoudl be
good because that lets us chmod the individual imagges to allow QEMU
user to open them, while not allowing people to list the contents of
the directory
Okay, committing this.
Cheers,
Mark.
From: Mark McLoughlin <markmc(a)redhat.com>
Subject: [PATCH] Set perms on /var/lib/libvirt/images to 0711
Allow qemu user to open images in this dir, but still prevent others
from listing it.
* libvirt.spec.in: set /var/lib/libvirt/images perms to 0711
---
libvirt.spec.in | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index c295629..fdc2210 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -489,7 +489,7 @@ fi
%dir %{_localstatedir}/run/libvirt/
%dir %{_localstatedir}/lib/libvirt/
-%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/images/
+%dir %attr(0711, root, root) %{_localstatedir}/lib/libvirt/images/
%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/boot/
%dir %attr(0700, root, root) %{_localstatedir}/cache/libvirt/
--
1.6.2.5
5630
days inactive
5631
days old
4 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Mark McLoughlin