[libvirt] [PATCH] qemudDomainMemoryPeek: chown temporary file to qemu.qemu.
This attached patch resolves a fairly obvious problem which stops qemuDomainMemoryPeek from working at all. However, it's not the whole story. Read on ... (1) qemu still fails unless I manually set the mode of /var/cache/libvirt to 0755 (it is normally 0700). Owner Fails Works /var/cache/libvirt root.root 0700 0755 /var/cache/libvirt/qemu qemu.qemu 0750 0750 qemu is doing: open("/var/cache/libvirt/qemu/qemu.mem.fdVCod", O_WRONLY|O_CREAT|O_TRUNC, 0666) It's kinda annoying that /etc/libvirt and /var/{cache,lib}/libvirt are unreadable by other users. Is there some deep reason to do this? (2) After applying this patch, using virDomainMemoryPeek causes libvirtd to generate the following serious-looking warning. It doesn't appear to crash or fail in any way that I can tell: 15:17:09.982: 7784: error : virDomainFree:2122 : invalid domain pointer in virDomainFree I don't know where this is coming from. It only appears when my program actually does virDomainMemoryPeek, not when I have the same code with virDomainMemoryPeek commented out. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
On Fri, May 20, 2011 at 03:28:14PM +0100, Richard W.M. Jones wrote:
This attached patch resolves a fairly obvious problem which stops qemuDomainMemoryPeek from working at all.
However, it's not the whole story. Read on ...
(1) qemu still fails unless I manually set the mode of /var/cache/libvirt to 0755 (it is normally 0700).
Owner Fails Works /var/cache/libvirt root.root 0700 0755 /var/cache/libvirt/qemu qemu.qemu 0750 0750
qemu is doing:
open("/var/cache/libvirt/qemu/qemu.mem.fdVCod", O_WRONLY|O_CREAT|O_TRUNC, 0666)
It's kinda annoying that /etc/libvirt and /var/{cache,lib}/libvirt are unreadable by other users. Is there some deep reason to do this?
Both /etc/libvirt & /var/lib/libvirt stores files which may contain passwords. /var/cache/libvirt contains data such as these memory dumps which potentially have sensitive data in them. Unprivileges users on the host must be prevented from seeing any of this data unless authorized. I think we likely need /var/cache/libvirt to be 0711 so that QEMU can access directories below it, but not actually read it.
(2) After applying this patch, using virDomainMemoryPeek causes libvirtd to generate the following serious-looking warning. It doesn't appear to crash or fail in any way that I can tell:
15:17:09.982: 7784: error : virDomainFree:2122 : invalid domain pointer in virDomainFree
I don't know where this is coming from. It only appears when my program actually does virDomainMemoryPeek, not when I have the same code with virDomainMemoryPeek commented out.
Oh, there is a bogus 'if (dom) virDomainFree(dom)' call in the remote dispatcher remoteDispatchDomainMemoryPeek
From 1a2ba0c1b58142a06602722c6bb0995ef6e8b347 Mon Sep 17 00:00:00 2001 From: Richard W.M. Jones <rjones@redhat.com> Date: Fri, 20 May 2011 13:56:46 +0100 Subject: [PATCH] qemudDomainMemoryPeek: chown temporary file to qemu.qemu.
Otherwise qemu is unable to write to it, with the error:
libvir: QEMU error : internal error unable to execute QEMU command 'memsave': Could not open '/var/cache/libvirt/qemu/qemu.mem.RRNvLv' --- src/qemu/qemu_driver.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 44acc6a..08d2549 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5536,6 +5536,14 @@ qemudDomainMemoryPeek (virDomainPtr dom, goto endjob; }
+ if (qemu_driver->privileged && + chown(tmp, qemu_driver->user, qemu_driver->group) < 0) { + virReportSystemError(errno, + _("unable to set ownership on %s to %d:%d"), + tmp, qemu_driver->user, qemu_driver->group); + goto endjob; + }
We will also need to set the SELinux context on the file. So instead of directly using chown, we need to call virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, vm, tmp); and after the monitor command completes, run virSecurityManagerRestoreSavedStateLabel(qemu_driver->securityManager, vm, tmp); to revoke QEMU's permission again 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 Fri, May 20, 2011 at 03:40:35PM +0100, Daniel P. Berrange wrote:
I think we likely need /var/cache/libvirt to be 0711 so that QEMU can access directories below it, but not actually read it.
0711 does indeed work fine. However, where/what sets this?
Oh, there is a bogus 'if (dom) virDomainFree(dom)' call in the remote dispatcher remoteDispatchDomainMemoryPeek
Ah, well spotted! The attached patch does indeed remove the warning/error.
We will also need to set the SELinux context on the file. So instead of directly using chown, we need to call
virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, vm, tmp);
OK, this works -- see updated patch attached.
and after the monitor command completes, run
virSecurityManagerRestoreSavedStateLabel(qemu_driver->securityManager, vm, tmp);
This says: 15:52:28.144: 11128: warning : SELinuxRestoreSecurityFileLabel:460 : cannot lookup default selinux label for /var/cache/libvirt/qemu/qemu.mem.Cjn86L Is it really necessary to restore the label for a file we're going to delete? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
On Fri, May 20, 2011 at 03:57:03PM +0100, Richard W.M. Jones wrote:
On Fri, May 20, 2011 at 03:40:35PM +0100, Daniel P. Berrange wrote:
I think we likely need /var/cache/libvirt to be 0711 so that QEMU can access directories below it, but not actually read it.
0711 does indeed work fine. However, where/what sets this?
The RPM specfile %files section is in charge.
Oh, there is a bogus 'if (dom) virDomainFree(dom)' call in the remote dispatcher remoteDispatchDomainMemoryPeek
Ah, well spotted! The attached patch does indeed remove the warning/error.
We will also need to set the SELinux context on the file. So instead of directly using chown, we need to call
virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, vm, tmp);
OK, this works -- see updated patch attached.
and after the monitor command completes, run
virSecurityManagerRestoreSavedStateLabel(qemu_driver->securityManager, vm, tmp);
This says:
15:52:28.144: 11128: warning : SELinuxRestoreSecurityFileLabel:460 : cannot lookup default selinux label for /var/cache/libvirt/qemu/qemu.mem.Cjn86L
Is it really necessary to restore the label for a file we're going to delete?
No, not really required.
From db103b9f9f5c3916d3f6eafea8d732cad01ab979 Mon Sep 17 00:00:00 2001 From: Richard W.M. Jones <rjones@redhat.com> Date: Fri, 20 May 2011 13:56:46 +0100 Subject: [PATCH 1/2] qemudDomainMemoryPeek: change ownership/selinux label on temporary file.
Otherwise qemu is unable to write to it, with the error:
libvir: QEMU error : internal error unable to execute QEMU command 'memsave': Could not open '/var/cache/libvirt/qemu/qemu.mem.RRNvLv' --- src/qemu/qemu_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 44acc6a..691965d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5536,6 +5536,8 @@ qemudDomainMemoryPeek (virDomainPtr dom, goto endjob; }
+ virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, vm, tmp); + priv = vm->privateData; qemuDomainObjEnterMonitor(vm); if (flags == VIR_MEMORY_VIRTUAL) {
From b01b6232ff0bff85d5c2521ce1f75ca18718333c Mon Sep 17 00:00:00 2001 From: Richard W.M. Jones <rjones@redhat.com> Date: Fri, 20 May 2011 15:55:40 +0100 Subject: [PATCH 2/2] remote: remove bogus virDomainFree.
--- daemon/remote.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 42e1cb9..941e92f 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -916,8 +916,6 @@ remoteDispatchDomainMemoryPeek(struct qemud_server *server ATTRIBUTE_UNUSED, if (virDomainMemoryPeek(dom, offset, size, ret->buffer.buffer_val, flags) < 0) goto cleanup; - if (dom) - virDomainFree(dom);
rv = 0;
ACK to both. 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 Fri, May 20, 2011 at 04:02:41PM +0100, Daniel P. Berrange wrote:
On Fri, May 20, 2011 at 03:57:03PM +0100, Richard W.M. Jones wrote:
On Fri, May 20, 2011 at 03:40:35PM +0100, Daniel P. Berrange wrote:
I think we likely need /var/cache/libvirt to be 0711 so that QEMU can access directories below it, but not actually read it.
0711 does indeed work fine. However, where/what sets this?
The RPM specfile %files section is in charge.
OK, please see the attached patch. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
On Fri, May 20, 2011 at 04:19:55PM +0100, Richard W.M. Jones wrote:
On Fri, May 20, 2011 at 04:02:41PM +0100, Daniel P. Berrange wrote:
On Fri, May 20, 2011 at 03:57:03PM +0100, Richard W.M. Jones wrote:
On Fri, May 20, 2011 at 03:40:35PM +0100, Daniel P. Berrange wrote:
I think we likely need /var/cache/libvirt to be 0711 so that QEMU can access directories below it, but not actually read it.
0711 does indeed work fine. However, where/what sets this?
The RPM specfile %files section is in charge.
OK, please see the attached patch.
Rich.
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
From ee9ceb3295e8e68da4c5838c57cc3c1e3a5dccfb Mon Sep 17 00:00:00 2001 From: Richard W.M. Jones <rjones@redhat.com> Date: Fri, 20 May 2011 16:18:11 +0100 Subject: [PATCH] libvirt.spec: /var/cache/libvirt should be 0711.
This allows qemu to create files in /var/cache/libvirt/qemu/, and specifically is required to fix virDomainMemoryPeek. --- libvirt.spec.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index 8a6912f..3ce9ebe 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -932,7 +932,7 @@ fi
%dir %attr(0711, root, root) %{_localstatedir}/lib/libvirt/images/ %dir %attr(0711, root, root) %{_localstatedir}/lib/libvirt/boot/ -%dir %attr(0700, root, root) %{_localstatedir}/cache/libvirt/ +%dir %attr(0711, root, root) %{_localstatedir}/cache/libvirt/
%if %{with_qemu} %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/
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 :|
participants (2)
-
Daniel P. Berrange -
Richard W.M. Jones