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

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.