[libvirt] [PATCH] Don't use O_TRUNC when opening QEMU logfiles

From: "Daniel P. Berrange" <berrange@redhat.com> SELinux wants all log files opened with O_APPEND. When running non-root though, libvirtd likes to use O_TRUNC to avoid log files growing in size indefinitely. Instead of using O_TRUNC though, we can use O_APPEND and then call ftruncate() which keeps SELinux happier. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_domain.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0ae30b7..e67c247 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1446,12 +1446,22 @@ qemuDomainOpenLogHelper(struct qemud_driver *driver, { char *logfile; int fd = -1; + bool trunc = false; if (virAsprintf(&logfile, "%s/%s.log", driver->logDir, vm->def->name) < 0) { virReportOOMError(); return -1; } + /* To make SELinux happy we always need to open in append mode. + * So we fake O_TRUNC by calling ftruncate after open instead + */ + if (oflags & O_TRUNC) { + oflags &= ~O_TRUNC; + oflags |= O_APPEND; + trunc = true; + } + if ((fd = open(logfile, oflags, mode)) < 0) { virReportSystemError(errno, _("failed to create logfile %s"), logfile); @@ -1463,6 +1473,13 @@ qemuDomainOpenLogHelper(struct qemud_driver *driver, VIR_FORCE_CLOSE(fd); goto cleanup; } + if (trunc && + ftruncate(fd, 0) < 0) { + virReportSystemError(errno, _("failed to truncate %s"), + logfile); + VIR_FORCE_CLOSE(fd); + goto cleanup; + } cleanup: VIR_FREE(logfile); -- 1.7.11.2

On 21.09.2012 11:39, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
SELinux wants all log files opened with O_APPEND. When running non-root though, libvirtd likes to use O_TRUNC to avoid log files growing in size indefinitely. Instead of using O_TRUNC though, we can use O_APPEND and then call ftruncate() which keeps SELinux happier.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_domain.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
ACK Michal

On Fri, Sep 21, 2012 at 10:39:19 +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
SELinux wants all log files opened with O_APPEND. When running non-root though, libvirtd likes to use O_TRUNC to avoid log files growing in size indefinitely. Instead of using O_TRUNC though, we can use O_APPEND and then call ftruncate() which keeps SELinux happier.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_domain.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0ae30b7..e67c247 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1446,12 +1446,22 @@ qemuDomainOpenLogHelper(struct qemud_driver *driver, { char *logfile; int fd = -1; + bool trunc = false;
if (virAsprintf(&logfile, "%s/%s.log", driver->logDir, vm->def->name) < 0) { virReportOOMError(); return -1; }
+ /* To make SELinux happy we always need to open in append mode. + * So we fake O_TRUNC by calling ftruncate after open instead + */ + if (oflags & O_TRUNC) { + oflags &= ~O_TRUNC; + oflags |= O_APPEND; + trunc = true; + } + if ((fd = open(logfile, oflags, mode)) < 0) { virReportSystemError(errno, _("failed to create logfile %s"), logfile); @@ -1463,6 +1473,13 @@ qemuDomainOpenLogHelper(struct qemud_driver *driver, VIR_FORCE_CLOSE(fd); goto cleanup; } + if (trunc && + ftruncate(fd, 0) < 0) { + virReportSystemError(errno, _("failed to truncate %s"), + logfile); + VIR_FORCE_CLOSE(fd); + goto cleanup; + }
I wonder if it's necessary to fail here or if VIR_WARN would be sufficient. ACK in any case. Jirka

On Fri, Sep 21, 2012 at 10:39:19AM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
SELinux wants all log files opened with O_APPEND. When running non-root though, libvirtd likes to use O_TRUNC to avoid log files growing in size indefinitely. Instead of using O_TRUNC though, we can use O_APPEND and then call ftruncate() which keeps SELinux happier.
As far as I can see, although this patch doesn't break anything, it doesn't fix the SELinux problem either. SELinux still prevents qemu from writing to the log. The AVCs look the same as before: type=AVC msg=audit(1348227948.158:14174): avc: denied { append } for pid=13139 comm="qemu-kvm" path="/home/rjones/.cache/libvirt/qemu/log/guestfs-wd6efsxohmy5jd2s.log" dev="dm-5" ino=1870215 scontext=unconfined_u:unconfined_r:svirt_t:s0:c69,c512 tcontext=unconfined_u:object_r:cache_home_t:s0 tclass=file type=AVC msg=audit(1348227948.158:14174): avc: denied { append } for pid=13139 comm="qemu-kvm" path="/home/rjones/.cache/libvirt/qemu/log/guestfs-wd6efsxohmy5jd2s.log" dev="dm-5" ino=1870215 scontext=unconfined_u:unconfined_r:svirt_t:s0:c69,c512 tcontext=unconfined_u:object_r:cache_home_t:s0 tclass=file My test is to do: killall libvirtd lt-libvirtd ../libvirt/run guestfish -a /dev/null -- config "-xxx" "xxx" : run and then examine the ~/.cache/libvirt/qemu/log/guestfs-*.log to see if the error message appears there. The string "qemu-kvm: -xxx: invalid option" ought to appear in the log file. 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, Sep 21, 2012 at 12:54:25PM +0100, Richard W.M. Jones wrote:
On Fri, Sep 21, 2012 at 10:39:19AM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
SELinux wants all log files opened with O_APPEND. When running non-root though, libvirtd likes to use O_TRUNC to avoid log files growing in size indefinitely. Instead of using O_TRUNC though, we can use O_APPEND and then call ftruncate() which keeps SELinux happier.
As far as I can see, although this patch doesn't break anything, it doesn't fix the SELinux problem either. SELinux still prevents qemu from writing to the log. The AVCs look the same as before:
type=AVC msg=audit(1348227948.158:14174): avc: denied { append } for pid=13139 comm="qemu-kvm" path="/home/rjones/.cache/libvirt/qemu/log/guestfs-wd6efsxohmy5jd2s.log" dev="dm-5" ino=1870215 scontext=unconfined_u:unconfined_r:svirt_t:s0:c69,c512 tcontext=unconfined_u:object_r:cache_home_t:s0 tclass=file type=AVC msg=audit(1348227948.158:14174): avc: denied { append } for pid=13139 comm="qemu-kvm" path="/home/rjones/.cache/libvirt/qemu/log/guestfs-wd6efsxohmy5jd2s.log" dev="dm-5" ino=1870215 scontext=unconfined_u:unconfined_r:svirt_t:s0:c69,c512 tcontext=unconfined_u:object_r:cache_home_t:s0 tclass=file
The target context here is unconfined_u:object_r:cache_home_t:s0 which is wrong. The context ought to be virt_home_t instead of cache_home_t. Try changing the libvirt directories to have virt_home_t as their type. Then my patch ought to do something useful. We need to check if SELinux policy knows about $HOME/.cache/libvirt and $HOME/.config/libvirt, or whether it still only considers the old location of $HOME/.libvirt Regards, 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, Sep 21, 2012 at 01:00:26PM +0100, Daniel P. Berrange wrote:
On Fri, Sep 21, 2012 at 12:54:25PM +0100, Richard W.M. Jones wrote:
On Fri, Sep 21, 2012 at 10:39:19AM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
SELinux wants all log files opened with O_APPEND. When running non-root though, libvirtd likes to use O_TRUNC to avoid log files growing in size indefinitely. Instead of using O_TRUNC though, we can use O_APPEND and then call ftruncate() which keeps SELinux happier.
As far as I can see, although this patch doesn't break anything, it doesn't fix the SELinux problem either. SELinux still prevents qemu from writing to the log. The AVCs look the same as before:
type=AVC msg=audit(1348227948.158:14174): avc: denied { append } for pid=13139 comm="qemu-kvm" path="/home/rjones/.cache/libvirt/qemu/log/guestfs-wd6efsxohmy5jd2s.log" dev="dm-5" ino=1870215 scontext=unconfined_u:unconfined_r:svirt_t:s0:c69,c512 tcontext=unconfined_u:object_r:cache_home_t:s0 tclass=file type=AVC msg=audit(1348227948.158:14174): avc: denied { append } for pid=13139 comm="qemu-kvm" path="/home/rjones/.cache/libvirt/qemu/log/guestfs-wd6efsxohmy5jd2s.log" dev="dm-5" ino=1870215 scontext=unconfined_u:unconfined_r:svirt_t:s0:c69,c512 tcontext=unconfined_u:object_r:cache_home_t:s0 tclass=file
The target context here is unconfined_u:object_r:cache_home_t:s0 which is wrong. The context ought to be virt_home_t instead of cache_home_t. Try changing the libvirt directories to have virt_home_t as their type. Then my patch ought to do something useful.
This is correct: relabelling the log directory causes the patch to work.
We need to check if SELinux policy knows about $HOME/.cache/libvirt and $HOME/.config/libvirt, or whether it still only considers the old location of $HOME/.libvirt
I've filed a bug against SELinux: https://bugzilla.redhat.com/show_bug.cgi?id=859395 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
participants (4)
-
Daniel P. Berrange
-
Jiri Denemark
-
Michal Privoznik
-
Richard W.M. Jones