Re: libvirt? qemu change that mmaps ELF files breaks libvirt svirt handling for os.kernel

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Oct 04, 2019 at 02:18:49PM +0200, Christian Borntraeger wrote:
> 
> 
> On 04.10.19 14:13, Paolo Bonzini wrote:
> > On 04/10/19 14:03, Christian Borntraeger wrote:
> >> Stefano, Paolo,
> >>
> >> I have an interesting fail in QEMU 
> >>
> >> 2019-10-04T12:00:32.675188Z qemu-system-s390x: GLib: g_mapped_file_unref: assertion 'file != NULL' failed
> >> that bisected to 
> >> commit 816b9fe450220e19acb91a0ce4a8ade7000648d1 (refs/bisect/bad)
> >>     elf-ops.h: Map into memory the ELF to load
> >>
> >> strace tells that I can read the ELF file, but not mmap
> >> strace:
> >> 214365 openat(AT_FDCWD, "/var/lib/libvirt/images/test_cpu_timer.elf", O_RDONLY) = 36
> >> 214365 read(46, "\177ELF\2\2\1\0\0\0\0\0\0\0\0\0", 16) = 16
> >> 214365 lseek(46, 0, SEEK_SET)           = 0
> >> [...]
> >> 214365 fstat(46, {st_mode=S_IFREG|0755, st_size=168176, ...}) = 0
> >> 214365 mmap(NULL, 168176, PROT_READ|PROT_WRITE, MAP_PRIVATE, 46, 0) = -1 EACCES (Permission denied)
> >>
> >> So reading from /var/lib/libvirt/images/test_cpu_timer.elf does work, mmaping does not.
> >> setenforce 0 makes the problem go away. 
> >>
> >> This might be more of an issue in libvirt, setting the svirt context too
> >> restrictive, but I am not too deep into the svirt part of libvirt.
> >> Reverting the qemu commit makes the problem go away.
> > 
> > Yes, the policy is too restrictive in my opinion.
> > 
> > Can you include the output of "audit2allow" and/or "audit2allow -R"?
> > 
> > Thanks,
> > 
> > Paolo
> > 
> 
> require {
> 	type unconfined_t;
> 	type virt_content_t;
> 	type svirt_t;
> 	type systemd_tmpfiles_t;
> 	type user_home_t;
> 	type NetworkManager_t;
> 	class file { entrypoint execute ioctl lock map open read write };
> 	class bpf prog_run;
> }
> 
> #============= svirt_t ==============
> allow svirt_t user_home_t:file { entrypoint execute ioctl lock open read write };
> 
> #!!!! This avc can be allowed using the boolean 'domain_can_mmap_files'

This is an unrelated boolean and we don't want to use that so ignore
this suggestion !

> allow svirt_t virt_content_t:file map;

This matches your strace.  virt_content_t is the label we use on
files that are exposed to QEMU read-only.

The svirt policy only allows mmap on files that re exposed read-write
currently.

I believe we can safely allow this mmap on read-only files too
because  the read-only restriction is enforced at time of open,
and any writes to the mapped file  will result in a private
copy on write.

Please file a bz entry against the selinux-policy component for
whatever distro you're using, and/or Fedora rawhide, and CC me
on it too.

> corecmd_bin_entry_type(svirt_t)
> userdom_manage_user_home_content_dirs(svirt_t)
> userdom_map_user_home_files(svirt_t)
> virt_rw_svirt_image(svirt_t)

These look unrelated to the problem above.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|